mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ben Horgan <ben.horgan@arm.com>
To: Jinjie Ruan <ruanjinjie@huawei.com>,
	tony.luck@intel.com, reinette.chatre@intel.com,
	Dave.Martin@arm.com, james.morse@arm.com, babu.moger@amd.com,
	peternewman@google.com, bp@alien8.de, x86@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fs/resctrl: Fix CLOSID propagation when reparenting monitor groups
Date: Mon, 2 Nov 2026 14:46:54 +0000	[thread overview]
Message-ID: <3a637631-c8ec-4e26-a03c-5ac80d16fab3@arm.com> (raw)
In-Reply-To: <20261031101052.1692230-1-ruanjinjie@huawei.com>

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.

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.
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.

[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);
>  }


      reply	other threads:[~2026-11-02 14:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-31 10:10 Jinjie Ruan
2026-11-02 14:46 ` Ben Horgan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3a637631-c8ec-4e26-a03c-5ac80d16fab3@arm.com \
    --to=ben.horgan@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=babu.moger@amd.com \
    --cc=bp@alien8.de \
    --cc=james.morse@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peternewman@google.com \
    --cc=reinette.chatre@intel.com \
    --cc=ruanjinjie@huawei.com \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®