linux-kernel - Re: [PATCH v3 1/4] sched/fair: add SD

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <36ca372f-5f08-5c1a-e468-4db026051e17@huawei.com>
Date: Tue, 6 Feb 2024 16:21:17 +0800
From: Yicong Yang <yangyicong@...wei.com>
To: Valentin Schneider <vschneid@...hat.com>, <alexs@...nel.org>, Ingo Molnar
	<mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Juri Lelli
	<juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt
	<rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Daniel Bristot de
 Oliveira <bristot@...hat.com>, <linux-kernel@...r.kernel.org>,
	<ricardo.neri-calderon@...ux.intel.com>, <sshegde@...ux.ibm.com>
CC: <yangyicong@...ilicon.com>
Subject: Re: [PATCH v3 1/4] sched/fair: add SD_CLUSTER in comments

On 2024/2/2 22:27, Valentin Schneider wrote:
> 
> Subject nit: the prefix should be sched/topology
> 
> On 01/02/24 19:54, alexs@...nel.org wrote:
>> From: Alex Shi <alexs@...nel.org>
>>
>> The description of SD_CLUSTER is missing. Add it.
>>
>> Signed-off-by: Alex Shi <alexs@...nel.org>
>> To: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
>> To: Valentin Schneider <vschneid@...hat.com>
>> To: Vincent Guittot <vincent.guittot@...aro.org>
>> To: Juri Lelli <juri.lelli@...hat.com>
>> To: Peter Zijlstra <peterz@...radead.org>
>> To: Ingo Molnar <mingo@...hat.com>
>> ---
>>  kernel/sched/topology.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
>> index 10d1391e7416..8b45f16a1890 100644
>> --- a/kernel/sched/topology.c
>> +++ b/kernel/sched/topology.c
>> @@ -1554,6 +1554,7 @@ static struct cpumask		***sched_domains_numa_masks;
>>   * function:
>>   *
>>   *   SD_SHARE_CPUCAPACITY   - describes SMT topologies
>> + *   SD_CLUSTER             - describes CPU Cluster topologies
> 
> So I know this is the naming we've gone for the "Cluster" naming, but this
> comment isn't really explaining anything.
> 
> include/linux/sched/sd_flags.h has a bit more info already:
>  * Domain members share CPU cluster (LLC tags or L2 cache)
> 

Cluster topology in scheduler should mean CPUs beyond the SMT which are sharing
some cache resources (currently L2 on some Intel platforms or L3 Tag on our platforms)
but not the LLC.

A drawing in c5e22feffdd7 ("topology: Represent clusters of CPUs within a die") has
a good illustration and comment of cpus_share_resources() also illustrate this a bit:

/*
 * Whether CPUs are share cache resources, which means LLC on non-cluster
 * machines and LLC tag or L2 on machines with clusters.
 */
bool cpus_share_resources(int this_cpu, int that_cpu)

> I had to go through a bit of git history to remember what the CLUSTER thing
> was about, how about this:
> 
> * SD_CLUSTER             - describes shared shared caches, cache tags or busses
> * SD_SHARE_PKG_RESOURCES - describes shared LLC cache
> 
> And looking at this it would make sense to:
>   rename SD_CLUSTER into SD_SHARE_PKG_RESOURCES
>   rename SD_SHARE_PKG_RESOURCES into SD_SHARE_LLC
> but that's another topic...
> 
>>   *   SD_SHARE_PKG_RESOURCES - describes shared caches
>>   *   SD_NUMA                - describes NUMA topologies
>>   *
>> --
>> 2.43.0
> 
> 
> .
>