Re: [RFC PATCH 07/38] arm_mpam: resctrl: Add boilerplate cpuhp and domain allocation

[Ben Horgan <ben.horgan@arm.com>]

Hi Jonathan,

On 12/18/25 11:30, Jonathan Cameron wrote:
> On Fri, 5 Dec 2025 21:58:30 +0000
> James Morse <james.morse@arm.com> wrote:
> 
>> resctrl has its own data structures to describe its resources. We
>> can't use these directly as we play tricks with the 'MBA' resource,
>> picking the MPAM controls or monitors that best apply. We may export
>> the same component as both L3 and MBA.
>>
>> Add mpam_resctrl_exports[] as the array of class->resctrl mappings we
>> are exporting, and add the cpuhp hooks that allocated and free the
>> resctrl domain structures.
>>
>> While we're here, plumb in a few other obvious things.
>>
>> CONFIG_ARM_CPU_RESCTRL is used to allow this code to be built
>> even though it can't yet be linked against resctrl.
>>
>> Signed-off-by: James Morse <james.morse@arm.com>
> Hi,
> 
> A few code flow related comments. Fairly trivial stuff but I think
> some parts of this can be made more readable / maintainable with
> minor reorganization.
> 
> Jonathan
> 
> 
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index 2996ad93fc3e..efaf7633bc35 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
> ...
> 
>> @@ -2516,6 +2522,12 @@ static void mpam_enable_once(void)
>>  	mutex_unlock(&mpam_list_lock);
>>  	cpus_read_unlock();
>>  
>> +	if (!err) {
>> +		err = mpam_resctrl_setup();
>> +		if (err)
>> +			pr_err("Failed to initialise resctrl: %d\n", err);
>> +	}
>> +
>>  	if (err) {
>>  		mpam_disable_reason = "Failed to enable.";
>>  		schedule_work(&mpam_broken_work);
> 
> I'd be tempted to move this to an error handling block via a goto
> making this bit
> 	if (err)
> 		goto err_disable_mpam;
> 
> 	err = mpam_resctrl_setup();
> 	if (err) {
> 		pr_err();
> 		goto err_dsiable_mpam;
> 	}
> 
> Up to you though. Personally I like all my good paths as straight line
> code with the errors handled in if (err) as that consistency really helps
> readability.  
> 

I'll leave this one as is. It looks like James tried hard to avoid a goto.

Thanks,

Ben