Messages in this thread | | | Subject | Re: [PATCH v2 02/24] x86/resctrl: Split struct rdt_domain | From | Reinette Chatre <> | Date | Thu, 8 Apr 2021 13:13:01 -0700 |
| |
Hi James,
On 4/8/2021 10:20 AM, James Morse wrote: > On 31/03/2021 22:36, Reinette Chatre wrote: >> On 3/12/2021 9:58 AM, 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/. >>> >>> Split struct rdt_domain up too. Move everything that that is particular >> >> s/that that/that/ >> >>> to resctrl into a new header file. resctrl code paths touching a 'hw' >>> struct indicates where an abstraction is needed. >> >> Similar to previous patch it would help to explain how this split was chosen. For example, >> why are the CPUs to which a resource is associated not considered a hardware property? > > Similarly, because the meaning of those CPUs doesn't differ or change between architectures.
Got it. This seems to be a repeat of the discussion about patch 1. Please note that the description of rdt_hw_domain in this patch reads "hw attributes of a group of CPUs that share a resource". This can be understood to mean that the struct contains attributes discovered from hardware.
> I've expanded the middle paragraph in the commit message to explain why the arch specific > things are arch specific: > | 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 code paths touching a 'hw' struct > | indicates where an abstraction is needed. > > and similarly changed the kernel doc comment.
Thank you. I assume this includes changing the "hw attributes of a group of CPUs that share a resource" I mention above.
> > Let me know if you prefer some other struct name. >
I am ok with current naming. Other folks may have better ideas.
Thank you
Reinette
| |