Re: [PATCH v3 4/4] cpuset: generalize the validate_partition() interface

[Ridong Chen <ridong.chen@linux.dev>]

On 11/9/2026 10:52 PM, Waiman Long wrote:
> On 11/7/26 7:01 AM, Ridong Chen wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Refactor validate_partition() to take the parent partition, the new
>> partition root state, the new effective exclusive CPUs and the CPUs to
>> be deleted, instead of a trial cpuset. A NULL parent stands for a
>> remote partition, whose CPUs come from top_cpuset. This makes the
>> interface flexible enough to handle both local and remote partition
>> validation.
>>
>> The no CPU left for tasks check now uses the given parent instead of
>> parent_cs(cs), and top_cpuset for a remote partition, as that is where
>> its CPUs are taken from. The cgroup parent of a remote partition does
>> not give it any CPU.
>>
>> The housekeeping conflict check is folded into validate_partition(). It
>> is skipped when the parent is not a valid partition root, as @cs can't
>> be a valid local partition in that case.
>>
>> Use the new interface for local partition enablement in
>> update_parent_effective_cpumask() and in partition_cpus_change(),
>> replacing the open-coded checks there. The order of the checks is now
>> the same for both paths: empty CPUs, housekeeping conflict, then no CPU
>> left for tasks.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   kernel/cgroup/cpuset.c | 119 ++++++++++++++++++++---------------------
>>   1 file changed, 59 insertions(+), 60 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index fc9de6e16ff5..0ec20412b3ce 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1414,6 +1414,48 @@ static bool prstate_housekeeping_conflict(int new_prs, 
>> int parent_prs,
>>       return res;
>>   }
>> +/**
>> + * validate_partition - Validate a cpuset partition configuration
>> + * @cs: The cpuset to validate
>> + * @parent: Parent partition of @cs, NULL for remote partition
>> + * @new_prs: The proposed new partition root state
>> + * @new_excpus: The new effective exclusive CPUs mask to validate
>> + * @del: exclusive CPUs to be deleted from @cs, can be NULL
>> + *
>> + * A remote partition gets its CPUs from top_cpuset, so top_cpuset is used
>> + * as the parent when @parent is NULL.
>> + *
>> + * The housekeeping check is skipped if @parent is not a valid partition
>> + * root as @cs can't be a valid local partition in that case.
>> + *
>> + * Return: PRS error code (0 if valid, non-zero error code if invalid)
>> + */
>> +static enum prs_errcode validate_partition(struct cpuset *cs,
>> +                       struct cpuset *parent, int new_prs,
>> +                       struct cpumask *new_excpus,
>> +                       struct cpumask *del)
>> +{
>> +    int parent_prs;
>> +
>> +    if (new_prs == PRS_MEMBER)
>> +        return PERR_NONE;
>> +
>> +    if (cpumask_empty(new_excpus))
>> +        return PERR_INVCPUS;
>> +
>> +    if (!parent)
>> +        parent = &top_cpuset;
>> +    parent_prs = parent->partition_root_state;
>> +    if (parent_prs > 0 &&
>> +        prstate_housekeeping_conflict(new_prs, parent_prs, new_excpus, del))
>> +        return PERR_HKEEPING;
>> +
>> +    if (tasks_nocpu_error(parent, cs, new_excpus))
>> +        return PERR_NOCPUS;
>> +
>> +    return PERR_NONE;
>> +}
>> +
>>   /*
>>    * cpuset_update_sd_hk_unlock - Rebuild sched domains, update HK & unlock
>>    *
>> @@ -1934,18 +1976,10 @@ static int update_parent_effective_cpumask(struct 
>> cpuset *cs, int cmd,
>>       if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
>>           new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
>> -        /*
>> -         * Enabling partition root is not allowed if its
>> -         * effective_xcpus is empty.
>> -         */
>> -        if (cpumask_empty(tmp->delmask))
>> -            return PERR_INVCPUS;
>> -
>> -        if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask, 
>> NULL))
>> -            return PERR_HKEEPING;
>> -
>> -        if (nocpu)
>> -            return PERR_NOCPUS;
>> +        part_error = validate_partition(cs, parent, new_prs,
>> +                        tmp->delmask, NULL);
>> +        if (part_error)
>> +            return part_error;
>>           /*
>>            * This function will only be called when all the preliminary
>> @@ -2424,32 +2458,6 @@ static int parse_cpuset_cpulist(const char *buf, struct 
>> cpumask *out_mask)
>>       return 0;
>>   }
>> -/**
>> - * validate_partition - Validate a cpuset partition configuration
>> - * @cs: The cpuset to validate
>> - * @trialcs: The trial cpuset containing proposed configuration changes
>> - *
>> - * If any validation check fails, the appropriate error code is set in the
>> - * cpuset's prs_err field.
>> - *
>> - * Return: PRS error code (0 if valid, non-zero error code if invalid)
>> - */
>> -static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset 
>> *trialcs)
>> -{
>> -    struct cpuset *parent = parent_cs(cs);
>> -
>> -    if (cs_is_member(trialcs))
>> -        return PERR_NONE;
>> -
>> -    if (cpumask_empty(trialcs->effective_xcpus))
>> -        return PERR_INVCPUS;
>> -
>> -    if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
>> -        return PERR_NOCPUS;
>> -
>> -    return PERR_NONE;
>> -}
>> -
>>   /**
>>    * partition_cpus_change - Handle partition state changes due to CPU mask 
>> updates
>>    * @cs: The target cpuset being modified
>> @@ -2463,33 +2471,24 @@ static void partition_cpus_change(struct cpuset *cs, 
>> struct cpuset *trialcs,
>>                     struct tmpmasks *tmp)
>>   {
>>       enum prs_errcode prs_err;
>> -    int parent_prs = is_remote_partition(cs)
>> -               ? PRS_ROOT : parent_cs(cs)->partition_root_state;
>> +    struct cpuset *parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>>       if (cs_is_member(cs))
>>           return;
>> -    prs_err = validate_partition(cs, trialcs);
>> -    if (!prs_err && (parent_prs > 0) &&
>> -        !cpumask_empty(trialcs->effective_xcpus)) {
>> -        int prs = cs->partition_root_state;
>> -
>> -        /*
>> -         * Check for housekeeping conflict by passing the whole
>> -         * effective_xcpus as well as those CPUs that are to be
>> -         * removed from cs as delmask.
>> -         */
>> -        if (prs > 0)
>> -            cpumask_andnot(tmp->delmask, cs->effective_xcpus,
>> -                       trialcs->effective_xcpus);
>> -        else
>> -            cpumask_clear(tmp->delmask);
>> -
>> -        if (prstate_housekeeping_conflict(prs, parent_prs,
>> -                          trialcs->effective_xcpus, tmp->delmask))
>> -            prs_err = PERR_HKEEPING;
>> -    }
>> +    /*
>> +     * Check for housekeeping conflict by passing the whole
>> +     * effective_xcpus as well as those CPUs that are to be
>> +     * removed from cs as delmask.
>> +     */
>> +    if (is_partition_valid(cs))
>> +        cpumask_andnot(tmp->delmask, cs->effective_xcpus,
>> +                   trialcs->effective_xcpus);
>> +    else
>> +        cpumask_clear(tmp->delmask);
>> +    prs_err = validate_partition(cs, parent, trialcs->partition_root_state,
>> +                     trialcs->effective_xcpus, tmp->delmask);
>>       if (prs_err) {
>>           WRITE_ONCE(cs->prs_err, prs_err);
>>           trialcs->prs_err = prs_err;
> 
> Again, this code change looks good to me. Similar to previous patch, it includes 
> both code restructuring and bug fixes.
> 
Hi Longman,

Thanks for the review. Yes, besides the restructuring, it fixes a false
PERR_NOCPUS error for remote partitions. The no CPU left for tasks check
in partition_cpus_change() is done against the cgroup parent, but the
CPUs of a remote partition come from top_cpuset, not from its parent.

It can be reproduced on a 4-CPU system:

	cd /sys/fs/cgroup
	echo +cpuset > cgroup.subtree_control
	mkdir P
	echo 2-3 > P/cpuset.cpus
	echo 2-3 > P/cpuset.cpus.exclusive
	echo +cpuset > P/cgroup.subtree_control
	mkdir P/C
	echo 2-3 > P/C/cpuset.cpus
	echo 2 > P/C/cpuset.cpus.exclusive
	echo root > P/C/cpuset.cpus.partition
	sleep 1000 &
	echo $! > P/cgroup.procs
	echo 2-3 > P/C/cpuset.cpus.exclusive
	cat P/C/cpuset.cpus.partition
	root invalid (Parent is not a partition root)

The effective_cpus of P is 3, a subset of the new exclusive CPUs 2-3 of
C, and P has a task, so the expansion is rejected. However, P is not a
partition root and does not give any CPU to C, and top_cpuset still has
CPUs 0-1 for the task. Enabling C directly with exclusive CPUs 2-3
succeeds, as remote_partition_enable() checks top_cpuset instead. With
this patch, the expansion succeeds too:

	cat P/C/cpuset.cpus.partition
	root

The issue has been there since remote partitions were introduced. I
reproduced it on commit 181c8e091aae as well, so I'd like to describe
it in the commit message and add

	Fixes: 181c8e091aae ("cgroup/cpuset: Introduce remote partition")

in the next version.

Does that look OK to you? Please let me know if you are referring to something else.

-- 
Best regards
Ridong