Re: [PATCH v2 17/29] arm_mpam: Extend reset logic to allow devices to be reset any time

[James Morse]

Hi Ben,

On 12/09/2025 12:42, Ben Horgan wrote:
> On 9/10/25 21:42, James Morse wrote:
>> cpuhp callbacks aren't the only time the MSC configuration may need to
>> be reset. Resctrl has an API call to reset a class.
>> If an MPAM error interrupt arrives it indicates the driver has
>> misprogrammed an MSC. The safest thing to do is reset all the MSCs
>> and disable MPAM.
>>
>> Add a helper to reset RIS via their class. Call this from mpam_disable(),
>> which can be scheduled from the error interrupt handler.
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index e7faf453b5d7..a9d3c4b09976 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -842,8 +842,6 @@ static int mpam_reset_ris(void *arg)
>>  	u16 partid, partid_max;
>>  	struct mpam_msc_ris *ris = arg;
>>
>> -	mpam_assert_srcu_read_lock_held();
>> -
>
> Remove where it is introduced. There is already one in
> mpam_reset_ris_partid() at that time.

Mmmm, this should really have been replaced with a comment.

I prefer each function to have an assert like this as documentation. In this case, a new
caller may miss the lock, but always hit the 'in_reset_state' case during testing, and be
caught out when a call to mpam_reset_ris_partid() occurs. Having documentation comments
that can also bark at you when you ignore them is really handy!

It's removed in this  patch  because calling it via mpam_touch_msc() puts it behind an
call to schedule, and lockdep expects 'current' to be the one holding the lock.

I'll add the comment. Looks like it just got dropped when mpam_touch_msc() stopped using
an IPI...


>>  	if (ris->in_reset_state)
>>  		return 0;
>
> Reviewed-by: Ben Horgan <ben.horgan@xxxxxxx>

Thanks!


James