linux-kernel - Re: [PATCH 4/4] sched/topology: the group balance cpu must be a cpu where the group is installed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170424130326.nfbaujvcdjca22tl@hirez.programming.kicks-ass.net>
Date:   Mon, 24 Apr 2017 15:03:26 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Lauro Ramos Venancio <lvenanci@...hat.com>
Cc:     lwang@...hat.com, riel@...hat.com, Mike Galbraith <efault@....de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] sched/topology: the group balance cpu must be a cpu
 where the group is installed

On Thu, Apr 20, 2017 at 04:51:43PM -0300, Lauro Ramos Venancio wrote:

> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index e77c93a..694e799 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c

> @@ -505,7 +507,11 @@ static void build_group_mask(struct sched_domain *sd, struct sched_group *sg)
>  
>  	for_each_cpu(i, sg_span) {
>  		sibling = *per_cpu_ptr(sdd->sd, i);
> -		if (!cpumask_test_cpu(i, sched_domain_span(sibling)))
> +
> +		if (!sibling->groups)
> +			continue;

How can this happen?

> +
> +		if (!cpumask_equal(sg_span, sched_group_cpus(sibling->groups)))
>  			continue;
>  
>  		cpumask_set_cpu(i, sched_group_mask(sg));


> @@ -1482,6 +1502,14 @@ struct sched_domain *build_sched_domain(struct sched_domain_topology_level *tl,
>  		}
>  	}
>  
> +	/* Init overlap groups */
> +	for_each_cpu(i, cpu_map) {
> +		for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
> +			if (sd->flags & SD_OVERLAP)
> +				init_overlap_sched_groups(sd);
> +		}
> +	}

Why does this have to be a whole new loop? This is because in
build_group_mask() we could encounter @sibling that were not constructed
yet?

So this is the primary fix?

> +
>  	/* Calculate CPU capacity for physical packages and nodes */
>  	for (i = nr_cpumask_bits-1; i >= 0; i--) {
>  		if (!cpumask_test_cpu(i, cpu_map))


Also, would it not make sense to re-order patch 2 to come after this,
such that we _do_ have the group_mask available and don't have to jump
through hoops in order to link up the sgc? Afaict we don't actually use
the sgc until the above (reverse) loop computing the CPU capacities.