Messages in this thread | | | Subject | Re: [PATCH 04/24] x86/resctrl: Add a separate schema list for resctrl | From | Reinette Chatre <> | Date | Tue, 17 Nov 2020 13:29:21 -0800 |
| |
Hi James,
On 10/30/2020 9:11 AM, James Morse wrote: > To support multiple architectures, the resctrl code needs to be split > into a 'fs' specific part in core code, and an arch-specific backend. > > It should be difficult for the arch-specific backends to diverge, > supporting slightly different ABIs for user-space. For example, > generating, parsing and validating the schema configuration values > should be done in what becomes the core code to prevent divergence. > Today, the schema emerge from which entries in the rdt_resources_all > array the arch code has chosen to enable. > > Start by creating a struct resctrl_schema, which will eventually hold > the name and pending configuration values for resctrl.
Looking ahead I am not able to identify the "pending configuration values" that will eventually be held in resctrl_schema. With entire series applied there is the name, type, num_closid, and pointer to the resource.
> > Signed-off-by: James Morse <james.morse@arm.com> > --- > arch/x86/kernel/cpu/resctrl/internal.h | 1 + > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 43 +++++++++++++++++++++++++- > include/linux/resctrl.h | 9 ++++++ > 3 files changed, 52 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h > index f7aab9245259..682e84aebd14 100644 > --- a/arch/x86/kernel/cpu/resctrl/internal.h > +++ b/arch/x86/kernel/cpu/resctrl/internal.h > @@ -106,6 +106,7 @@ extern unsigned int resctrl_cqm_threshold; > extern bool rdt_alloc_capable; > extern bool rdt_mon_capable; > extern unsigned int rdt_mon_features; > +extern struct list_head resctrl_all_schema;
Considering the relationship with rdt_resources_all it seems more consistent to call it resctrl_schema_all?
> > enum rdt_group_type { > RDTCTRL_GROUP = 0, > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index df10135f021e..f79a5e548138 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -39,6 +39,9 @@ static struct kernfs_root *rdt_root; > struct rdtgroup rdtgroup_default; > LIST_HEAD(rdt_all_groups); > > +/* list of entries for the schemata file */ > +LIST_HEAD(resctrl_all_schema); > + > /* Kernel fs node for "info" directory under root */ > static struct kernfs_node *kn_info; > > @@ -2117,6 +2120,35 @@ static int rdt_enable_ctx(struct rdt_fs_context *ctx) > return ret; > } > > +static int create_schemata_list(void) > +{
Could you please use the new namespace (schema_list_) as a prefix instead of suffix? That will make it easier to search.
> + struct resctrl_schema *s; > + struct rdt_resource *r; > + > + for_each_alloc_enabled_rdt_resource(r) { > + s = kzalloc(sizeof(*s), GFP_KERNEL); > + if (!s) > + return -ENOMEM; > + > + s->res = r; > + > + INIT_LIST_HEAD(&s->list); > + list_add(&s->list, &resctrl_all_schema); > + } > + > + return 0; > +} > + > +static void destroy_schemata_list(void) > +{ > + struct resctrl_schema *s, *tmp; > + > + list_for_each_entry_safe(s, tmp, &resctrl_all_schema, list) { > + list_del(&s->list); > + kfree(s); > + } > +} > + > static int rdt_get_tree(struct fs_context *fc) > { > struct rdt_fs_context *ctx = rdt_fc2context(fc);
...
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h > index dfb0f32b73a1..de6cbc725753 100644 > --- a/include/linux/resctrl.h > +++ b/include/linux/resctrl.h > @@ -163,6 +163,15 @@ struct rdt_resource { > > }; > > +/**
Missing a kernel-doc description here
> + * @list: Member of resctrl's schema list > + * @res: The rdt_resource for this entry > + */ > +struct resctrl_schema { > + struct list_head list; > + struct rdt_resource *res; > +}; > + > /* The number of closid supported by this resource regardless of CDP */ > u32 resctrl_arch_get_num_closid(struct rdt_resource *r); > >
Reinette
| |