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

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/10/26 23:08, Ben Horgan 写道:
> Hi Jinjie,
> 
> On 20/10/2026 14:42, Jinjie Ruan wrote:
>> From: Zeng Heng <zengheng4@huawei.com>
>>
>> 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@intel.com>
>> Cc: Reinette Chatre <reinette.chatre@intel.com>
>> Cc: Dave Martin <Dave.Martin@arm.com>
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Babu Moger <babu.moger@amd.com>
>> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>  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@huawei.com/

Hi Ben,

Thank you for the reminder, will update the patch based on the
discussion and your new suggestion.

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

Thank you! I will notice Tony's patch and post next version separately.

> 
> [1] https://lore.kernel.org/lkml/20260928221509.68002-14-tony.luck@intel.com/
> 
> 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())
> 

-- 
Best regards,
Jinjie