Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1623121

Re: [RFC 3/3] sched/topology: Different sched groups must not have the same balance cpu

From Rik van Riel <riel@redhat.com>
Newsgroups linux.kernel
Subject Re: [RFC 3/3] sched/topology: Different sched groups must not have the same balance cpu
Date 2017-04-13 17:30 +0200
Message-ID <tvOR4-4Ou-5@gated-at.bofh.it> (permalink)
References <tvNrX-3KR-5@gated-at.bofh.it> <tvNrY-3KR-29@gated-at.bofh.it>
Organization Red Hat, Inc.

Show all headers | View raw


On Thu, 2017-04-13 at 10:56 -0300, Lauro Ramos Venancio wrote:
> Currently, the group balance cpu is the groups's first CPU. But with
> overlapping groups, two different groups can have the same first CPU.
> 
> This patch uses the group mask to mark all the CPUs that have a
> particular group as its main sched group. The group balance cpu is
> the
> first group CPU that is also in the mask.
> 

This is not your fault, but this code is really hard
to understand.

Your comments tell me what the code does, but not
really why. 

> +++ b/kernel/sched/topology.c
> @@ -477,27 +477,31 @@ enum s_alloc {
>  };
>  
>  /*
> - * Build an iteration mask that can exclude certain CPUs from the
> upwards
> - * domain traversal.
> + * An overlap sched group may not be present in all CPUs that
> compose the
> + * group. So build the mask, marking all the group CPUs where it is
> present.
>   *
>   * Asymmetric node setups can result in situations where the domain
> tree is of
>   * unequal depth, make sure to skip domains that already cover the
> entire
>   * range.
> - *
> - * In that case build_sched_domains() will have terminated the
> iteration early
> - * and our sibling sd spans will be empty. Domains should always
> include the
> - * CPU they're built on, so check that.
>   */

Why are we doing this?

Could the comment explain why things need to be
this way?


> -	for_each_cpu(i, span) {
> +	for_each_cpu(i, sg_span) {
>  		sibling = *per_cpu_ptr(sdd->sd, i);
> -		if (!cpumask_test_cpu(i,
> sched_domain_span(sibling)))
> +
> +		/*
> +		 * Asymmetric node setups: skip domains that are
> already
> +		 * done.
> +		 */
> +		if (!sibling->groups)
> +			continue;
> +

What does this mean?

I would really like it if this code was a little
better documented.  This is not something I can
put on you for most of the code, but I can at least
ask it for the code you are adding :)

The same goes for the rest of this patch.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RFC 0/3] sched/topology: fix sched groups on NUMA machines with mesh topology Lauro Ramos Venancio <lvenanci@redhat.com> - 2017-04-13 16:00 +0200
  [RFC 1/3] sched/topology: Refactor function build_overlap_sched_groups() Lauro Ramos Venancio <lvenanci@redhat.com> - 2017-04-13 16:00 +0200
    Re: [RFC 1/3] sched/topology: Refactor function  build_overlap_sched_groups() Rik van Riel <riel@redhat.com> - 2017-04-13 17:00 +0200
  [RFC 3/3] sched/topology: Different sched groups must not have the same balance cpu Lauro Ramos Venancio <lvenanci@redhat.com> - 2017-04-13 16:00 +0200
    Re: [RFC 3/3] sched/topology: Different sched groups must not have  the same balance cpu Rik van Riel <riel@redhat.com> - 2017-04-13 17:30 +0200
    Re: [RFC 3/3] sched/topology: Different sched groups must not have  the same balance cpu Peter Zijlstra <peterz@infradead.org> - 2017-04-14 18:50 +0200
      Re: [RFC 3/3] sched/topology: Different sched groups must not have  the same balance cpu Lauro Venancio <lvenanci@redhat.com> - 2017-04-17 17:40 +0200
        Re: [RFC 3/3] sched/topology: Different sched groups must not have  the same balance cpu Peter Zijlstra <peterz@infradead.org> - 2017-04-18 14:40 +0200

csiph-web