Re: [PATCH v7 7/9] cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside cpuset_attach_task()

[Waiman Long]

On 6/21/26 10:48 PM, Ridong Chen wrote:
> On 6/21/2026 11:28 AM, Waiman Long wrote:
> > The cpuset_attach_task() was introduced in commit 42a11bf5c543
> > ("cgroup/cpuset: Make cpuset_fork() handle CLONE_INTO_CGROUP properly")
> > to enable the CLONE_INTO_CGROUP flag of clone(2) to behave more like
> > moving a task from one cpuset into another one. That commits didn't
> > move the mpol_rebind_mm() and cpuset_migrate_mm() calls for group leader
> > into cpuset_attach_task().
> >
> > When the CLONE_INTO_CGROUP flag is used without CLONE_THREAD, the new
> > task is its own group leader. So it is still not equivalent to moving
> > task between cpusets in this case. Make CLONE_INTO_CGROUP behaves
> > more close to cpuset_attach() by moving the mpol_rebind_mm() and
> > cpuset_migrate_mm() calls inside cpuset_attach_task(). As a result,
> > the following static variables will have to be updated in cpuset_fork().
> >   - cpuset_attach_old_cs
> >   - attach_cpus_updated
> >   - attach_mems_updated
> >   - queue_task_work
> >
> > Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
> > ---
> >   kernel/cgroup/cpuset.c | 105 ++++++++++++++++++++++++-----------------
> >   1 file changed, 62 insertions(+), 43 deletions(-)
> >
> > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> > index 0375dae26d0b..511afb077e2d 100644
> > --- a/kernel/cgroup/cpuset.c
> > +++ b/kernel/cgroup/cpuset.c
> > @@ -2981,8 +2981,13 @@ static int update_prstate(struct cpuset *cs, 
> > int new_prs)
> >   /*
> >    * cpuset_can_attach() and cpuset_attach() specific internal data
> >    * Protected by cpuset_mutex
> > + *
> > + * The attach_cpus_updated/attach_mems_updated flags are set in either
> > + * cpuset_attach() or cpuset_fork() and used in cpuset_attach_task().
> >    */
> >   static struct cpuset *cpuset_attach_old_cs;
> > +static bool attach_cpus_updated;
> > +static bool attach_mems_updated;
> >     /*
> >    * Check to see if a cpuset can accept a new task
> > @@ -3157,9 +3162,12 @@ static void cpuset_cancel_attach(struct 
> > cgroup_taskset *tset)
> >    */
> >   static cpumask_var_t cpus_attach;
> >   static nodemask_t cpuset_attach_nodemask_to;
> > +static bool queue_task_work;
>
> There are more and more of these standalone state variables now, and 
> it's getting harder to maintain. Could we group them into a struct and 
> manage them together rather than keep adding globals?
>
> Just like:
>
> ```
> struct cpuset_attach_ctx {
>     struct cpuset     *old_cs;
>     struct llist_head src_cs, dst_cs;
>     bool              cpus_updated, mems_updated, queue_work;
>     nodemask_t        nodemask_to;
> };
> ```

Yes, that makes sense. Will incorporate that in the next version.

Cheers,
Longman