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

[Guopeng Zhang]

在 2026/5/8 10:26, Waiman Long 写道:
> 
> On 5/7/26 10:14 PM, Chen Ridong wrote:
>>
>> 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?
>>
> Yes, that should work too.
> 
Thanks for the review.

Yes, I will update cpuset_can_attach() to use the common ret-based
cleanup in out_unlock.

Thanks,
Guopeng
> Cheers,
> Longman