Re: [PATCH-next v2 2/5] cgroup/cpuset: Correctly set cpus_updated in cpuset_hotplug_update_tasks()

[Waiman Long <longman@redhat.com>]

On 11/10/26 3:40 AM, Ridong Chen wrote:
>
>
> On 11/10/2026 3:09 AM, Waiman Long wrote:
>> When cpuset_hotplug_update_tasks() is called with the conditions that
>> either remote_partition_disable() or update_parent_effective_cpumask()
>> has to be called, the cs->effective_cpus can be updated such that it
>> matches what is computed in new_cpus. In such case, cpus_updated can
>> be incorrectly set to false leading to stale CPU afinity remained in the
>> tasks not matching the new cpuset value. Fix that by forcing 
>> cpus_updated
>> to true whenever one of these partition helpers is being called.
>>
>> Fixes: 4b842da276a8a ("cpuset: Make CPU hotplug work with partition")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Link: 
>> https://sashiko.dev/#/bug/linux-ab90f7e8-0d60-40ca-83e9-e405075b24d2
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   kernel/cgroup/cpuset.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 6074327c3477..3955d2d574cf 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -4002,7 +4002,7 @@ static void cpuset_hotplug_update_tasks(struct 
>> cpuset *cs, struct tmpmasks *tmp)
>>   {
>>       static cpumask_t new_cpus;
>>       static nodemask_t new_mems;
>> -    bool cpus_updated;
>> +    bool cpus_updated = false;
>>       bool mems_updated;
>>       int partcmd = -1;
>>       struct cpuset *parent;
>> @@ -4015,12 +4015,18 @@ static void 
>> cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
>>       if (!tmp || !cs->partition_root_state)
>>           goto update_tasks;
>>   +    /*
>> +     * cpus_updated should be set to true when 
>> remote_partition_disable()
>> +     * or update_parent_effective_cpumask() is called as 
>> cs->effective_cpus
>> +     * may have been updated.
>> +     */
>>       if (is_remote_partition(cs) && 
>> (cpumask_empty(subpartitions_cpus) ||
>>                          (cpumask_empty(&new_cpus) &&
>>                       partition_is_populated(cs, NULL)))) {
>>           WRITE_ONCE(cs->prs_err, PERR_HOTPLUG);
>>           remote_partition_disable(cs, tmp);
>>           compute_effective_cpumask(&new_cpus, cs, parent);
>> +        cpus_updated = true;
>>       }
>>         /*
>> @@ -4045,6 +4051,7 @@ static void cpuset_hotplug_update_tasks(struct 
>> cpuset *cs, struct tmpmasks *tmp)
>>           partcmd = partcmd_update;
>>         if (partcmd >= 0) {
>> +        cpus_updated = true;
>>           update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
>>           if (is_partition_valid(cs) || (partcmd == 
>> partcmd_invalidate)) {
>>               compute_effective_cpumask(&new_cpus, cs, parent);
>> @@ -4053,7 +4060,7 @@ static void cpuset_hotplug_update_tasks(struct 
>> cpuset *cs, struct tmpmasks *tmp)
>>       }
>>     update_tasks:
>> -    cpus_updated = !cpumask_equal(&new_cpus, cs->effective_cpus);
>> +    cpus_updated |= !cpumask_equal(&new_cpus, cs->effective_cpus);
>>       mems_updated = !nodes_equal(new_mems, cs->effective_mems);
>>       if (!cpus_updated && !mems_updated)
>>           goto unlock;    /* Hotplug doesn't affect this cpuset */
>
> I think the issue only happens in the remote partition case.
> remote_partition_disable() calls reset_partition_data(), which sets
> cs->effective_cpus to what compute_effective_cpumask() will return
> afterwards, so the cpumask_equal() check misses the change while the
> tasks still have the old affinity.
>
> update_parent_effective_cpumask() only updates the effective_cpus of
> the parent (through partition_xcpus_add/del()), and cs is skipped in
> update_sibling_cpumasks(). So cs->effective_cpus isn't changed there and
> the original check still works. If new_cpus is the same as
> cs->effective_cpus in that case, forcing cpus_updated just walks all the
> tasks in cs and calls set_cpus_allowed_ptr() with their current mask.
> Should cpus_updated only be set in the remote partition case?
>
> Also, as the problem comes from remote_partition_disable() in the
> hotplug path, should the Fixes tag be the following instead?
>
> Fixes: 181c8e091aae ("cgroup/cpuset: Introduce remote partition")
>
Yes, you are right. Thanks for the review. Will send out a v3 with the 
suggested changes.

Cheers,
Longman