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

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/10/30 23:29, 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.

Hi Ben,

Thanks for the explanation.

Perhaps we could add a comment in mpam_msc_drv_probe(), or clarify this
somewhere in the documentation—otherwise it can be quite confusing.

Thanks,
Jinjie

> 
> Thanks,
> 
> Ben
> 
>>
>>>
>>> Thanks,
>>>
>>> Ben
>>>
>>> [1] https://lore.kernel.org/lkml/59c21340-4e3d-4c48-9d9f-c0f78a023bac@arm.com/
>>>
>>>>
>>>> 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@vger.kernel.org
>>>> Cc: Ben Horgan <ben.horgan@arm.com>
>>>> Cc: Reinette Chatre <reinette.chatre@intel.com>
>>>> Cc: Fenghua Yu <fenghuay@nvidia.com>
>>>> Cc: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Fixes: f04046f2577a5 ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
>>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>>> ---
>>>>  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 = {
>>>
>>
> 

-- 
Best regards,
Jinjie