Messages in this thread | | | Subject | Re: [PATCH v4 02/24] x86/resctrl: Split struct rdt_domain | From | James Morse <> | Date | Thu, 17 Jun 2021 18:02:15 +0100 |
| |
Hi Babu,
On 15/06/2021 18:51, Babu Moger wrote: > On 6/14/21 3:09 PM, James Morse wrote: >> resctrl is the defacto Linux ABI for SoC resource partitioning features. >> >> To support it on another architecture, it needs to be abstracted from >> the features provided by Intel RDT and AMD PQoS, and moved to /fs/. >> struct rdt_resource contains a mix of architecture private details >> and properties of the filesystem interface user-space users. >> >> Continue by splitting struct rdt_domain, into an architecture private >> 'hw' struct, which contains the common resctrl structure that would be >> used by any architecture. The hardware values in ctrl_val and mbps_val >> need to be accessed via helpers to allow another architecture to convert >> these into a different format if necessary. After this split, filesystem >> code paths touching a 'hw' struct indicates where an abstraction >> is needed. >> >> Splitting this structure only moves types around, and should not lead >> to any change in behaviour.
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h >> index 43c8cf6b2b12..235cf621c878 100644 >> --- a/arch/x86/kernel/cpu/resctrl/internal.h >> +++ b/arch/x86/kernel/cpu/resctrl/internal.h >> @@ -299,44 +299,25 @@ struct mbm_state {
>> -struct rdt_domain { >> - struct list_head list; >> - int id; >> - struct cpumask cpu_mask; >> - unsigned long *rmid_busy_llc; >> - struct mbm_state *mbm_total; >> - struct mbm_state *mbm_local; >> - struct delayed_work mbm_over; >> - struct delayed_work cqm_limbo; >> - int mbm_work_cpu; >> - int cqm_work_cpu; >> +struct rdt_hw_domain { >> + struct rdt_domain resctrl;
> Naming is bit confusing here. There is another field with the same > name(patch1).
But a totally different type, you'd only access its members via the resource or domain, so its always clear which it is. (and if you get them wrong, it won't build)
> +struct rdt_hw_resource { > + struct rdt_resource resctrl; > > I think we should make this bit more clearer. May be or something similar. > > struct rdt_hw_domain { > struct rdt_domain d_resctrl; Sure, I guess it makes it clear when quoting something.
Thanks,
James
| |