Re: [PATCH v10 RESEND 05/15] arm_mpam: Don't stall MPAM enable when an MSC fails to probe

[Ben Horgan]

Hi Jinjie,

On 27/10/2026 01:47, Jinjie Ruan wrote:
> 
> 
> 在 2026/10/26 22:49, Ben Horgan 写道:
>> Hi Jinjie,
>>
>>
>>
>> On 20/10/2026 14:42, Jinjie Ruan wrote:
>>> mpam_msc_drv_probe() registers the discovery cpuhp callbacks only when
>>> the number of successfully probed MSCs reaches fw_num_msc. Every failure
>>> path returns before the counter is incremented, and a failed probe is
>>> final - the driver core does not retry. One failing MSC therefore leaves
>>> the counter below fw_num_msc forever: the discovery callbacks are never
>>> registered, mpam_enable() is never scheduled, and MPAM stays dead
>>> system-wide - the resctrl filesystem is never registered, and no error
>>> message hints at the cause.
>>>
>>> An MSC can legitimately fail to probe on an otherwise working system:
>>> firmware may describe MSCs that are not accessible from any possible
>>> CPU, e.g. when the possible CPUs are restricted with nr_cpus=. The MSCs
>>> of the clipped CPUs then fail with "MSC is not accessible from any
>>> CPU!", or their cache resources become unresolvable from PPTT.
>>
>> As the MPAM resources are system wide we need to query the support from each of the MSC to know what
>> resources will end up. I do appreciate that if you have full control of your system then you can
>> know this in advance but then you can also adjust your acpi tables to mark the non-accessible MSC as
>> disabled. As such, I don't think changes such as this should be in the driver. Here is a previous
>> explanation for a similar patch [1]
> 
> Hi Ben,
> 
> Does this mean that "nr_cpus=" and the MPAM enabling are mutually exclusive?
> 
> I was previously doing experiments related to MB nodes and ran into a
> failure of topology_matches_l3(). I therefore tried to reduce the number
> of system CPUs from two clusters to one cluster using "nr_cpus=", but as
> a result I ran into this problem, which prevented MPAM from being
> enabled properly.

Yes, unfortunately this is expected behaviour.

Thanks,

Ben

> 
>>
>> Thanks,
>>
>> Ben
>>
>> [1] https://lore.kernel.org/lkml/59c21340-4e3d-4c48-9d9f-c0f78a023bac@xxxxxxx/
>>
>>>
>>> Count every completed probe, successful or not. Failed MSCs are not on
>>> mpam_all_msc (either never added, or removed by mpam_msc_drv_remove()),
>>> so the discovery path skips them naturally, and MPAM is enabled for the
>>> surviving devices.
>>>
>>> Cc: stable@xxxxxxxxxxxxxxx
>>> Cc: Ben Horgan <ben.horgan@xxxxxxx>
>>> Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
>>> Cc: Fenghua Yu <fenghuay@xxxxxxxxxx>
>>> Cc: Shaopeng Tan <tan.shaopeng@xxxxxxxxxxxxxx>
>>> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
>>> Fixes: f04046f2577a5 ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
>>> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
>>> ---
>>>  drivers/resctrl/mpam_devices.c | 26 ++++++++++++++++++--------
>>>  1 file changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>>> index 61cbc75625fa..a44d66a5733d 100644
>>> --- a/drivers/resctrl/mpam_devices.c
>>> +++ b/drivers/resctrl/mpam_devices.c
>>> @@ -2120,21 +2120,31 @@ static int mpam_msc_drv_probe(struct platform_device *pdev)
>>>  	msc = do_mpam_msc_drv_probe(pdev);
>>>  	mutex_unlock(&mpam_list_lock);
>>>  
>>> -	if (IS_ERR(msc))
>>> -		return PTR_ERR(msc);
>>> +	if (IS_ERR(msc)) {
>>> +		err = PTR_ERR(msc);
>>> +		goto out;
>>> +	}
>>>  
>>>  	/* Create RIS entries described by firmware */
>>>  	err = acpi_mpam_parse_resources(msc, plat_data);
>>> -	if (err) {
>>> +	if (err)
>>>  		mpam_msc_drv_remove(pdev);
>>> -		return err;
>>> -	}
>>> -
>>> -	if (atomic_add_return(1, &mpam_num_msc) == fw_num_msc)
>>> +out:
>>> +	/*
>>> +	 * A failed MSC must not prevent MPAM being enabled for the
>>> +	 * surviving devices. Firmware may describe MSCs that are not
>>> +	 * accessible from any possible CPU, e.g. when the possible CPUs
>>> +	 * were restricted by nr_cpus=. The failed device is not
>>> +	 * re-probed, so count it here too: otherwise the counter never
>>> +	 * reaches fw_num_msc, the discovery cpuhp callbacks are never
>>> +	 * registered and mpam_enable() is never scheduled - MPAM stays
>>> +	 * dead system-wide, without any error message.
>>> +	 */
>>> +	if (atomic_inc_return(&mpam_num_msc) == fw_num_msc)
>>>  		mpam_register_cpuhp_callbacks(mpam_discovery_cpu_online, NULL,
>>>  					      "mpam:drv_probe");
>>>  
>>> -	return 0;
>>> +	return err;
>>>  }
>>>  
>>>  static struct platform_driver mpam_msc_driver = {
>>
>