mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] fs/resctrl: Disallow monitor group rename when the RMID changes
@ 2026-11-04 10:19 Jinjie Ruan
  2026-11-04 14:36 ` Ben Horgan
  0 siblings, 1 reply; 3+ messages in thread
From: Jinjie Ruan @ 2026-11-04 10:19 UTC (permalink / raw)
  To: tony.luck, reinette.chatre, Dave.Martin, james.morse, babu.moger,
	corbet, skhan, rdunlap, jic23, tan.shaopeng, gshan, zengheng4,
	x86, linux-kernel, linux-doc, ben.horgan
  Cc: ruanjinjie

From: Peter Newman <peternewman@google.com>

Renaming a monitor group to move it below a different control group
changes the CLOSID of its tasks. Where the RMID only has meaning
together with the CLOSID, the RMID index changes as well: on arm64 MPAM
the RMID is a PMG that extends the PARTID, so the group's counters
belong to the old PARTID and cannot follow the move. The monitor data
would be reset and would then report the new PARTID's traffic.

Check that the RMID index is unchanged before moving the group and
reject the rename with -EOPNOTSUPP otherwise. Only MPAM systems are
affected; architectures where the RMID is independent of the CLOSID,
such as x86, keep the same index and still allow the move. Document
the restriction in resctrl.rst.

Fixes: 3e9b35823aab ("arm_mpam: resctrl: Add rmid index helpers")
Suggested-by: Ben Horgan <ben.horgan@arm.com>
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/
---
 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..5f21ab20030a 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, such as
+arm64 MPAM. No other move operation is currently allowed other than
+simply renaming a CTRL_MON or MON group.
 
 All groups contain the following files:
 
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 9290a346386e..358f2dc8301d 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -4405,6 +4405,19 @@ static int rdtgroup_rename(struct kernfs_node *kn,
 		goto out;
 	}
 
+	/*
+	 * Moving the group changes the CLOSID. Where the RMID is only
+	 * meaningful together with the CLOSID, the RMID index changes and the
+	 * monitor state cannot follow the move.
+	 */
+	if (rdtgrp->mon.parent != new_prdtgrp && resctrl_arch_mon_capable() &&
+	    resctrl_arch_rmid_idx_encode(rdtgrp->closid, rdtgrp->mon.rmid) !=
+	    resctrl_arch_rmid_idx_encode(new_prdtgrp->closid, rdtgrp->mon.rmid)) {
+		rdt_last_cmd_puts("Changing parent control group not supported\n");
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
 	/*
 	 * If the MON group is monitoring CPUs, the CPUs must be assigned to the
 	 * current parent CTRL_MON group and therefore cannot be assigned to
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] fs/resctrl: Disallow monitor group rename when the RMID changes
  2026-11-04 10:19 [PATCH] fs/resctrl: Disallow monitor group rename when the RMID changes Jinjie Ruan
@ 2026-11-04 14:36 ` Ben Horgan
  2026-11-04 16:18   ` Ben Horgan
  0 siblings, 1 reply; 3+ messages in thread
From: Ben Horgan @ 2026-11-04 14:36 UTC (permalink / raw)
  To: Jinjie Ruan, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, babu.moger, corbet, skhan, rdunlap, jic23,
	tan.shaopeng, gshan, zengheng4, x86, linux-kernel, linux-doc

Hi Jinjie,

On 04/11/2026 10:19, Jinjie Ruan wrote:
> From: Peter Newman <peternewman@google.com>
> 
> Renaming a monitor group to move it below a different control group
> changes the CLOSID of its tasks. Where the RMID only has meaning
> together with the CLOSID, the RMID index changes as well: on arm64 MPAM
> the RMID is a PMG that extends the PARTID, so the group's counters
> belong to the old PARTID and cannot follow the move. The monitor data
> would be reset and would then report the new PARTID's traffic.
> 
> Check that the RMID index is unchanged before moving the group and
> reject the rename with -EOPNOTSUPP otherwise. Only MPAM systems are
> affected; architectures where the RMID is independent of the CLOSID,
> such as x86, keep the same index and still allow the move. Document
> the restriction in resctrl.rst.
> 
> Fixes: 3e9b35823aab ("arm_mpam: resctrl: Add rmid index helpers")
> Suggested-by: Ben Horgan <ben.horgan@arm.com>

Perhaps stretching the meaning of a 'Suggested-by' tag as I only suggested the repost of Peter's patch.

It is probably worth pointing out that the moving of monitor groups is existingly broken other than
just the resetting of mbm_total_bytes counters and so the current behaviour isn't something that an
existing user could be relying on. As you already point out in [1]

[1] https://lore.kernel.org/lkml/20261031101052.1692230-1-ruanjinjie@huawei.com/

Thanks,

Ben

> 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/
> ---
>  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..5f21ab20030a 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, such as
> +arm64 MPAM. No other move operation is currently allowed other than
> +simply renaming a CTRL_MON or MON group.
>  
>  All groups contain the following files:
>  
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 9290a346386e..358f2dc8301d 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -4405,6 +4405,19 @@ static int rdtgroup_rename(struct kernfs_node *kn,
>  		goto out;
>  	}
>  
> +	/*
> +	 * Moving the group changes the CLOSID. Where the RMID is only
> +	 * meaningful together with the CLOSID, the RMID index changes and the
> +	 * monitor state cannot follow the move.
> +	 */
> +	if (rdtgrp->mon.parent != new_prdtgrp && resctrl_arch_mon_capable() &&
> +	    resctrl_arch_rmid_idx_encode(rdtgrp->closid, rdtgrp->mon.rmid) !=
> +	    resctrl_arch_rmid_idx_encode(new_prdtgrp->closid, rdtgrp->mon.rmid)) {
> +		rdt_last_cmd_puts("Changing parent control group not supported\n");
> +		ret = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
>  	/*
>  	 * If the MON group is monitoring CPUs, the CPUs must be assigned to the
>  	 * current parent CTRL_MON group and therefore cannot be assigned to


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] fs/resctrl: Disallow monitor group rename when the RMID changes
  2026-11-04 14:36 ` Ben Horgan
@ 2026-11-04 16:18   ` Ben Horgan
  0 siblings, 0 replies; 3+ messages in thread
From: Ben Horgan @ 2026-11-04 16:18 UTC (permalink / raw)
  To: Jinjie Ruan, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, babu.moger, corbet, skhan, rdunlap, jic23,
	tan.shaopeng, gshan, zengheng4, x86, linux-kernel, linux-doc

Hi Jinjie,

On 04/11/2026 14:36, Ben Horgan wrote:
> Hi Jinjie,
> 
> On 04/11/2026 10:19, Jinjie Ruan wrote:
>> From: Peter Newman <peternewman@google.com>
>>
>> Renaming a monitor group to move it below a different control group
>> changes the CLOSID of its tasks. Where the RMID only has meaning
>> together with the CLOSID, the RMID index changes as well: on arm64 MPAM
>> the RMID is a PMG that extends the PARTID, so the group's counters
>> belong to the old PARTID and cannot follow the move. The monitor data
>> would be reset and would then report the new PARTID's traffic.
>>
>> Check that the RMID index is unchanged before moving the group and
>> reject the rename with -EOPNOTSUPP otherwise. Only MPAM systems are
>> affected; architectures where the RMID is independent of the CLOSID,
>> such as x86, keep the same index and still allow the move. Document
>> the restriction in resctrl.rst.
>>
>> Fixes: 3e9b35823aab ("arm_mpam: resctrl: Add rmid index helpers")
>> Suggested-by: Ben Horgan <ben.horgan@arm.com>
> 
> Perhaps stretching the meaning of a 'Suggested-by' tag as I only suggested the repost of Peter's patch.
> 
> It is probably worth pointing out that the moving of monitor groups is existingly broken other than
> just the resetting of mbm_total_bytes counters and so the current behaviour isn't something that an
> existing user could be relying on. As you already point out in [1]
> 
> [1] https://lore.kernel.org/lkml/20261031101052.1692230-1-ruanjinjie@huawei.com/
> 
> Thanks,
> 
> Ben
> 
>> 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/
>> ---
>>  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..5f21ab20030a 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, such as
>> +arm64 MPAM. 

In MPAMv2 there are likely to be systems where RMID doesn't depend on CLOSID and so best not to rely
on MPAM vs not for this distinction.

No other move operation is currently allowed other than
>> +simply renaming a CTRL_MON or MON group.
>>  

It was previously discussed about having a file that reports whether or not RMIDs have a fixed
CLOSID and whether or not there should be a separate file to report whether MON group moving is
allowed or not [1]. Do you think a info/moveable_mon_group file or such like would be useful?

I have the below patch that adds a new file for the RMID scoping but we should coordinate so we
don't end up with multiple changes to the same part of the documentation. Your MPAM patch [2] is
also related here as that that corrects 'num_rmids'.

[1] https://lore.kernel.org/linux-arm-kernel/d1110701-a2d5-4cc1-84de-4ed53ef02368@arm.com/
[2] https://lore.kernel.org/linux-arm-kernel/20261104101622.563280-1-ruanjinjie@huawei.com/

Thanks,

Ben

fs/resctrl: Add file info/mon_id_includes_control_id

    The maximum number of CTRL_MON + MON groups that can be created is visible to
    the user as the minimum of the values read from info/L3_MON/num_rmids and
    info/PERF_PKG_MON/num_rmids. Similarly, the maximum number of CTRL_MON groups is
    the minimum of the values in info/<resource>/num_closids.

    However, the number of MON that can be created as children of a given CTRL_MON
    group is not directly knowable by the user. On x86 systems, there is no separate
    limit and so in the extreme there can be a single CTRL_MON groups and num_rmids
    - 1 MON groups. On MPAM systems however, the number of MON groups per CTRL_MON
    groups is limited by the number of PMG bits and is equal
    to (num_rmids/num_closids) - 1.

    So, that the user can easily calculate the maximum number of monitoring groups
    per control add a new read only file, info/mon_id_includes_control_id.  If this
    new file reads 1, the maximum number of MON groups under a CTRL_MON group
    is (num_rmids/num_closids) - 1 and if it reads 0 the maximum is num_rmids -
    1. On a monitor only platform it will read 0 allowing the file to be easily
    interpreted when there are no num_closids files.

    Signed-off-by: Ben Horgan <ben.horgan@arm.com>
    ---

    The num_rmids in MPAM monitor only platforms is currently wrong as it takes into
    account PARTIDs that aren't usable.

diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
index b52795e03303..ca586b4e502f 100644
--- a/Documentation/filesystems/resctrl.rst
+++ b/Documentation/filesystems/resctrl.rst
@@ -510,8 +510,16 @@ with the following files:
 "mon_features":
                Lists the telemetry monitoring events that are enabled on this system.

-The upper bound for how many "CTRL_MON" + "MON" can be created
-is the smaller of the L3_MON and PERF_PKG_MON "num_rmids" values.
+The upper bound for how many "CTRL_MON" + "MON" groups can be created is the
+smaller of the L3_MON and PERF_PKG_MON "num_rmids" values.  The
+"mon_id_includes_control_id" file in the top level of the "info" directory helps
+determine how many "MON" groups can be created in any given "mon_groups"
+directory. If it reads 1, then the maximum number of "MON" groups that can be
+created in a "mon_groups" directory is equal to the amount created if the
+maximum number of "CTRL_MON" groups are created and "MON" groups evenly
+distributed between them. If it reads 0, then the number of "MON" groups than
+can be created is only limited by the global restriction on the number of
+"CTRL_MON" + "MON" groups.

 Finally, in the top level of the "info" directory there is a file
 named "last_cmd_status". This is reset with every "command" issued
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 57ab090072c2..90712a1337d0 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -1200,6 +1200,17 @@ static int rdt_num_rmids_show(struct kernfs_open_file *of,
        return 0;
 }

+static int rdt_mon_id_includes_control_id_show(struct kernfs_open_file *of,
+                                              struct seq_file *seq, void *v)
+{
+       bool dependent = IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID) &&
+                        resctrl_arch_alloc_capable();
+
+       seq_printf(seq, "%u\n", dependent);
+
+       return 0;
+}
+
 static int rdt_mon_features_show(struct kernfs_open_file *of,
                                 struct seq_file *seq, void *v)
 {
@@ -2009,6 +2020,13 @@ static struct rftype res_common_files[] = {
                .seq_show       = rdt_num_rmids_show,
                .fflags         = RFTYPE_MON_INFO,
        },
+       {
+               .name           = "mon_id_includes_control_id",
+               .mode           = 0444,
+               .kf_ops         = &rdtgroup_kf_single_ops,
+               .seq_show       = rdt_mon_id_includes_control_id_show,
+               .fflags         = RFTYPE_TOP_INFO,
+       },
        {
                .name           = "cbm_mask",
                .mode           = 0444,



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-04 16:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-04 10:19 [PATCH] fs/resctrl: Disallow monitor group rename when the RMID changes Jinjie Ruan
2026-11-04 14:36 ` Ben Horgan
2026-11-04 16:18   ` Ben Horgan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®