Re: [PATCH -next 4/6] cpuset: move update_domain_attr_tree to cpuset_v1.c

[Waiman Long]

On 12/17/25 7:44 PM, Chen Ridong wrote:
> On 2025/12/18 1:09, Waiman Long wrote:
> > On 12/17/25 3:49 AM, Chen Ridong wrote:
> > > From: Chen Ridong <chenridong@xxxxxxxxxx>
> > >
> > > Since relax_domain_level is only applicable to v1, move
> > > update_domain_attr_tree() to cpuset-v1.c, which solely updates
> > > relax_domain_level,
> > >
> > > Additionally, relax_domain_level is now initialized in cpuset1_inited.
> > > Accordingly, the initialization of relax_domain_level in top_cpuset is
> > > removed. The unnecessary remote_partition initialization in top_cpuset
> > > is also cleaned up.
> > >
> > > As a result, relax_domain_level can be defined in cpuset only when
> > > CONFIG_CPUSETS_V1=y.
> > >
> > > Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
> > > ---
> > >    kernel/cgroup/cpuset-internal.h | 11 ++++++++---
> > >    kernel/cgroup/cpuset-v1.c       | 28 ++++++++++++++++++++++++++++
> > >    kernel/cgroup/cpuset.c          | 31 -------------------------------
> > >    3 files changed, 36 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
> > > index a32517da8231..677053ffb913 100644
> > > --- a/kernel/cgroup/cpuset-internal.h
> > > +++ b/kernel/cgroup/cpuset-internal.h
> > > @@ -150,9 +150,6 @@ struct cpuset {
> > >         */
> > >        int attach_in_progress;
> > >    -    /* for custom sched domain */
> > > -    int relax_domain_level;
> > > -
> > >        /* partition root state */
> > >        int partition_root_state;
> > >    @@ -182,6 +179,9 @@ struct cpuset {
> > >      #ifdef CONFIG_CPUSETS_V1
> > >        struct fmeter fmeter;        /* memory_pressure filter */
> > > +
> > > +    /* for custom sched domain */
> > > +    int relax_domain_level;
> > >    #endif
> > >    };
> > >    @@ -296,6 +296,8 @@ void cpuset1_hotplug_update_tasks(struct cpuset *cs,
> > >    int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial);
> > >    void cpuset1_init(struct cpuset *cs);
> > >    void cpuset1_online_css(struct cgroup_subsys_state *css);
> > > +void update_domain_attr_tree(struct sched_domain_attr *dattr,
> > > +                    struct cpuset *root_cs);
> > >    #else
> > >    static inline void cpuset1_update_task_spread_flags(struct cpuset *cs,
> > >                        struct task_struct *tsk) {}
> > > @@ -307,6 +309,9 @@ static inline int cpuset1_validate_change(struct cpuset *cur,
> > >                    struct cpuset *trial) { return 0; }
> > >    static inline void cpuset1_init(struct cpuset *cs) {}
> > >    static inline void cpuset1_online_css(struct cgroup_subsys_state *css) {}
> > > +static inline void update_domain_attr_tree(struct sched_domain_attr *dattr,
> > > +                    struct cpuset *root_cs) {}
> > > +
> > >    #endif /* CONFIG_CPUSETS_V1 */
> > >      #endif /* __CPUSET_INTERNAL_H */
> > > diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
> > > index 574df740f21a..95de6f2a4cc5 100644
> > > --- a/kernel/cgroup/cpuset-v1.c
> > > +++ b/kernel/cgroup/cpuset-v1.c
> > > @@ -502,6 +502,7 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
> > >    void cpuset1_init(struct cpuset *cs)
> > >    {
> > >        fmeter_init(&cs->fmeter);
> > > +    cs->relax_domain_level = -1;
> > >    }
> > >      void cpuset1_online_css(struct cgroup_subsys_state *css)
> > > @@ -552,6 +553,33 @@ void cpuset1_online_css(struct cgroup_subsys_state *css)
> > >        cpuset_callback_unlock_irq();
> > >    }
> > >    +static void
> > > +update_domain_attr(struct sched_domain_attr *dattr, struct cpuset *c)
> > > +{
> > > +    if (dattr->relax_domain_level < c->relax_domain_level)
> > > +        dattr->relax_domain_level = c->relax_domain_level;
> > > +}
> > > +
> > > +void update_domain_attr_tree(struct sched_domain_attr *dattr,
> > > +                    struct cpuset *root_cs)
> > > +{
> > > +    struct cpuset *cp;
> > > +    struct cgroup_subsys_state *pos_css;
> > > +
> > > +    rcu_read_lock();
> > > +    cpuset_for_each_descendant_pre(cp, pos_css, root_cs) {
> > > +        /* skip the whole subtree if @cp doesn't have any CPU */
> > > +        if (cpumask_empty(cp->cpus_allowed)) {
> > > +            pos_css = css_rightmost_descendant(pos_css);
> > > +            continue;
> > > +        }
> > > +
> > > +        if (is_sched_load_balance(cp))
> > > +            update_domain_attr(dattr, cp);
> > > +    }
> > > +    rcu_read_unlock();
> > > +}
> > > +
> > >    /*
> > >     * for the common functions, 'private' gives the type of file
> > >     */
> > > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> > > index e836a1f2b951..88ca8b40e01a 100644
> > > --- a/kernel/cgroup/cpuset.c
> > > +++ b/kernel/cgroup/cpuset.c
> > > @@ -215,8 +215,6 @@ static struct cpuset top_cpuset = {
> > >        .flags = BIT(CS_CPU_EXCLUSIVE) |
> > >             BIT(CS_MEM_EXCLUSIVE) | BIT(CS_SCHED_LOAD_BALANCE),
> > >        .partition_root_state = PRS_ROOT,
> > > -    .relax_domain_level = -1,
> > As the cpuset1_init() function will not be called for top_cpuset, you should not remove the
> > initialization of relax_domain_level. Instead, put it inside a "ifdef CONFIG_CPUSETS_V1 block.
> In patch 3/6, I've made cpuset_init call cpuset1_init to initialize top_cpuset.fmeter. Thus, I think
> we could remove the relax_domain_level initialization here.

I missed that. You are right. Remove the initialization here should be 
all right.

Cheers,
Longman

> > > -    .remote_partition = false,
> > Yes, this is not really needed and can be removed.
> >
> > Cheers,
> > Longman