linux-kernel - Re: [PATCH v2 4/6] sched: Move sched domain name out of CONFIG_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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <89e35d89-f764-4de4-8250-6923a2fea721@amd.com>
Date: Tue, 24 Dec 2024 09:50:57 +0530
From: "Sapkal, Swapnil" <swapnil.sapkal@....com>
To: Shrikanth Hegde <sshegde@...ux.ibm.com>
CC: <dietmar.eggemann@....com>, <rostedt@...dmis.org>, <bsegall@...gle.com>,
	<mgorman@...e.de>, <vschneid@...hat.com>, <iamjoonsoo.kim@....com>,
	<qyousef@...alina.io>, <alexs@...nel.org>, <lukasz.luba@....com>,
	<gautham.shenoy@....com>, <kprateek.nayak@....com>, <ravi.bangoria@....com>,
	<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
	<peterz@...radead.org>, <mingo@...hat.com>, <juri.lelli@...hat.com>,
	<vincent.guittot@...aro.org>, <corbet@....net>
Subject: Re: [PATCH v2 4/6] sched: Move sched domain name out of
 CONFIG_SCHED_DEBUG

Hello Shrikanth,

Thank you for reviewing the series.

On 12/21/2024 12:29 AM, Shrikanth Hegde wrote:
> 
> 
> On 12/20/24 12:02, Swapnil Sapkal wrote:
>> /proc/schedstat file shows cpu and sched domain level scheduler
>> statistics. It does not show domain name instead shows domain level.
>> It will be very useful for tools like `perf sched stats`[1] to
>> aggragate domain level stats if domain names are shown in /proc/ 
>> schedstat.
>> But sched domain name is guarded by CONFIG_SCHED_DEBUG. As per the
>> discussion[2], move sched domain name out of CONFIG_SCHED_DEBUG.
>>
>> [1] https://lore.kernel.org/lkml/20241122084452.1064968-1- 
>> swapnil.sapkal@....com/
>> [2] https://lore.kernel.org/lkml/ 
>> fcefeb4d-3acb-462d-9c9b-3df8d927e522@....com/
>>
>> Suggested-by: Gautham R. Shenoy <gautham.shenoy@....com>
>> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@....com>
> 
> Similarly i think patch 4 and 5 can be merged into one. and similar 
> comment about updating the stats doc file.
> 

I will merge patch 4 and 5 into one.

>> ---
>>   include/linux/sched/topology.h | 8 --------
>>   kernel/sched/topology.c        | 4 ----
>>   2 files changed, 12 deletions(-)
>>
>> diff --git a/include/linux/sched/topology.h b/include/linux/sched/ 
>> topology.h
>> index 76a662e1ec24..7f3dbafe1817 100644
>> --- a/include/linux/sched/topology.h
>> +++ b/include/linux/sched/topology.h
>> @@ -143,9 +143,7 @@ struct sched_domain {
>>       unsigned int ttwu_move_affine;
>>       unsigned int ttwu_move_balance;
>>   #endif
>> -#ifdef CONFIG_SCHED_DEBUG
>>       char *name;
>> -#endif
>>       union {
>>           void *private;        /* used during construction */
>>           struct rcu_head rcu;    /* used during destruction */
>> @@ -201,18 +199,12 @@ struct sched_domain_topology_level {
>>       int            flags;
>>       int            numa_level;
>>       struct sd_data      data;
>> -#ifdef CONFIG_SCHED_DEBUG
>>       char                *name;
>> -#endif
>>   };
>>   extern void __init set_sched_topology(struct 
>> sched_domain_topology_level *tl);
>> -#ifdef CONFIG_SCHED_DEBUG
>>   # define SD_INIT_NAME(type)        .name = #type
>> -#else
>> -# define SD_INIT_NAME(type)
>> -#endif
>>   #else /* CONFIG_SMP */
>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
>> index 9c405f0e7b26..da33ec9e94ab 100644
>> --- a/kernel/sched/topology.c
>> +++ b/kernel/sched/topology.c
>> @@ -1635,9 +1635,7 @@ sd_init(struct sched_domain_topology_level *tl,
>>           .max_newidle_lb_cost    = 0,
>>           .last_decay_max_lb_cost    = jiffies,
>>           .child            = child,
>> -#ifdef CONFIG_SCHED_DEBUG
>>           .name            = tl->name,
>> -#endif
>>       };
>>       sd_span = sched_domain_span(sd);
>> @@ -2338,10 +2336,8 @@ static struct sched_domain 
>> *build_sched_domain(struct sched_domain_topology_leve
>>           if (!cpumask_subset(sched_domain_span(child),
>>                       sched_domain_span(sd))) {
>>               pr_err("BUG: arch topology borken\n");
>> -#ifdef CONFIG_SCHED_DEBUG
>>               pr_err("     the %s domain not a subset of the %s 
>> domain\n",
>>                       child->name, sd->name);
>> -#endif
>>               /* Fixup, ensure @sd has at least @child CPUs. */
>>               cpumask_or(sched_domain_span(sd),
>>                      sched_domain_span(sd),
> 
--
Thanks and Regards,
Swapnil