Re: [PATCH v2] fs/resctrl: Disallow monitor group rename when the RMID changes

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

Hi Reinette,

On 09/11/2026 17:40, Reinette Chatre wrote:
> Hi Ben,
> 
> On 11/9/26 3:03 AM, Ben Horgan wrote:
>> On 09/11/2026 10:52, Ben Horgan wrote:
>>> On 05/11/2026 12:35, Jinjie Ruan wrote:
>>>> From: Peter Newman <peternewman@google.com>
>>>>
>>>> A monitor group can be renamed into the mon_groups directory of another
>>>> control group to move its tasks to that control group's allocation
>>>> while keeping their monitoring data. Where the RMID is only meaningful
>>>> together with the CLOSID, as on arm64 MPAM where the RMID is a PMG that
>>>> extends the PARTID, this move does not work. The rename succeeds, but:
>>>>
>>>> - The group's monitoring data is reset. Its counters are identified by
>>>>   the CLOSID and RMID together, so after the move the group reads the
>>>>   counters of the new CLOSID instead of the ones its tasks used.
>>>>
>>>> - The tasks are not moved. mongrp_reparent() sets the new parent and
>>>>   CLOSID of the group before rdt_move_group_tasks() looks for the tasks
>>>>   to update, and the search then matches on the new CLOSID, which none
>>>>   of the tasks carries yet. The tasks keep the old CLOSID and the old
>>>>   control group's allocation, and the group does not count them.
>>>>
>>>> The move has never worked on these systems, so rejecting it does not
>>>> take away behavior that an existing user can rely on.
>>>>
>>>> Reject the move with -EOPNOTSUPP when it changes the RMID index of the
>>>> group, and document the restriction in resctrl.rst. Where the RMID does
>>>> not depend on the CLOSID, such as on x86, the move keeps the same index
>>>> and is still allowed.
>>>>
>>>> Fixes: 3e9b35823aab ("arm_mpam: resctrl: Add rmid index helpers")
>>>> Signed-off-by: Peter Newman <peternewman@google.com>
>>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>>> Cc: <stable@vger.kernel.org> # 7.1.x
>>>> Link: https://lore.kernel.org/lkml/20241205153845.394714-1-peternewman@google.com/
>>>> Link: https://lore.kernel.org/lkml/20261031101052.1692230-1-ruanjinjie@huawei.com/
>>>> ---
>>>> v2:
>>>> - Drop the Suggested-by tag.
>>>> - Describe the systems by whether the RMID depends on the CLOSID rather
>>>>   than by MPAM, in both the changelog and resctrl.rst.
>>>> - Explain that moving a monitor group is already broken on these systems
>>>>   because the tasks are not moved, so no existing user relies on it.
>>>> - v1: https://lore.kernel.org/all/20261104101913.586595-1-ruanjinjie@huawei.com/
>>>> ---
>>>>  Documentation/filesystems/resctrl.rst |  7 ++++---
>>>>  fs/resctrl/rdtgroup.c                 | 13 +++++++++++++
>>>>  2 files changed, 17 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
>>>> index b52795e03303..9ae34aace0ff 100644
>>>> --- a/Documentation/filesystems/resctrl.rst
>>>> +++ b/Documentation/filesystems/resctrl.rst
>>>> @@ -552,9 +552,10 @@ will automatically remove all MON groups below it.
>>>>  Moving MON group directories to a new parent CTRL_MON group is supported
>>>>  for the purpose of changing the resource allocations of a MON group
>>>>  without impacting its monitoring data or assigned tasks. This operation
>>>> -is not allowed for MON groups which monitor CPUs. No other move
>>>> -operation is currently allowed other than simply renaming a CTRL_MON or
>>>> -MON group.
>>>> +is not allowed for MON groups which monitor CPUs, and fails with
>>>> +-EOPNOTSUPP on systems where the RMID depends on the CLOSID. No other
>>>> +move operation is currently allowed other than simply renaming a
>>>> +CTRL_MON or MON group.
>>>
>>> A user may not readily know whether RMID depends on CLOSID. To document
>>> this properly we need something specific we can point the user too.
>>> Perhaps this can be a file in the info directory specifying whether the
>>> move is possible or not, info/moveable_mon_group, as we discussed in v1.
>>> I suggest a precursor patch adding this file, followed by this patch
>>> which can then refer to the file.
>>
>> On the other hand, that does add an extra dependency for any backporting
>> of this fix.
> Does this fix really depend on a new user interface? 
> 
> I understand this is helpful to user space and this can be added, but I do
> not see how this fix requires it.

Not strictly, no. Up to a point the documentation does but we could go
back to the documentation in v1 which mentions MPAM for the fix and
then, as a new feature, add the new interface on top of that. The
documentation can then be updated to mention the new interface only at
the time the new interface is added.

Thanks,

Ben

> 
> Reinette
> 
>