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

[Waiman Long <llong@redhat.com>]

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@huawei.com>
>>>
>>> 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@huawei.com>
>>> ---
>>>    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
>>