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

[Sapkal, Swapnil]

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