Re: [PATCH v5 02/40] x86/resctrl: Add a helper to avoid reaching into the arch code resource list
From: James Morse
Date: Fri Oct 18 2024 - 13:07:52 EST
Hi Tony,
On 15/10/2024 23:57, Tony Luck wrote:
> On Fri, Oct 04, 2024 at 06:03:09PM +0000, James Morse wrote:
>> +struct rdt_resource *resctrl_arch_get_resource(enum resctrl_res_level l)
>> +{
>> + if (l >= RDT_NUM_RESOURCES)
>> + return NULL;
>> +
>> + return &rdt_resources_all[l].r_resctrl;
>> +}
>
> Is this a bit fragile if someone adds a new item in enum resctrl_res_level
> but doesn't add a new entry to struct rdt_hw_resource rdt_resources_all[]
> in arch/x86/kernel/cpu/resctrl/core.c
>
> Any caller of resctrl_arch_get_resource(new item name) will get past
> the check "if (l >= RDT_NUM_RESOURCES)" and then return a pointer past
> the end of the rdt_resources_all[] array.
>
> Maybe make sure the array is padded out to the right size?
>
> struct rdt_hw_resource rdt_resources_all[RDT_NUM_RESOURCES - 1] = {
> ...
> };
Sure.
I was planning to do away with the 'must not return NULL' behaviour before extra resources
start appearing. It's done like this to avoid the churn when x86 supports 'all' the
resources anyway, buy you're right it can be less-churn and safer at the same time!
Thanks,
James