Re: [PATCH v2 4/6] sched: Move sched domain name out of CONFIG_SCHED_DEBUG

["Sapkal, Swapnil" <swapnil.sapkal@amd.com>]

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@amd.com/
>> [2] https://lore.kernel.org/lkml/ 
>> fcefeb4d-3acb-462d-9c9b-3df8d927e522@amd.com/
>>
>> Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
>> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.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