Re: [PATCH v10 RESEND 06/15] fs/resctrl: Unmount resctrl with the CDP state it was mounted with

[Ben Horgan]

Hi Jinjie,

On 20/10/2026 14:42, Jinjie Ruan wrote:
> From: Zeng Heng <zengheng4@xxxxxxxxxx>
> 
> resctrl disables CDP before the control and monitor groups are torn down
> on unmount. The architecture translates between CLOSID and PARTID using
> the CDP state, so the RMIDs freed by rmdir_all_sub() and
> resctrl_fs_teardown() are parsed with the wrong mapping. On arm64 the
> RMID limbo handler can still be running, and a CLOSID left in the limbo
> list is re-interpreted with the CDP state of the next mount. Enabling
> CDP doubles the CLOSID on arm64, which can push the stale value out of
> range and take an MPAM error interrupt.
> 
> Disable CDP after the groups have been torn down, so every CLOSID freed
> on unmount is parsed with the CDP state it was allocated under. Return
> the RMIDs still in limbo to the free list and stop the worker, so
> nothing carries over to the next mount.
> 
> Cc: Tony Luck <tony.luck@xxxxxxxxx>
> Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Cc: Dave Martin <Dave.Martin@xxxxxxx>
> Cc: James Morse <james.morse@xxxxxxx>
> Cc: Babu Moger <babu.moger@xxxxxxx>
> Signed-off-by: Zeng Heng <zengheng4@xxxxxxxxxx>
> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
> ---
>  fs/resctrl/rdtgroup.c | 46 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 5dcbb0a964e8..40821cf07649 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -3145,6 +3145,41 @@ static void resctrl_fs_teardown(void)
>  	rdtgroup_destroy_root();
>  }
>  
> +/*
> + * The RMID limbo handler keeps running after unmount. Any CLOSID still in
> + * limbo would be re-interpreted with the CDP state of the next mount. MPAM
> + * doubles CLOSID when CDP is enabled, which can push the stale value out of
> + * range. Force the limbo list back to the free list before the CLOSID state
> + * is torn down.

If the limbo list is being emptied then the limbo handler is effectively not being run. This is a
reduction of functionality of resctrl. We either need a way to keep it running or rebuild the list
at mount. There was some discussion at
https://lore.kernel.org/all/20260413085405.1166412-2-zengheng4@xxxxxxxxxx/

This is a fs/resctrl patch that doesn't directly relate with the subject of the series and so
unlikely to get much attention from the resctrl maintainers. If you respin I'd suggest posting this
separately. I note that Tony Luck has a patch in the same area, [1].

[1] https://lore.kernel.org/lkml/20260928221509.68002-14-tony.luck@xxxxxxxxx/

Thanks,

Ben

> + */
> +static void rdt_flush_limbo(void)
> +{
> +	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
> +	struct rdt_l3_mon_domain *d;
> +
> +	if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
> +		return;
> +
> +	if (!resctrl_is_mon_event_enabled(QOS_L3_OCCUP_EVENT_ID))
> +		return;
> +
> +	list_for_each_entry(d, &r->mon_domains, hdr.list) {
> +		if (!has_busy_rmid(d))
> +			continue;
> +
> +		/*
> +		 * The limbo handler acquires rdtgroup_mutex. Release it
> +		 * here to allow the handler to complete, then reacquire
> +		 * to maintain lock ordering for __check_limbo().
> +		 * This matches the pattern used in resctrl_offline_cpu().
> +		 */
> +		mutex_unlock(&rdtgroup_mutex);
> +		cancel_delayed_work_sync(&d->cqm_limbo);
> +		mutex_lock(&rdtgroup_mutex);
> +		__check_limbo(d, true);
> +	}
> +}
> +
>  static void resctrl_unmount(void)
>  {
>  	struct rdt_resource *r;
> @@ -3152,13 +3187,20 @@ static void resctrl_unmount(void)
>  	cpus_read_lock();
>  	mutex_lock(&rdtgroup_mutex);
>  
> -	rdt_disable_ctx();
> -
>  	/* Put everything back to default values. */
>  	for_each_alloc_capable_rdt_resource(r)
>  		resctrl_arch_reset_all_ctrls(r);
>  
>  	resctrl_fs_teardown();
> +	rdt_flush_limbo();
> +
> +	/*
> +	 * Clear the CDP state last. free_rmid() translates between CLOSID and
> +	 * PARTID using the CDP state; clearing it first would make the
> +	 * translation inconsistent for the RMIDs being torn down.
> +	 */
> +	rdt_disable_ctx();
> +
>  	if (resctrl_arch_alloc_capable())
>  		resctrl_arch_disable_alloc();
>  	if (resctrl_arch_mon_capable())