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

[Jinjie Ruan]

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

> 
> 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 = {
> 

-- 
Best regards,
Jinjie