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

[Waiman Long <longman@redhat.com>]

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@redhat.com>
>> ---
>>   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