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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7100e3a7-4f08-4156-bf0b-6598570abecd@arm.com>
Date: Fri, 19 Dec 2025 12:17:15 +0000
From: Ben Horgan <ben.horgan@....com>
To: Jonathan Cameron <jonathan.cameron@...wei.com>,
 James Morse <james.morse@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 D Scott Phillips OS <scott@...amperecomputing.com>,
 carl@...amperecomputing.com, lcherian@...vell.com,
 bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
 baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
 Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
 dfustini@...libre.com, amitsinght@...vell.com,
 David Hildenbrand <david@...nel.org>, Dave Martin <dave.martin@....com>,
 Koba Ko <kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
 fenghuay@...dia.com, baisheng.gao@...soc.com, Gavin Shan <gshan@...hat.com>,
 rohit.mathew@....com, reinette.chatre@...el.com,
 Punit Agrawal <punit.agrawal@....qualcomm.com>
Subject: Re: [RFC PATCH 07/38] arm_mpam: resctrl: Add boilerplate cpuhp and
 domain allocation

Hi Jonathan,

On 12/18/25 11:30, Jonathan Cameron wrote:
> On Fri, 5 Dec 2025 21:58:30 +0000
> James Morse <james.morse@....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@....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