Re: [PATCH v5 RESEND 6/7] fs/resctrl: Disallow monitor group move when the RMID index changes

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Jinjie,

On 11/10/26 5:39 AM, Jinjie Ruan wrote:
> From: Peter Newman <peternewman@google.com>
> 
> A MON group can be moved into the mon_groups directory of another
> CTRL_MON group to move its tasks to that 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, the move changes the RMID index of the group. The
> move 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 group uses an RMID index that was never allocated to it. When
>   the group is removed, free_rmid() puts an RMID index of the new CLOSID
>   into limbo without checking that the group owns it. If another group
>   of that CLOSID holds the index, the limbo handler tests and frees an
>   RMID that is still in use, while the RMID index allocated to the
>   moved group is never freed.

It may also be that the new RMID index is not used by another group ...
the RMID thus being used while considered free for allocation.

> 
> - 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
>   CTRL_MON group's allocation, and the group does not count them.
> 
> Fixing the task move does not make the move work. The RMID index is the
> hardware ID the counters count for, so a group whose CLOSID changes
> cannot keep its monitoring data, and it would need a new RMID index
> allocated for the new CLOSID, which fails when every RMID index of that
> CLOSID is in use. The documentation promises that a move keeps the
> monitoring data, so the move cannot be supported, as decided in the
> thread at the Link tag.
> 
> Reject the move with -EOPNOTSUPP when it changes the RMID index of the
> group, and document that a move is not allowed when the monitoring data
> or assigned tasks would be impacted.


Below stops being imperative halfway. 

 Where the RMID does not depend on
> the CLOSID, such as on x86, the move keeps the same RMID index and is
> still allowed. Renaming a MON group within the same CTRL_MON group keeps
> the CLOSID and is still allowed everywhere.

How about:
	Leave the move allowed where the RMID does not depend on the CLOSID,
	such as on x86, and leave renaming within the same CTRL_MON group
	allowed everywhere; neither changes the RMID index.

> 
> 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>
> Link: https://lore.kernel.org/lkml/3a637631-c8ec-4e26-a03c-5ac80d16fab3@arm.com/
> ---

| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette