Re: [PATCH cgroup/for-next 0/4] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach()

[Waiman Long <longman@redhat.com>]

On 5/14/26 5:46 PM, Tejun Heo wrote:
> Hello,
>
> Quick AI-assisted review pass; passing the points along for human eyes.
Thank for passing the AI review info.
> Patch 4:
>
> - The leader loop comment says "Only v1 supports memory_migrate", but
>    CS_MEMORY_MIGRATE is set unconditionally on v2 cpusets in
>    cpuset_css_alloc(). With v2 controller-disable folding children with
>    differing effective_mems into the parent, picking a single
>    llist_entry(src_cs_head.first, ...) as oldcs passes the wrong source
>    nodemask to cpuset_migrate_mm() for every leader whose actual source
>    differs. Looks like the source needs to be looked up per leader.
I falsely assumes that CS_MEMORY_MIGRATE isn't set for v2. I am going to 
move the cpuset_migrate_mm() call into cpuset_attach_task() as well. To 
enable access to right oldcs information, I have to reluctantly add the 
oldcs pointer to task_struct as that is the simplest way moving forward.
>
> - cs->old_mems_allowed updates are inconsistent across destinations: the
>    mid-loop transition assigns cs->effective_mems (raw) while the tail
>    assignment uses cpuset_attach_nodemask_to (after guarantee_online_mems).
>    The v2 fast-path also updates only the first-task cs, leaving other
>    destinations on dst_cs_head stale.
I will clarify the confusion and fix the v2 fast-path in v2.
> Patch 3:
>
> - Changelog says "the newly cloned task isn't the group leader", but for
>    CLONE_INTO_CGROUP without CLONE_THREAD the new task is its own
>    group_leader, so the new mpol_rebind_mm() block in cpuset_attach_task()
>    does run from cpuset_fork(). Either acknowledge as an incidental
>    improvement or guard the new path.
Will update the patch description.
>
> Patch 1:
>
> - alloc_dl_bw() reads confusingly next to the scheduler's dl_bw_alloc()
>    while doing more (pick cpu, call dl_bw_alloc, record cs->dl_bw_cpu).
>    Something like cpuset_reserve_dl_bw() would be clearer.
Sure, cpuset_reserve_dl_bw() sounds like a good name.
> - The relocated "Mark attach is in progress" comment sits inside a
>    braceless else; either move it above the if (ret) or brace both arms.

Will add the additional braces though that will be removed in a later patch.

Cheers,
Longman

>
> Patch 2 looked clean.
>
> Thanks.
>
> --
> tejun
>