Re: [PATCH] fs/resctrl: Fix CLOSID propagation when reparenting monitor groups

[Jinjie Ruan]

在 2026/11/2 22:46, Ben Horgan 写道:
> Hi Jinjie,
> 
> On 31/10/2026 10:10, Jinjie Ruan wrote:
>> Renaming a monitor group to move it below a different control group
>> changes the CLOSID of every task in that monitor group. mongrp_reparent()
>> updates rdtgrp->mon.parent and rdtgrp->closid before calling
>> rdt_move_group_tasks() to propagate the new CLOSID to the tasks.
>>
>> rdt_move_group_tasks() uses its first argument to identify the tasks to
>> move. For a monitor group this is is_rmid_match(), which on MPAM compares
>> both the PARTID and the PMG of the task with the parent control group's
>> CLOSID and the monitor group's RMID. After rdtgrp->mon.parent and
>> rdtgrp->closid have been updated, the match uses the new CLOSID, so no
>> task in the monitor group matches and none is moved. The tasks keep the
>> old PARTID and continue to use the old control group's allocation.
> 
> Ah, that's pretty broken.

Hi Ben,

You are right! I found several issues about this with mpam.

> 
> I was previously under the impression that moving monitor groups on MPAM systems was not permitted
> as the monitor bandwidth counters are required to be always running and so would glitch when moved.

Got it. Then this operation should indeed be forbidden. Unfortunately,
in practice it hasn't been forbidden yet.

> Peter Newman posted a patch in 2024 which added this restriction [1] but it didn't reach upstream.
> As, the moving of monitor groups is already broken for MPAM systems I think it would be ok to forbid
> them.

Should I pick it up and rebased on v7.4-rc1 and send out it again?

> 
> [1] https://lore.kernel.org/lkml/20241205153845.394714-1-peternewman@xxxxxxxxxx/
> 
> Thanks,
> 
> Ben
> 
>>
>> x86 matches on the RMID alone, so it is unaffected. Now only arm64 mpam
>> affected.
>>
>> Save the monitor group state before it is modified and use the saved state
>> as the source of the task move. The updated group remains the destination,
>> so matching tasks are moved to the new CLOSID and RMID.
>>
>> Fixes: 8da2b938eb7e ("x86/resctrl: Implement rename op for mon groups")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx # 7.1.x
>> ---
>>  fs/resctrl/rdtgroup.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index 57ab090072c2..33b23b59a6de 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -4322,6 +4322,7 @@ static void mongrp_reparent(struct rdtgroup *rdtgrp,
>>  			    cpumask_var_t cpus)
>>  {
>>  	struct rdtgroup *prdtgrp = rdtgrp->mon.parent;
>> +	struct rdtgroup old_rdtgrp = *rdtgrp;
>>  
>>  	WARN_ON(rdtgrp->type != RDTMON_GROUP);
>>  	WARN_ON(new_prdtgrp->type != RDTCTRL_GROUP);
>> @@ -4338,7 +4339,7 @@ static void mongrp_reparent(struct rdtgroup *rdtgrp,
>>  	rdtgrp->closid = new_prdtgrp->closid;
>>  
>>  	/* Propagate updated closid to all tasks in this group. */
>> -	rdt_move_group_tasks(rdtgrp, rdtgrp, cpus);
>> +	rdt_move_group_tasks(&old_rdtgrp, rdtgrp, cpus);
>>  
>>  	update_closid_rmid(cpus, NULL);
>>  }
> 

-- 
Best regards,
Jinjie