Re: [PATCH v5 RESEND 3/7] fs/resctrl: Clear the CDP state after the cleanup

[Jinjie Ruan]

在 2026/11/12 2:27, Reinette Chatre 写道:
> Hi Jinjie,
> 
> On 11/10/26 5:39 AM, Jinjie Ruan wrote:
>> On MPAM a CLOSID is translated into the PARTIDs that the hardware
>> configuration uses. Without CDP, CLOSID c is PARTID c; with CDP it is
>> PARTIDs 2c and 2c+1. resctrl_unmount() clears the CDP state with
>> rdt_disable_ctx() before resctrl_fs_teardown() removes the groups, so
>> the teardown translates the CLOSIDs differently from when the groups
>> were set up.
> 
> This behavior is not unique to MPAM. Above makes this sound like
> architecture specific behavior that makes one wonder why this needs
> to be fixed in resctrl filesystem

Will update it >
>>
>> For example, rdtgroup_unassign_cntrs() unassigns and resets the MBM
>> counters of each group through resctrl_arch_config_cntr(). On MPAM that
>> helper maps a counter to a monitor with the index
>> resctrl_get_config_index() returns for the CDP state. A counter assigned
> 
> Please take care not to narrate the code. Instead just describe the flow
> on a high level to help understand the change without needing reader to
> refer to the code much.

Sorry, will describe it on a high level>
>> while CDP was enabled is mapped to the monitor for PARTIDs 2c and 2c+1.
>> With CDP cleared, MPAM removes the mapping of PARTID c and resets the
>> monitor for PARTID c instead, so the mappings of PARTIDs 2c and 2c+1 are
>> left behind. The helper had an empty body on MPAM until the ABMC support
>> implemented it, which is when the teardown started to program the
>> mapping with the cleared state.
>>
>> Clear the CDP state after resctrl_fs_teardown(), so the teardown runs
>> with the CDP state the configuration was created under.
>>
>> x86 is not affected as its RMIDs and counters do not depend on the
>> CLOSID.
> 
> I think x86 is lucky here to not have encountered this issue and this fix
> applies to it just as much, adding to motivation for this needing to be
> included in the filesystem. Below is an example of a changelog that aims
> to capture all the comments, you need not use verbatim and are welcome to
> improve (and please correct) it:

Will consider the example, thank you!

> 
> 	To support CDP, resctrl halves the number of CLOSIDs available to
> 	resource groups and gives each remaining CLOSID two hardware
> 	configurations instead of one, distinguished by index: 2 * CLOSID for
> 	data and 2 * CLOSID + 1 for code. Without CDP a CLOSID has a single
> 	configuration, at index CLOSID. Every architecture computes these
> 	indices the same way.
> 
> 	Unmounting clears the CDP state before it undoes what the groups were
> 	given, so the cleanup computes index CLOSID where the group was set up
> 	with 2 * CLOSID and 2 * CLOSID + 1. The group's two configurations are
> 	left in place and index CLOSID is cleared instead, which with CDP
> 	enabled is a configuration of a different CLOSID. The architecture may
> 	keep this state for as long as the resource exists.
> 
> 	Only MPAM can reach this today. Of everything the cleanup has to undo,
> 	MPAM addresses only the MBM counter assignment by the translated CLOSID,
> 	and it only began doing so when it implemented assignable counters. x86
> 	addresses its counters by RMID alone.
> 
> 	Clear the CDP state after the cleanup, so that the cleanup runs with the
> 	CDP state the groups were set up under.
> 
>>
>> Fixes: 6e0e538a75ce ("arm_mpam: resctrl: Add resctrl_arch_config_cntr() for ABMC use")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
>> Cc: <stable@xxxxxxxxxxxxxxx> # 7.3+
> 
> Looks like 6e0e538a75ce landed in 7.3 so the stable constraint looks unnecessary?
> 
>> ---
> Patch looks good to me.

Thanks for the review.

> 
> Thank you very much.
> 
> Reinette

-- 
Best regards,
Jinjie