lkml.org 
[lkml]   [2020]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 04/24] x86/resctrl: Add a separate schema list for resctrl
    From
    Date
    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

    \
     
     \ /
      Last update: 2020-11-17 22:30    [W:3.669 / U:24.468 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site