linux-kernel - Re: [PATCH 08/24] x86/resctrl: Walk the resctrl schema list instead of an arch list

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 for Android: free password hash cracker in your pocket

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

Message-ID: <d512c41e-7bea-873d-f209-debf1b963301@arm.com>
Date:   Fri, 12 Mar 2021 17:37:48 +0000
From:   James Morse <james.morse@....com>
To:     Reinette Chatre <reinette.chatre@...el.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Cc:     Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        shameerali.kolothum.thodi@...wei.com,
        Jamie Iles <jamie@...iainc.com>,
        D Scott Phillips OS <scott@...amperecomputing.com>
Subject: Re: [PATCH 08/24] x86/resctrl: Walk the resctrl schema list instead
 of an arch list

Hi Reinette,

On 17/11/2020 22:52, Reinette Chatre wrote:
> On 10/30/2020 9:11 AM, James Morse wrote:
>> Now that resctrl has its own list of resources it is using, walk that
>> list instead of the architectures list. This means resctrl has somewhere
>> to keep schema properties with the resource that is using them.
>>
>> Most users of for_each_alloc_enabled_rdt_resource() are per-schema,
>> and also want a schema property, like the conf_type. Switch these to
>> walk the schema list. Schema were only created for alloc_enabled
>> resources so these two lists are currently equivalent.
>>
> 
> From what I understand based on this description the patch will essentially change
> instances of for_each_alloc_enabled_rdt_resource() to walking the schema list ....

>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> index 8ac104c634fe..d3f9d142f58a 100644
>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> @@ -57,9 +57,10 @@ static bool bw_validate(char *buf, unsigned long *data, struct
>> rdt_resource *r)
>>       return true;
>>   }
>>   -int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
>> +int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
>>            struct rdt_domain *d)
>>   {
>> +    struct rdt_resource *r = s->res;
>>       unsigned long bw_val;
>>         if (d->have_new_ctrl) {
> 
> ... this change and also the ones to parse_cbm() and rdtgroup_cbm_overlaps() are not clear
> to me because it seems they replace the rdt_resource parameter with resctrl_schema, but
> all in turn use that to access rdt_resource again. That seems unnecessary?

I previously restructured the series to do the schema stuff first, as I thought it would
make it easier to follow. It looks like this patch has picked up other stuff - I 'll split
this up so those changes get their own commit message.

By the end of the series, the rdt_resource isn't unique, the same 'L3' is provided to
L3CODE and L3DATA. The things that make these different are stored in the schema. In both
these cases the configuration is read/written, where that should go depends on the type of
the schema, which lives in the schema struct.


Thanks,

James