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

[Chen Ridong]

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@xxxxxxxxxx>
>>>> ---
>>>>    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.

-- 
Best regards,
Ridong