[v1,01/14] fs/resctrl: group the resource control types for schemata list
Commit Message
At the moment, resource control group support the basic control features
(that can be applied to system resources like cache, and Memory)
such as CAT (Cache Allocation Technology), and MBA (Memory Bandwidth
Allocation).
Apart from these basic controls, System can support other controls that
does not directly affect the allocation of memory-system resources.
Instead, it has an effect on conflicts that arise during access to
resources, such as Priority partitioning found on ARM MPAM.
In order to support control types of different nature, lets divide them
into different groups, already existing control features (CAT, and MBA)
is grouped under basic schemata type .i.e. SCHEMATA_BASIC, and to support
priority partition (the downstream priority one), it is placed under
SCHEMA_DSPRI control type. These control type is associated with list(s)
of schemata.
Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
Changes since RFC:
* No change, it's new patch.
---
fs/resctrl/rdtgroup.c | 26 ++++++++++++++++----------
include/linux/resctrl.h | 1 +
include/linux/resctrl_types.h | 6 ++++++
3 files changed, 23 insertions(+), 10 deletions(-)
Comments
Hi Amit,
On Wed, Jan 17, 2024 at 6:14 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 3ad308e9e226..125c4b0c2ff7 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -249,6 +249,7 @@ struct resctrl_schema {
> struct list_head list;
> char name[8];
> enum resctrl_conf_type conf_type;
> + enum resctrl_ctrl_type ctrl_type;
I don't see a difference between a conf_type and a ctrl_type other
than conf_type being used for CDP and ctrl_type being used for DSPRI.
> struct rdt_resource *res;
> u32 num_closid;
> };
> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> index 3897de9c4ecb..b9268ec3ba71 100644
> --- a/include/linux/resctrl_types.h
> +++ b/include/linux/resctrl_types.h
> @@ -57,6 +57,12 @@ enum resctrl_res_level {
> RDT_NUM_RESOURCES,
> };
>
> +enum resctrl_ctrl_type {
> + SCHEMA_BASIC = 0,
> + SCHEMA_DSPRI,
> + SCHEMA_NUM_CTRL_TYPE
> +};
Rather than enumerate every control type on all implementations, I
would prefer the approach taken in the MPAM resctrl_arch_rmid_read(),
where the MPAM driver would embed the resctrl_schema into a private
structure it defines and use container_of() in
resctrl_arch_update_domains(), which should take a resctrl_schema
parameter instead of an rdt_resource. Like resctrl_arch_rmid_read(),
the containing structure would direct the values to the appropriate
hardware.
I would like to see the MPAM driver owning the enumeration of this
feature and the common FS code just understanding the control as a
value that is passed back to the driver if its value is within a
stated acceptable range.
I think there's still a lot of work that needs to be done in the
overall design of resctrl to provide general support for controls and
monitors which do not exist in RDT and these patches provide useful
information to help us understand the issues.
Thanks!
-Peter
@@ -2256,7 +2256,9 @@ static int rdt_enable_ctx(struct rdt_fs_context *ctx)
return ret;
}
-static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type)
+static int schemata_list_add(struct rdt_resource *r,
+ enum resctrl_conf_type type,
+ enum resctrl_ctrl_type ctrl_type)
{
struct resctrl_schema *s;
const char *suffix = "";
@@ -2284,10 +2286,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
break;
}
- ret = snprintf(s->name, sizeof(s->name), "%s%s", r->name, suffix);
- if (ret >= sizeof(s->name)) {
- kfree(s);
- return -EINVAL;
+ if (ctrl_type == SCHEMA_BASIC) {
+ ret = snprintf(s->name, sizeof(s->name), "%s%s", r->name, suffix);
+ if (ret >= sizeof(s->name)) {
+ kfree(s);
+ return -EINVAL;
+ }
}
cl = strlen(s->name);
@@ -2300,14 +2304,15 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
if (r->cdp_capable && !resctrl_arch_get_cdp_enabled(r->rid))
cl += 4;
- if (cl > max_name_width)
+ if (cl > max_name_width && ctrl_type == SCHEMA_BASIC)
max_name_width = cl;
/*
* Choose a width for the resource data based on the resource that has
* widest cbm/data_width.
*/
- max_data_width = max(max_data_width, r->data_width);
+ if (ctrl_type == SCHEMA_BASIC)
+ max_data_width = max(max_data_width, r->data_width);
INIT_LIST_HEAD(&s->list);
list_add(&s->list, &resctrl_schema_all);
@@ -2328,17 +2333,18 @@ static int schemata_list_create(void)
continue;
if (resctrl_arch_get_cdp_enabled(r->rid)) {
- ret = schemata_list_add(r, CDP_CODE);
+ ret = schemata_list_add(r, CDP_CODE, SCHEMA_BASIC);
if (ret)
break;
- ret = schemata_list_add(r, CDP_DATA);
+ ret = schemata_list_add(r, CDP_DATA, SCHEMA_BASIC);
} else {
- ret = schemata_list_add(r, CDP_NONE);
+ ret = schemata_list_add(r, CDP_NONE, SCHEMA_BASIC);
}
if (ret)
break;
+
}
return ret;
@@ -249,6 +249,7 @@ struct resctrl_schema {
struct list_head list;
char name[8];
enum resctrl_conf_type conf_type;
+ enum resctrl_ctrl_type ctrl_type;
struct rdt_resource *res;
u32 num_closid;
};
@@ -57,6 +57,12 @@ enum resctrl_res_level {
RDT_NUM_RESOURCES,
};
+enum resctrl_ctrl_type {
+ SCHEMA_BASIC = 0,
+ SCHEMA_DSPRI,
+ SCHEMA_NUM_CTRL_TYPE
+};
+
#define CDP_NUM_TYPES (CDP_DATA + 1)
/*