Re: [PATCH v2 1/2] cgroup/cpuset: reset DL migration state on can_attach() failure

[Chen Ridong]

On 2026/5/7 22:31, Waiman Long wrote:
> On 5/7/26 6:33 AM, Guopeng Zhang wrote:
>> cpuset_can_attach() accumulates temporary SCHED_DEADLINE migration
>> state in the destination cpuset while walking the taskset.
>>
>> If a later task_can_attach() or security_task_setscheduler() check
>> fails, cgroup_migrate_execute() treats cpuset as the failing subsystem
>> and does not call cpuset_cancel_attach() for it. The partially
>> accumulated state is then left behind and can be consumed by a later
>> attach, corrupting cpuset DL task accounting and pending DL bandwidth
>> accounting.
>>
>> Reset the pending DL migration state before returning from those
>> per-task failure paths.
>>
>> Fixes: 2ef269ef1ac0 ("cgroup/cpuset: Free DL BW in case can_attach() fails")
>> Signed-off-by: Guopeng Zhang <zhangguopeng@xxxxxxxxxx>
>> ---
>>   kernel/cgroup/cpuset.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index e3a081a07c6d..ae41736399a1 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -3029,12 +3029,12 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
>>       cgroup_taskset_for_each(task, css, tset) {
>>           ret = task_can_attach(task);
>>           if (ret)
>> -            goto out_unlock;
>> +            goto out_reset_dl_data;
>>             if (setsched_check) {
>>               ret = security_task_setscheduler(task);
>>               if (ret)
>> -                goto out_unlock;
>> +                goto out_reset_dl_data;
>>           }
>>             if (dl_task(task)) {
>> @@ -3070,6 +3070,10 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
>>        * changes which zero cpus/mems_allowed.
>>        */
>>       cs->attach_in_progress++;
>> +    goto out_unlock;
>> +
>> +out_reset_dl_data:
>> +    reset_migrate_dl_data(cs);
>>   out_unlock:
>>       mutex_unlock(&cpuset_mutex);
>>       return ret;
> 
> I would prefer the likely success path be a straight line instead of doing a
> goto. IOW, move out_reset_dl_data below return. Other than that, this patch
> looks good to me.
> 

I've read the code and found several places that call reset_migrate_dl_data(cs).

I think it would be better to call reset_migrate_dl_data(cs) only when we
encounter an error, for example:

```
static int cpuset_can_attach(struct cgroup_taskset *tset)
{
...
out_unlock:
	if (ret)
		reset_migrate_dl_data(cs);
	mutex_unlock(&cpuset_mutex);
	return ret;
}
```
After that, no other places would need to call reset_migrate_dl_data(cs), right?

-- 
Best regards,
Ridong