linux-kernel - Re: [PATCH 1/2] sched: Add per_cpu cluster domain info and cpus_share

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 23 Dec 2021 20:09:46 +0530
From:   "Gautham R. Shenoy" <gautham.shenoy@....com>
To:     Vincent Guittot <vincent.guittot@...aro.org>
Cc:     Barry Song <21cnbao@...il.com>,
        Yicong Yang <yangyicong@...ilicon.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        LAK <linux-arm-kernel@...ts.infradead.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ben Segall <bsegall@...gle.com>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        prime.zeng@...wei.com,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        ego@...ux.vnet.ibm.com,
        Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
        Linuxarm <linuxarm@...wei.com>,
        Barry Song <song.bao.hua@...ilicon.com>,
        Guodong Xu <guodong.xu@...aro.org>
Subject: Re: [PATCH 1/2] sched: Add per_cpu cluster domain info and
 cpus_share_cluster API

Hello Vincent, Barry,

On Wed, Dec 22, 2021 at 02:33:37PM +0100, Vincent Guittot wrote:
> On Wed, 22 Dec 2021 at 14:14, Vincent Guittot
[..snip..]

> > > > > @@ -669,6 +671,12 @@ static void update_top_cache_domain(int cpu)
> > > > >       per_cpu(sd_llc_id, cpu) = id;
> > > > >       rcu_assign_pointer(per_cpu(sd_llc_shared, cpu), sds);
> > > > >
> > > > > +     sd = lowest_flag_domain(cpu, SD_CLUSTER);
> > > > > +     if (sd)
> > > > > +             id = cpumask_first(sched_domain_span(sd));
> > > > > +     rcu_assign_pointer(per_cpu(sd_cluster, cpu), sd);
> > > > > +     per_cpu(sd_cluster_id, cpu) = id;
> > > > > +
> >
> > This deserves a large comment to highlight that the new code above is
> > not self contained and  relies on the fact that it is done just after
> > looking for LLC and sd and that id and sd must not be changed in
> > between inorder to ensures that per_cpu(sd_cluster_id, cpu) equals
> > per_cpu(sd_llc_id, cpu) if there is no domain with SD_CLUSTER.
> >
> > and per_cpu(sd_cluster_id, cpu) might not be cluster but llc
> 
> Maybe you should not name this sd_cluster_id at all but
> per_cpu(sd_share_id, cpu) = id;
> 
> Then you have a function named
> 
> +bool cpus_share_resources(int this_cpu, int that_cpu)
> +{
> +     if (this_cpu == that_cpu)
> +             return true;
> +
> +     return per_cpu(sd_share_id, this_cpu) == per_cpu(sd_share_id, that_cpu);
> +}
> +
> which returns true when cpu shares resources which can be LLC or cluster

+1. This would make it more readable than overloading the meaning of cluster itself.

--
Thanks and Regards
gautham.