Hi Reinette, On 17/11/2020 22:30, Reinette Chatre wrote: > On 10/30/2020 9:11 AM, James Morse wrote: >> Before the name for the schema can be generated, the type of the >> configuration being applied to the resource needs to be known. Label >> all the entries in rdt_resources_all[], and copy that value in to struct > > s/in to/into/ or s/in to/to/ ? > >> resctrl_schema. >> > > This commit message does not explain why it is needed to copy this value. > >> Subsequent patches will generate the schema names in what will become >> the fs code. Eventually the fs code will generate pairs of CODE/DATA if >> the platform supports CDP for this resource. > > Explaining how the copy is a step towards accomplishing this goal would be > very helpful.
(I've added text about what this is used for, and why it can't assign the values it wants at this point in the series) >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index 1bd785b1920c..628e5eb4d7a9 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -2141,6 +2141,7 @@ static int create_schemata_list(void) >> s->res = r; >> s->num_closid = resctrl_arch_get_num_closid(r); > > Above seems to be last user of this helper remaining ... why is helper needed > instead of > something similar to below? Great question, resctrl_to_arch_res(r)->num_closid? That won't work for MPAM, or would at least force all architectures to copy x86's arch-specific structure. schemata_list_create() needs to be part of the filesystem code after the split, but it can't touch the hw structure like the conf_type is doing here. This is mentioned in the commit message of the first two patches: | resctrl code paths touching a 'hw' struct indicates where an abstraction is needed. I evidently need to make that clearer. >> + s->conf_type = resctrl_to_arch_res(r)->conf_type; I'll to do this temporarily, as by then end of the series schemata_list_create() chooses the value, so this disappears. Thanks, James
