Re: [PATCH -next] cpuset: Remove unnecessary checks in rebuild_sched_domains_locked

[Waiman Long <llong@redhat.com>]

On 11/25/25 10:17 PM, Chen Ridong wrote:
>
> On 2025/11/26 10:33, Waiman Long wrote:
>> On 11/25/25 8:01 PM, Chen Ridong wrote:
>>> On 2025/11/26 2:16, Waiman Long wrote:
>>>>> active CPUs, preventing partition_sched_domains from being invoked with
>>>>> offline CPUs.
>>>>>
>>>>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>>>>> ---
>>>>>     kernel/cgroup/cpuset.c | 29 ++++++-----------------------
>>>>>     1 file changed, 6 insertions(+), 23 deletions(-)
>>>>>
>>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>>> index daf813386260..1ac58e3f26b4 100644
>>>>> --- a/kernel/cgroup/cpuset.c
>>>>> +++ b/kernel/cgroup/cpuset.c
>>>>> @@ -1084,11 +1084,10 @@ void dl_rebuild_rd_accounting(void)
>>>>>      */
>>>>>     void rebuild_sched_domains_locked(void)
>>>>>     {
>>>>> -    struct cgroup_subsys_state *pos_css;
>>>>>         struct sched_domain_attr *attr;
>>>>>         cpumask_var_t *doms;
>>>>> -    struct cpuset *cs;
>>>>>         int ndoms;
>>>>> +    int i;
>>>>>           lockdep_assert_cpus_held();
>>>>>         lockdep_assert_held(&cpuset_mutex);
>>>> In fact, the following code and the comments above in rebuild_sched_domains_locked() are also no
>>>> longer relevant. So you may remove them as well.
>>>>
>>>>           if (!top_cpuset.nr_subparts_cpus &&
>>>>               !cpumask_equal(top_cpuset.effective_cpus, cpu_active_mask))
>>>>                   return;
>>>>
>>> Thank you for reminding me.
>>>
>>> I initially retained this code because I believed it was still required for cgroup v1, as I recalled
>>> that synchronous operation is exclusive to cgroup v2.
>>>
>>> However, upon re-examining the code, I confirm it can be safely removed. For cgroup v1,
>>> rebuild_sched_domains_locked is called synchronously, and only the migration task (handled by
>>> cpuset_migrate_tasks_workfn) operates asynchronously. Consequently, cpuset_hotplug_workfn is
>>> guaranteed to complete before the hotplug workflow finishes.
>> Yes, v1 still have a task migration part that is done asynchronously because of the lock ordering
>> issue. Even if this code has to be left because of v1, you should still update the comment to
>> reflect that. Please try to keep the comment updated to help others to have a better understanding
>> of what the code is doing.
>>
>> Thanks,
>> Longman
>>
> Hi Longman,
>
> Just to confirm (in case I misunderstood): I believe it is safe to remove the check on
> top_cpuset.effective_cpus (for both cgroup v1 and v2). I will proceed to remove both the
> corresponding code and its associated comment(not update the comment).
>
>            if (!top_cpuset.nr_subparts_cpus &&
>                !cpumask_equal(top_cpuset.effective_cpus, cpu_active_mask))
>                    return;
>
> Additionally, I should add a comment to clarify the rationale for introducing the
> WARN_ON_ONCE(!cpumask_subset(doms[i], cpu_active_mask)) warning.
>
> Does this approach look good to you? Please let me know if I’ve missed anything or if further
> adjustments are needed.
>
Yes, that is good for me. I was just talking about a hypothetical 
situation, not that you have to update the comment.

Cheers,
Longman