Re: [PATCH-next 2/6] cgroup/cpuset: Properly compute effective xcpus in update_parent_effective_cpumask()

[Waiman Long <longman@redhat.com>]

On 10/23/26 2:45 AM, Ridong Chen wrote:
>
>
> On 10/22/2026 11:16 AM, Waiman Long wrote:
>> Since the introduction of cpuset.cpus.exclusive in commit
>> e2ffe502ba45 ("cgroup/cpuset: Add cpuset.cpus.exclusive for v2"), a
>> compute_effective_exclusive_cpumask() helper which is later renamed to
>> compute_excpus() is added to compute the effective xcpus of a partition
>> root. However update_parent_effective_cpumask() is not updated in all
>> proper places to use the new helper to check if a given cpuset should
>> remain a valid partition root. This can cause a cpuset to incorrectly
>> remain a valid partition root even if no effective xcpus is left.
>>
>> Fix that problem by using compute_excpus() to compute the effective 
>> xcpus
>> into delmask in all cases except partcmd_disable and partcmd_invalidate
>> and use it for tasks_nocpu_error() check as well as checking its
>> emptiness. This enables us to catch invalid partition root more
>> accurately in update_parent_effective_cpumask().
>>
>> Some test case are added to test_cpuset_prs.sh for invalidating a
>> partition root due to its effective xcpus becoming empty as well as
>> making sure that an invalid partition can only be revived when exclusive
>> CPUs are available agin.
>>
>> Fixes: e2ffe502ba45 ("cgroup/cpuset: Add cpuset.cpus.exclusive for v2")
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   kernel/cgroup/cpuset.c                        | 36 +++++++++----------
>>   .../selftests/cgroup/test_cpuset_prs.sh       |  6 ++++
>>   2 files changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 7ead8c3b7974..db6e4663f4be 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1798,7 +1798,6 @@ static int 
>> update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>       int deleting;    /* Deleting cpus from parent's 
>> effective_cpus    */
>>       int old_prs, new_prs;
>>       int part_error = PERR_NONE;    /* Partition error? */
>> -    struct cpumask *xcpus = user_xcpus(cs);
>>       int parent_prs = parent->partition_root_state;
>>       bool nocpu;
>>   @@ -1841,30 +1840,29 @@ static int 
>> update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>       if (!newmask && xcpus_empty(cs))
>>           return PERR_CPUSEMPTY;
>>   -    nocpu = tasks_nocpu_error(parent, cs, xcpus);
>> +    /*
>> +     * Except for partcmd_disable, use delmask to compute the current
>> +     * effective xcpus value and use it for tasks_nocpu_error check.
>> +     */
>> +    if (cmd != partcmd_disable) {
>> +        compute_excpus(cs, tmp->delmask);
>> +        nocpu = tasks_nocpu_error(parent, cs, tmp->delmask);
>> +    }
>>         if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
>> -        /*
>> -         * Need to call compute_excpus() in case
>> -         * exclusive_cpus not set. Sibling conflict should only happen
>> -         * if exclusive_cpus isn't set.
>> -         */
>> -        xcpus = tmp->delmask;
>> -        if (compute_excpus(cs, xcpus))
>> - WARN_ON_ONCE(!cpumask_empty(cs->exclusive_cpus));
>>           new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
>>             /*
>>            * Enabling partition root is not allowed if its
>>            * effective_xcpus is empty.
>>            */
>> -        if (cpumask_empty(xcpus))
>> +        if (cpumask_empty(tmp->delmask))
>>               return PERR_INVCPUS;
>>   -        if (prstate_housekeeping_conflict(new_prs, parent_prs, 
>> xcpus, NULL))
>> +        if (prstate_housekeeping_conflict(new_prs, parent_prs, 
>> tmp->delmask, NULL))
>>               return PERR_HKEEPING;
>>   -        if (tasks_nocpu_error(parent, cs, xcpus))
>> +        if (nocpu)
>>               return PERR_NOCPUS;
>
> update_parent_effective_cpumask is only used for the local partition. 
> So I'd suggest adding a local_partition_validate helper, which can be 
> used for both partition enable and update. For now this logic is 
> spread out, and many corner cases would easily be missed.

I am thinking about breaking up update_parent_effective_cpumask() to be 
more like what we are doing with remote partition to share as much 
common part as possible. This cleanup work will be in a later patch 
series. Since this is a fix patch, I don't want to make too many changes 
to make it hard to backport to earlier releases.

Cheers,
Longman