Re: [PATCH v4 1/2] cgroup/cpuset: Keep an active housekeeping CPU in the top sched domain

[Guopeng Zhang <guopeng.zhang@linux.dev>]

在 2026/11/12 11:05, Ridong Chen 写道:
> 
> 
> On 11/11/2026 9:42 PM, Guopeng Zhang wrote:
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> Boot-isolated CPUs remain in top_cpuset.effective_cpus, but are excluded
>> from its scheduling domain. A partition can therefore take all boot
>> domain housekeeping CPUs while leaving effective_cpus nonempty. For
>> example, on a 32-CPU system with all CPUs online and
>> isolcpus=domain,10-31:
>>
>>    echo 0-9 > A/cpuset.cpus
>>    echo root > A/cpuset.cpus.partition
>>    Oops: general protection fault
>>    RIP: 0010:build_perf_domains+...
>>
>> tasks_nocpu_error() also rejects valid CPU replacements because it does
>> not account for CPUs returned by the update. With a fresh cgroup A under
>> the same boot configuration:
>>
>>    echo 2-31 > A/cpuset.cpus
>>    echo isolated > A/cpuset.cpus.partition
>>    echo 0-4 > A/cpuset.cpus
>>    cat A/cpuset.cpus.partition
>>    isolated invalid (Parent unable to distribute cpu downstream)
>>
>> The update should return CPUs 5-31 to top_cpuset, including housekeeping
>> CPUs 5-9, and leave A valid. This replacement failure also occurs without
>> boot isolation and under populated local parents.
>>
>> Extend tasks_nocpu_error() to take an explicit mask of CPUs being
>> returned and use it for local and remote updates. Require an active
>> HK_TYPE_DOMAIN_BOOT CPU to remain in top_cpuset. For other populated
>> parents, check within their current exclusive grant so an ancestor
>> update cannot credit CPUs that the parent no longer owns.
>>
>> CPU hotplug can also empty the top scheduling domain. Pass no returned
>> CPUs during its availability check, and only invalidate a partition to
>> restore the top domain if it can return an active boot domain
>> housekeeping CPU. Still invalidate a populated partition that has no
>> active CPU of its own.
>>
>> Fixes: 9b496a8bbed9 ("cgroup/cpuset: Prevent leakage of isolated CPUs into sched domains")
>> Fixes: 0c7f293efc87 ("cgroup/cpuset: Add cpuset.cpus.exclusive.effective for v2")
>> Co-developed-by: Ridong Chen <chenridong@xiaomi.com>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> ---
>>   kernel/cgroup/cpuset.c | 85 ++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 70 insertions(+), 15 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 3052413e41e7..85f92ae0c6f4 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1181,18 +1181,54 @@ static void update_partition_sd_lb(struct cpuset *cs, int old_prs)
>>   }
>>     /*
>> - * tasks_nocpu_error - Return true if tasks will have no effective_cpus
>> + * tasks_nocpu_error - Return true if tasks or the top domain lack active CPUs
>> + * @parent: parent partition, NULL for a remote partition
>> + * @cs: the partition
>> + * @xcpus: exclusive CPUs that @cs will hold after the update
>> + * @del_cpus: exclusive CPUs to be returned to @parent, can be NULL
>> + *
>> + * @del_cpus may include CPUs that are also in @xcpus and will stay in
>> + * @cs. They are skipped by the @xcpus check, so callers can pass the
>> + * current effective_xcpus of @cs without masking it first.
>> + *
>> + * top_cpuset always has tasks. Boot-isolated CPUs remain in its
>> + * effective_cpus but are excluded from its sched domain, which must
>> + * retain an active boot domain housekeeping CPU.
>>    */
>>   static bool tasks_nocpu_error(struct cpuset *parent, struct cpuset *cs,
>> -                  struct cpumask *xcpus)
>> +                  const struct cpumask *xcpus,
>> +                  const struct cpumask *del_cpus)
>>   {
>> +    const struct cpumask *mask;
>> +    int cpu;
>> +
>> +    /* A populated @cs must have an active CPU in @xcpus. */
>> +    if (!cpumask_intersects(xcpus, cpu_active_mask) &&
>> +        partition_is_populated(cs, NULL))
>> +        return true;
>> +
>>       /*
>> -     * A populated partition (cs or parent) can't have empty effective_cpus
>> +     * A populated @parent must keep an active CPU in @mask, either from
>> +     * its effective_cpus or from @del_cpus, that isn't taken by @cs.
>>        */
>> -    return (cpumask_subset(parent->effective_cpus, xcpus) &&
>> -        partition_is_populated(parent, cs)) ||
>> -           (!cpumask_intersects(xcpus, cpu_active_mask) &&
>> -        partition_is_populated(cs, NULL));
>> +    if (!parent)
>> +        parent = &top_cpuset;
>> +
>> +    /* CPUs outside the parent's current grant cannot be returned. */
>> +    mask = parent->effective_xcpus;
>> +    if (parent == &top_cpuset)
>> +        mask = housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT);
>> +    else if (!partition_is_populated(parent, cs))
>> +        return false;
>> +
> 
> The mask selection looks right to me. HK_TYPE_DOMAIN_BOOT is the right
> one for top_cpuset, since CPUs returned by an isolated child are not in
> HK_TYPE_DOMAIN yet. Limiting a local parent to its effective_xcpus also
> correctly drops stale CPUs in del_cpus after the parent shrinks.
> 
> One corner case: validate_partition() also calls this for an invalid
> cs whose parent is not a valid partition. If the parent is an invalid
> partition without cpuset.cpus.exclusive, reset_partition_data() has
> cleared its effective_xcpus, the loop does nothing, and we return true
> when the parent is populated. The child's prs_err then changes from
> PERR_INVPARENT to PERR_NOCPUS, which reports the wrong reason. The old
> code did not do that. How about:
> 
>       else if (!is_partition_valid(parent) ||
>                !partition_is_populated(parent, cs))
>               return false;
>            
> The invalid parent case is already reported by PERR_INVPARENT/PERR_NOTPART.
> 

Hi Ridong,

Thanks for the review.

I tested your guard and will add it. However, I reproduced the error
change under slightly different conditions from those you described:
the populated invalid parent still had an explicit exclusive mask.
In that case, v4 changes PERR_INVPARENT to PERR_NOCPUS. Your check
preserves the original reason, and also preserves PERR_NOTPART in the
corresponding case where the parent is a member.

In the case you described, with no explicit exclusive mask and the
parent's effective_xcpus already cleared, I get PERR_INVCPUS in both the
base kernel and v4. compute_trialcs_excpus() also produces an empty child
trial mask, so validate_partition() returns PERR_INVCPUS before calling
tasks_nocpu_error().

> Nit: the comment "Return true if tasks or the top domain lack active
> CPUs" doesn't cover local parents. Maybe "Return true if @cs or @parent
> would lack active CPUs; for top_cpuset, only active boot domain
> housekeeping CPUs count".
> 
Sure, I'll update the comment.
> 
>> +    for_each_cpu_and(cpu, mask, cpu_active_mask) {
>> +        if (cpumask_test_cpu(cpu, xcpus))
>> +            continue;
>> +        if (cpumask_test_cpu(cpu, parent->effective_cpus) ||
>> +            (del_cpus && cpumask_test_cpu(cpu, del_cpus)))
>> +            return false;
>> +    }
>> +    return true;
>>   }
>>     static void reset_partition_data(struct cpuset *cs)
>> @@ -1724,7 +1760,7 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
>>        */
>>       compute_excpus(cs, tmp->new_cpus);
>>       if (!cpumask_intersects(tmp->new_cpus, cpu_active_mask) ||
>> -        cpumask_subset(top_cpuset.effective_cpus, tmp->new_cpus))
>> +        tasks_nocpu_error(NULL, cs, tmp->new_cpus, NULL))
>>           return PERR_INVCPUS;
>>       if (cpumask_intersects(tmp->new_cpus, subpartitions_cpus))
>>           return PERR_NOCPUS;
>> @@ -1828,7 +1864,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>>           if (!capable(CAP_SYS_ADMIN))
>>               WRITE_ONCE(cs->prs_err, PERR_ACCESS);
>>           else if (cpumask_intersects(tmp->addmask, subpartitions_cpus) ||
>> -             cpumask_subset(top_cpuset.effective_cpus, tmp->addmask))
>> +             tasks_nocpu_error(NULL, cs, excpus, cs->effective_xcpus))
>>               WRITE_ONCE(cs->prs_err, PERR_NOCPUS);
>>           else if (prstate_housekeeping_conflict(prs, PRS_ROOT,
>>                                  excpus, tmp->delmask))
>> @@ -1961,7 +1997,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>        */
>>       if ((cmd != partcmd_disable) && !newmask) {
>>           compute_excpus(cs, tmp->delmask);
>> -        nocpu = tasks_nocpu_error(parent, cs, tmp->delmask);
>> +        nocpu = tasks_nocpu_error(parent, cs, tmp->delmask,
>> +                      is_partition_valid(cs) ? cs->effective_xcpus : NULL);
>>       }
>>         if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
>> @@ -2497,7 +2534,9 @@ static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *tri
>>       if (cpumask_empty(trialcs->effective_xcpus))
>>           return PERR_INVCPUS;
>>   -    if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
>> +    if (tasks_nocpu_error(is_remote_partition(cs) ? NULL : parent, cs,
>> +                  trialcs->effective_xcpus,
>> +                  is_partition_valid(cs) ? cs->effective_xcpus : NULL))
>>           return PERR_NOCPUS;
>>         return PERR_NONE;
>> @@ -3997,6 +4036,7 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
>>       static nodemask_t new_mems;
>>       bool cpus_updated;
>>       bool mems_updated;
>> +    bool nocpu;
>>       int partcmd = -1;
>>       struct cpuset *parent;
>>   @@ -4008,9 +4048,23 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
>>       if (!tmp || !cs->partition_root_state)
>>           goto update_tasks;
>>   -    if (is_remote_partition(cs) && (cpumask_empty(subpartitions_cpus) ||
>> -                       (cpumask_empty(&new_cpus) &&
>> -                    partition_is_populated(cs, NULL)))) {
>> +    /* Do not count any CPUs as returned during this hotplug check. */
>> +    nocpu = tasks_nocpu_error(is_remote_partition(cs) ? NULL : parent, cs,
>> +                  &new_cpus, NULL);
>> +    if (nocpu && (parent == &top_cpuset || is_remote_partition(cs))) {
>> +        /*
>> +         * Only invalidate a partition for an empty top sched domain if
>> +         * it can return an active boot domain housekeeping CPU. A
>> +         * populated partition with no active CPU must still be invalidated.
>> +         */
>> +        cpumask_and(tmp->addmask, cs->effective_xcpus,
>> +                housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT));
>> +        if (!cpumask_intersects(tmp->addmask, cpu_active_mask))
>> +            nocpu = cpumask_empty(&new_cpus) &&
>> +                partition_is_populated(cs, NULL);
>> +    }
>> +
> 
> This is hard to follow. nocpu is computed first and then partly undone,
> so that only a partition owning an active housekeeping CPU gets
> invalidated.
> 

v3 used separate branches for these cases. Adapting that structure to
the unified helper, would something like the following be easier to
follow?

        if (parent == &top_cpuset || is_remote_partition(cs)) {
                /*
                 * Do not count child partition CPUs as already returned.
                 * Only invalidate a partition for an empty top domain if
                 * it can return an active boot domain housekeeping CPU.
                 * A populated partition with no active CPU of its own
                 * must still be invalidated.
                 */
                cpumask_and(tmp->addmask, cs->effective_xcpus,
                            housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT));
                nocpu = cpumask_intersects(tmp->addmask, cpu_active_mask) &&
                        tasks_nocpu_error(NULL, cs, cs->effective_xcpus,
                                          NULL);
                nocpu |= cpumask_empty(&new_cpus) &&
                         partition_is_populated(cs, NULL);
        } else {
                nocpu = tasks_nocpu_error(parent, cs, &new_cpus, NULL);
        }

> Do we really need this? When hotplug leaves top_cpuset with no active
> CPU, cpuset_handle_hotplug() already clears subpartitions_cpus and
> invalidates all the partitions under it. If we handle an empty top
> sched domain the same way, this block can go away.
> 

The check keeps partitions that cannot return an active HK CPU from
being invalidated just because the top domain is empty. For example,
on a 32-CPU system with isolcpus=domain,10-31, A is an isolated partition
holding CPU 10 and its sibling B is a root partition holding CPUs 0,2-9.
When CPU 1 goes offline, v4 invalidates B and keeps A valid.

As I understand it, if we use the reset approach, it can restore the
top scheduling domain, but it should invalidate both A and B. If we
eventually want the same reset policy for an empty top scheduling
domain, I can make that change and update the existing test expectations.

Thanks,
Guopeng

> Longman, what do you think?
> 
>> +    if (is_remote_partition(cs) && (cpumask_empty(subpartitions_cpus) || nocpu)) {
>>           WRITE_ONCE(cs->prs_err, PERR_HOTPLUG);
>>           remote_partition_disable(cs, tmp);
>>           compute_effective_cpumask(&new_cpus, cs, parent);
>> @@ -4023,10 +4077,11 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
>>        * 2) parent is invalid or doesn't grant any cpus to child
>>        *    partitions.
>>        * 3) subpartitions_cpus is empty.
>> +     * 4) top_cpuset needs an active boot domain housekeeping CPU from cs.
>>        */
>>       if (is_local_partition(cs) &&
>>           (!is_partition_valid(parent) ||
>> -         tasks_nocpu_error(parent, cs, &new_cpus) ||
>> +         nocpu ||
>>            cpumask_empty(subpartitions_cpus)))
>>           partcmd = partcmd_invalidate;
>>       /*
>