linux-kernel - Re: [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched

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: <jhjeegt7rdg.mognet@arm.com>
Date:   Fri, 05 Mar 2021 11:48:59 +0000
From:   Valentin Schneider <valentin.schneider@....com>
To:     Barry Song <song.bao.hua@...ilicon.com>,
        vincent.guittot@...aro.org, mingo@...hat.com, peterz@...radead.org,
        juri.lelli@...hat.com, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de
Cc:     linux-kernel@...r.kernel.org, linuxarm@...neuler.org,
        Barry Song <song.bao.hua@...ilicon.com>
Subject: Re: [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched_group

On 05/03/21 11:29, Barry Song wrote:
> mask is built in build_balance_mask() by for_each_cpu(i, sg_span), so
> it must be a subset of sched_group_span(sg).

So we should indeed have

  cpumask_subset(sched_group_span(sg), mask)

but that doesn't imply

  cpumask_first(sched_group_span(sg)) == cpumask_first(mask)

does it? I'm thinking if in your topology of N CPUs, CPUs 0 and N-1 are the
furthest away, you will most likely hit

  !cpumask_equal(sg_pan, sched_domain_span(sibling->child))
                 ^^^^^^                    ^^^^^^^^^^^^^
                 CPUN-1                        CPU0

which should be the case on your Kunpeng920 system.

> Though cpumask_first_and
> doesn't lead to a wrong result of balance cpu, it is pointless to do
> cpumask_and again.
>
> Signed-off-by: Barry Song <song.bao.hua@...ilicon.com>
> ---
>  kernel/sched/topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 12f8058..45f3db2 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -934,7 +934,7 @@ static void init_overlap_sched_group(struct sched_domain *sd,
>  	int cpu;
>  
>  	build_balance_mask(sd, sg, mask);
> -	cpu = cpumask_first_and(sched_group_span(sg), mask);
> +	cpu = cpumask_first(mask);
>  
>  	sg->sgc = *per_cpu_ptr(sdd->sgc, cpu);
>  	if (atomic_inc_return(&sg->sgc->ref) == 1)
> -- 
> 1.8.3.1