Re: [PATCH-next v2 1/6] cgroup/cpuset: Consolidate isolated_cpus_can_update() into prstate_housekeeping_conflict()

[Waiman Long <longman@redhat.com>]

On 10/14/26 2:58 AM, Guopeng Zhang wrote:
>
> 在 2026/10/14 02:59, Waiman Long 写道:
>> On 10/11/26 1:49 AM, Guopeng Zhang wrote:
>>> Hi，Longman,
>>> 在 2026/10/11 06:19, Waiman Long 写道:
>>>> The isolated_cpus_can_update() and prstate_housekeeping_conflict() are
>>>> checking different aspects of upcoming cpumask changes that may conflict
>>>> with the current setting of the housekeeping cpumasks. There are places
>>>> where both are called together. There are also places where only one
>>>> of them is called. That inconsistency can contribute to missing check
>>>> where invalid cpumask changes may be allowed to move forward.
>>>>
>>>> Fix that by consolidating isolated_cpus_can_update() into
>>>> prstate_housekeeping_conflict() and call prstate_housekeeping_conflict()
>>>> in all the places where either one of them or both are called. The
>>>> exception is the validate_partition() function where the
>>>> prstate_housekeeping_conflict() call is removed. It is because
>>>> validate_partition() is called only from partition_cpus_change()
>>>> where prstate_housekeeping_conflict() will be called from either
>>>> remote_cpus_update() or update_parent_effective_cpumask() with
>>>> partcmd_update if not for partition invalidatation or disablement.
>>>>
>>>> Now prstate_housekeeping_conflict() will be called in the following
>>>> locations:
>>>>    - when a partition is enabled in remote_partition_enable() or in
>>>>      update_parent_effective_cpumask() with partcmd_enable*.
>>>>    - when a cpumask is updated in remote_cpus_update() or in
>>>>      update_parent_effective_cpumask() with partcmd_update.
>>>>    - when a partition state changes from root to isolated or vice versa
>>>>      in update_prstate().
>>>>
>>>> Fixes: 4a74e418881f ("cgroup/cpuset: Check partition conflict with housekeeping setup")
>>>> Fixes: 103b08709e8a ("cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping")
>>>> Signed-off-by: Waiman Long <longman@redhat.com>
>>>> ---
>>>>    kernel/cgroup/cpuset.c | 123 +++++++++++++++++------------------------
>>>>    1 file changed, 52 insertions(+), 71 deletions(-)
>>>>
>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>> index 0ebba2d646c3..f3cebb277a68 100644
>>>> --- a/kernel/cgroup/cpuset.c
>>>> +++ b/kernel/cgroup/cpuset.c
>>>> @@ -1353,65 +1353,62 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
>>>>    }
>>>>      /*
>>>> - * isolated_cpus_can_update - check for isolated & nohz_full conflicts
>>>> - * @add_cpus: cpu mask for cpus that are going to be isolated
>>>> - * @del_cpus: cpu mask for cpus that are no longer isolated, can be NULL
>>>> - * Return: false if there is conflict, true otherwise
>>>> - *
>>>> - * If nohz_full is enabled and we have isolated CPUs, their combination must
>>>> - * still leave housekeeping CPUs.
>>>> + * prstate_housekeeping_conflict - check for partition & housekeeping conflicts
>>>> + * @new_prs: new partition root state to be checked
>>>> + * @parent_prs: Parent partition root state
>>>> + * @add_cpus: additional CPUs to be added to current cpuset
>>>> + * @del_cpus: CPUs to be removed from current cpuset, can be NULL
>>>> + * Return: true if there is conflict, false otherwise
>>>>     *
>>>> - * TBD: Should consider merging this function into
>>>> - *      prstate_housekeeping_conflict().
>>>> + * There are two different housekeeping conflicts to be checked:
>>>> + * 1) If new_prs is PRS_ROOT, none of the @add_cpus can be a boot-time isolated
>>>> + *    CPU. IOW, the whole @add_cpus must be a subset of HK_TYPE_DOMAIN_BOOT.
>>>> + * 2) If nohz_full is enabled and we have isolated CPUs, their combination must
>>>> + *    still leave housekeeping CPUs. This check is only needed if new_prs
>>>> + *    differs from parent_prs and one of them is PRS_ISOLATED.
>>>>     */
>>>> -static bool isolated_cpus_can_update(struct cpumask *add_cpus,
>>>> -                     struct cpumask *del_cpus)
>>>> +static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
>>>> +                      struct cpumask *add_cpus,
>>>> +                      struct cpumask *del_cpus)
>>>>    {
>>>>        cpumask_var_t full_hk_cpus;
>>>> -    int res = true;
>>>> +    int res;
>>>>    -    if (!housekeeping_enabled(HK_TYPE_KERNEL_NOISE))
>>>> +    if (housekeeping_enabled(HK_TYPE_DOMAIN_BOOT) && (new_prs == PRS_ROOT) &&
>>>> +        !cpumask_subset(add_cpus, housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT)))
>>>>            return true;
>>>>    -    if (del_cpus && cpumask_weight_and(del_cpus,
>>>> -            housekeeping_cpumask(HK_TYPE_KERNEL_NOISE)))
>>>> -        return true;
>>>> +    if (!housekeeping_enabled(HK_TYPE_KERNEL_NOISE) ||
>>>> +        (new_prs == parent_prs) ||
>>>> +        (!del_cpus && (new_prs != PRS_ISOLATED)) ||
>>>> +        ((new_prs != PRS_ISOLATED) && (parent_prs != PRS_ISOLATED)))
>>>> +        return false;
>>>>    
>>> When a child partition switches from root to isolated under an isolated
>>> parent, this returns false even though the child's CPUs were not isolated
>>> before the switch.
>>>
>>> On a fresh boot with CPUs 0-15 online and nohz_full=2-15:
>>>
>>>       # cd /sys/fs/cgroup
>>>       # echo +cpuset > cgroup.subtree_control
>>>       # mkdir A B
>>>       # echo 1-2 > A/cpuset.cpus
>>>       # echo isolated > A/cpuset.cpus.partition
>>>       # echo +cpuset > A/cgroup.subtree_control
>>>       # mkdir A/C
>>>       # echo 1 > A/C/cpuset.cpus
>>>       # echo root > A/C/cpuset.cpus.partition
>>>       # echo 0 > B/cpuset.cpus
>>>       # echo isolated > B/cpuset.cpus.partition
>>>       # cat cpuset.cpus.isolated
>>>       0,2
>>>       # echo isolated > A/C/cpuset.cpus.partition
>>>       # cat A/C/cpuset.cpus.partition
>>>       isolated
>>>       # cat cpuset.cpus.isolated
>>>       0-2
>>>
>>> CPU 1 was the last remaining housekeeping CPU: it was in neither the
>>> nohz_full mask nor the cpuset.cpus.isolated mask. The final write isolates
>>> CPU 1 as well, leaving no housekeeping CPU.
>>>
>>> Should this check also take the child's previous partition state into
>>> account, rather than just comparing its new state with the parent's state?
>> This is actually a problem that is hard to fix without introducing a lot of complexity. After spending a day or so trying to find a possible fix from 2 different perspectives, I realize that either one will introduce quite a lot of complexity into the existing cpuset partition code. When looking at it again from a higher level, I think we may have allowed too much freedom to create a complicated nested partition hierarchy. If we simply don't allow the creation of root partition under an isolated partition parent, this problem will not exist. It is indeed a new limitation imposed on the existing cpuset partition functionality, but it is a simple solution and I don't believe there is a real use case in the creation of a root partition under isolated partition. Please correct me if my use case assumption is wrong.
>>
> Thanks for taking the time to look into this.
> Your suggestion to disallow root partitions under isolated parents gives us a different perspective on some of the issues we've encountered in our product.
> As part of our transition from cgroup v1 to v2, we've been testing various cpuset configurations allowed by the current rules.
> Some of these tests have uncovered corner cases, including the one reported here.
> So far, we haven't received any customer reports of a use case that requires a root partition under an isolated parent.

Thanks for the quick reply. Creation of an isolated partition is usually 
a leaf or terminal node that has latency sensitive application running. 
Even creating a child isolated partition underneath it will not be 
common. By imposing this new limitation, we won't need to spend effort 
to address corner cases that are highly unlikely to be used at all. So I 
am going to impose this new restriction on the next version.

Cheers,
Longman