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

[Chen Ridong <chenridong@huaweicloud.com>]

On 2025/11/27 3:47, Waiman Long wrote:
> On 11/26/25 4:11 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Commit 406100f3da08 ("cpuset: fix race between hotplug work and later CPU
>> offline") added a check for empty effective_cpus in partitions for cgroup
>> v2. However, this check did not account for remote partitions, which were
>> introduced later.
>>
>> After commit 2125c0034c5d ("cgroup/cpuset: Make cpuset hotplug processing
>> synchronous"), cpuset hotplug handling is now synchronous. This eliminates
>> the race condition with subsequent CPU offline operations that the original
>> check aimed to fix.
>>
>> Instead of extending the check to support remote partitions, this patch
>> removes all the redundant effective_cpus check. Additionally, it adds a
>> check and warning to verify that all generated sched domains consist of
>> active CPUs, preventing partition_sched_domains from being invoked with
>> offline CPUs.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 50 +++++++++++++-----------------------------
>>   1 file changed, 15 insertions(+), 35 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 6e6eb09b8db6..fea577b4016a 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1103,53 +1103,33 @@ 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);
>>       force_sd_rebuild = false;
>>   -    /*
>> -     * If we have raced with CPU hotplug, return early to avoid
>> -     * passing doms with offlined cpu to partition_sched_domains().
>> -     * Anyways, cpuset_handle_hotplug() will rebuild sched domains.
>> -     *
>> -     * With no CPUs in any subpartitions, top_cpuset's effective CPUs
>> -     * should be the same as the active CPUs, so checking only top_cpuset
>> -     * is enough to detect racing CPU offlines.
>> -     */
>> -    if (cpumask_empty(subpartitions_cpus) &&
>> -        !cpumask_equal(top_cpuset.effective_cpus, cpu_active_mask))
>> -        return;
>> +    /* Generate domain masks and attrs */
>> +    ndoms = generate_sched_domains(&doms, &attr);
>>         /*
>> -     * With subpartition CPUs, however, the effective CPUs of a partition
>> -     * root should be only a subset of the active CPUs.  Since a CPU in any
>> -     * partition root could be offlined, all must be checked.
>> -     */
>> -    if (!cpumask_empty(subpartitions_cpus)) {
>> -        rcu_read_lock();
>> -        cpuset_for_each_descendant_pre(cs, pos_css, &top_cpuset) {
>> -            if (!is_partition_valid(cs)) {
>> -                pos_css = css_rightmost_descendant(pos_css);
>> -                continue;
>> -            }
>> -            if (!cpumask_subset(cs->effective_cpus,
>> -                        cpu_active_mask)) {
>> -                rcu_read_unlock();
>> -                return;
>> -            }
>> -        }
>> -        rcu_read_unlock();
>> +    * cpuset_hotplug_workfn is invoked synchronously now, thus this
>> +    * function should not race with CPU hotplug. And the effective CPUs
>> +    * must not include any offline CPUs. Passing an offline CPU in the
>> +    * doms to partition_sched_domains() will trigger a kernel panic.
>> +    *
>> +    * We perform a final check here: if the doms contains any
>> +    * offline CPUs, a warning is emitted and we return directly to
>> +    * prevent the panic.
>> +    */
>> +    for (i = 0; i < ndoms; ++i) {
>> +        if (WARN_ON_ONCE(!cpumask_subset(doms[i], cpu_active_mask)))
>> +            return;
>>       }
>>   -    /* Generate domain masks and attrs */
>> -    ndoms = generate_sched_domains(&doms, &attr);
>> -
>>       /* Have scheduler rebuild the domains */
>>       partition_sched_domains(ndoms, doms, attr);
>>   }
> Reviewed-by: Waiman Long <longman@redhat.com>
> 
> Thanks!
> 

Thanks.

-- 
Best regards,
Ridong