Messages in this thread | | | Subject | Re: [PATCH 05/24] x86/resctrl: Pass the schema in resdir's private pointer | From | Reinette Chatre <> | Date | Tue, 17 Nov 2020 13:49:56 -0800 |
| |
Hi James,
It is not clear what "resdir" mentioned in subject line refers to. Could it be changed to "info dir"?
On 10/30/2020 9:11 AM, James Morse wrote: > Moving properties that resctrl exposes to user-space into the core > 'fs' code, (e.g. the name of the schema), means some of the functions > that back the filesystem need the schema struct, but currently take the > resource.
I think a simple addition would help to parse the above ...
" ... need the schema struct (to where the properties are moved), ..."
> > Once the CDP resources are merged, the resource doesn't reflect the > right level of information. > > For the info dirs that represent a control, the information needed > is in the schema, as this is how the resource is being used. For the > monitors, its the resource as L3CODE_MON doesn't make sense, and would > monitor data too. > > This difference means the type of the private pointers varies > between control and monitor info dirs. > > If the flags are RF_MON_INFO, its a struct rdt_resource. If the > flags are RF_CTRL_INFO, its a struct resctrl_schema. Nothing in > res_common_files[] has both flags. > > Signed-off-by: James Morse <james.morse@arm.com> > > --- > Fake schema for monitors may simplify this if anyone thinks that is > preferable. > --- > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 37 +++++++++++++++++--------- > 1 file changed, 24 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index f79a5e548138..cb16454a6b0e 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
...
> @@ -1794,6 +1803,7 @@ static int rdtgroup_mkdir_info_resdir(struct rdt_resource *r, char *name, > > static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn) > { > + struct resctrl_schema *s; > struct rdt_resource *r; > unsigned long fflags; > char name[32]; > @@ -1809,9 +1819,10 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn) > if (ret) > goto out_destroy; > > - for_each_alloc_enabled_rdt_resource(r) { > + list_for_each_entry(s, &resctrl_all_schema, list) { > + r = s->res; > fflags = r->fflags | RF_CTRL_INFO; > - ret = rdtgroup_mkdir_info_resdir(r, r->name, fflags); > + ret = rdtgroup_mkdir_info_resdir(s, r->name, fflags); > if (ret) > goto out_destroy; > } >
I think it would be helpful to add a comment here to compensate for the symmetry that is removed ("for_each_alloc_enabled_rdt_resource()" followed by a "for_each_mon_enabled_rdt_resource()").
Reinette
| |