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

[Ben Horgan <ben.horgan@arm.com>]

Hi Jinjie,

On 03/11/2026 03:34, Jinjie Ruan wrote:
> 
> 
> 在 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?

That makes sense to me. So, yes please.

Thanks,

Ben

> 
>>
>> [1] https://lore.kernel.org/lkml/20241205153845.394714-1-peternewman@google.com/
>>
>> 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@huawei.com>
>>> Cc: stable@vger.kernel.org # 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);
>>>  }
>>
>