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

[Waiman Long]

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