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

Swapnil Sapkal posted 6 patches 1 year, 1 month ago
[PATCH v2 4/6] sched: Move sched domain name out of CONFIG_SCHED_DEBUG
Posted by Swapnil Sapkal 1 year, 1 month ago
/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>
---
 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),
-- 
2.43.0
Re: [PATCH v2 4/6] sched: Move sched domain name out of CONFIG_SCHED_DEBUG
Posted by Shrikanth Hegde 1 year, 1 month ago

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.

> ---
>   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),
Re: [PATCH v2 4/6] sched: Move sched domain name out of CONFIG_SCHED_DEBUG
Posted by Sapkal, Swapnil 1 year, 1 month ago
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
[tip: sched/core] sched: Move sched domain name out of CONFIG_SCHED_DEBUG
Posted by tip-bot2 for Swapnil Sapkal 1 year, 1 month ago
The following commit has been merged into the sched/core branch of tip:

Commit-ID:     1c055a0f5d3bafaca5d218bbb3e4e63d6307be45
Gitweb:        https://git.kernel.org/tip/1c055a0f5d3bafaca5d218bbb3e4e63d6307be45
Author:        Swapnil Sapkal <swapnil.sapkal@amd.com>
AuthorDate:    Fri, 20 Dec 2024 06:32:22 
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 20 Dec 2024 15:31:17 +01:00

sched: Move sched domain name out of CONFIG_SCHED_DEBUG

/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>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20241220063224.17767-5-swapnil.sapkal@amd.com
---
 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 76a662e..7f3dbaf 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 9c405f0..da33ec9 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),