Re: [PATCH 1/2] cgroup/cpuset: Avoid unnecessary cpus & mems update in cpuset_hotplug_update_tasks()

[Waiman Long <longman@redhat.com>]

On 6/22/26 9:14 PM, Ridong Chen wrote:
>
>
> On 6/23/2026 6:45 AM, Waiman Long wrote:
>> As reported by sashiko [1], cpuset_hotplug_update_tasks() may perform
>> unnecessary task iteration and updating of tasks' CPU and node masks
>> when mems_allowed and/or cpus_allowed are not set in cpuset v2. It is
>> due to the fact that the temporary new_cpus and new_mems masks do not
>> inherit parent's effective_cpus/mems when they are empty which is the
>> expected behavior for cpuset v2 since commit 4ec22e9c5a90 ("cpuset:
>> Enable cpuset controller in default hierarchy").
>>
>> Fix that and avoid unnecessay work by adding the empty mask checks and
>> inheriting the parent's versions if empty.
>>
>> [1] 
>> https://sashiko.dev/#/patchset/20260621032816.1806773-1-longman%40redhat.com
>>
>> Fixes: 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default 
>> hierarchy")
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   kernel/cgroup/cpuset.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index aff86acea701..bc0207fd6e57 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -3925,6 +3925,14 @@ static void cpuset_hotplug_update_tasks(struct 
>> cpuset *cs, struct tmpmasks *tmp)
>>       compute_effective_cpumask(&new_cpus, cs, parent);
>>       nodes_and(new_mems, cs->mems_allowed, parent->effective_mems);
>>   +    if (is_in_v2_mode()) {
>> +        /* Inherit parent's effective_cpus/mems if empty */
>> +        if (cpumask_empty(&new_cpus))
>> +            cpumask_copy(&new_cpus, parent->effective_cpus);
>> +        if (nodes_empty(new_mems))
>> +            new_mems = parent->effective_mems;
>> +    }
>> +
>>       if (!tmp || !cs->partition_root_state)
>>           goto update_tasks;
>
> I noticed that compute_effective_cpumask(...) is called in several 
> places, so I think the logic should be consolidated into that function.
>
> ```
> static void compute_effective_cpumask(struct cpumask *new_cpus,
>                       struct cpuset *cs, struct cpuset *parent)
> {
>     cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
>     if (cpumask_empty(&new_cpus) && is_in_v2_mode())
>         cpumask_copy(&new_cpus, parent->effective_cpus);
> }
>
> ```
>
> Similarly, for new_mems, should we introduce a dedicated helper like 
> compute_effective_nodemask? The same fallback logic is needed in 
> update_nodemasks_hier:
>
>
> ```
> static void update_nodemasks_hier(struct cpuset *cs, nodemask_t 
> *new_mems)
> {
> ...
>         bool has_mems = nodes_and(*new_mems, cp->mems_allowed, 
> parent->effective_mems);
>
>         /*
>          * If it becomes empty, inherit the effective mask of the
>          * parent, which is guaranteed to have some MEMs.
>          */
>         if (is_in_v2_mode() && !has_mems)
>             *new_mems = parent->effective_mems;
> ...
> ```
>
Yes, that makes sense. Will adopt this approach in the next version.

Cheers,
Longman