The resctrl_schema struct holds properties that vary with the style of
configuration that resctrl applies to a resource.
There are already two values for the hardware's num_closid, depending on
whether the architecture presents the L3 or L3CODE/L3DATA resources.
As the way CDP changes the number of control groups that resctrl can create
is part of the user-space interface, it should be managed by the filesystem
parts of resctrl. This allows the architecture code to only describe the
value the hardware supports.
Add num_closid to resctrl_schema. This is the value seen by the filesystem,
and when the CDP resources are merged, will be different to the value
described by the arch code when CDP is enabled.
These functions operate on the num_closid value that is exposed to
user-space:
rdtgroup_parse_resource()
rdtgroup_schemata_show()
rdt_num_closids_show()
closid_init()
These are changed to use the schema value instead.
schemata_list_create() sets this value, and reaches into the architecture
specific structure to get the value. This will eventually be replaced with
a helper.
Reviewed-by: Jamie Iles <[email protected]>
Signed-off-by: James Morse <[email protected]>
---
Changes since v1:
* Added missing : in a comment.
* Expanded commit message.
* Reordered patches
---
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 9 +++------
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 13 ++++---------
include/linux/resctrl.h | 2 ++
3 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index a6f9548a8a59..fcd6ca73ac41 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -286,14 +286,12 @@ int update_domains(struct rdt_resource *r, int closid)
static int rdtgroup_parse_resource(char *resname, char *tok,
struct rdtgroup *rdtgrp)
{
- struct rdt_hw_resource *hw_res;
struct resctrl_schema *s;
struct rdt_resource *r;
list_for_each_entry(s, &resctrl_schema_all, list) {
r = s->res;
- hw_res = resctrl_to_arch_res(s->res);
- if (!strcmp(resname, r->name) && rdtgrp->closid < hw_res->num_closid)
+ if (!strcmp(resname, r->name) && rdtgrp->closid < s->num_closid)
return parse_line(tok, r, rdtgrp);
}
rdt_last_cmd_printf("Unknown or unsupported resource name '%s'\n", resname);
@@ -404,7 +402,6 @@ static void show_doms(struct seq_file *s, struct rdt_resource *r, int closid)
int rdtgroup_schemata_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
- struct rdt_hw_resource *hw_res;
struct resctrl_schema *schema;
struct rdtgroup *rdtgrp;
struct rdt_resource *r;
@@ -432,8 +429,8 @@ int rdtgroup_schemata_show(struct kernfs_open_file *of,
} else {
closid = rdtgrp->closid;
list_for_each_entry(schema, &resctrl_schema_all, list) {
- hw_res = resctrl_to_arch_res(schema->res);
- if (closid < hw_res->num_closid)
+ r = schema->res;
+ if (closid < schema->num_closid)
show_doms(s, r, closid);
}
}
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 4f25c3c6f6e8..1ff883f68ee1 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -103,15 +103,12 @@ int closids_supported(void)
static void closid_init(void)
{
- struct rdt_hw_resource *hw_res;
struct resctrl_schema *s;
int rdt_min_closid = 32;
/* Compute rdt_min_closid across all resources */
- list_for_each_entry(s, &resctrl_schema_all, list) {
- hw_res = resctrl_to_arch_res(s->res);
- rdt_min_closid = min(rdt_min_closid, hw_res->num_closid);
- }
+ list_for_each_entry(s, &resctrl_schema_all, list)
+ rdt_min_closid = min(rdt_min_closid, s->num_closid);
closid_free_map = BIT_MASK(rdt_min_closid) - 1;
@@ -849,11 +846,8 @@ static int rdt_num_closids_show(struct kernfs_open_file *of,
struct seq_file *seq, void *v)
{
struct resctrl_schema *s = of->kn->parent->priv;
- struct rdt_resource *r = s->res;
- struct rdt_hw_resource *hw_res;
- hw_res = resctrl_to_arch_res(r);
- seq_printf(seq, "%d\n", hw_res->num_closid);
+ seq_printf(seq, "%u\n", s->num_closid);
return 0;
}
@@ -2140,6 +2134,7 @@ static int schemata_list_create(void)
s->res = r;
s->conf_type = resctrl_to_arch_res(r)->conf_type;
+ s->num_closid = resctrl_to_arch_res(r)->num_closid;
INIT_LIST_HEAD(&s->list);
list_add(&s->list, &resctrl_schema_all);
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index c6f749f54765..0ff10468940b 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -165,10 +165,12 @@ struct rdt_resource {
* @list: Member of resctrl's schema list
* @conf_type: Whether this entry is for code/data/both
* @res: The rdt_resource for this entry
+ * @num_closid: Number of CLOSIDs available for this resource
*/
struct resctrl_schema {
struct list_head list;
enum resctrl_conf_type conf_type;
struct rdt_resource *res;
+ int num_closid;
};
#endif /* _RESCTRL_H */
--
2.30.0
Hi James,
On 3/12/2021 9:58 AM, James Morse wrote:
> The resctrl_schema struct holds properties that vary with the style of
> configuration that resctrl applies to a resource.
>
> There are already two values for the hardware's num_closid, depending on
> whether the architecture presents the L3 or L3CODE/L3DATA resources.
>
> As the way CDP changes the number of control groups that resctrl can create
> is part of the user-space interface, it should be managed by the filesystem
s/create is part of/create as part of/ ?
> parts of resctrl. This allows the architecture code to only describe the
> value the hardware supports.
>
> Add num_closid to resctrl_schema. This is the value seen by the filesystem,
> and when the CDP resources are merged, will be different to the value
> described by the arch code when CDP is enabled.
>
> These functions operate on the num_closid value that is exposed to
> user-space:
> rdtgroup_parse_resource()
> rdtgroup_schemata_show()
> rdt_num_closids_show()
> closid_init()
>
> These are changed to use the schema value instead.
>
> schemata_list_create() sets this value, and reaches into the architecture
> specific structure to get the value. This will eventually be replaced with
> a helper.
>
> Reviewed-by: Jamie Iles <[email protected]>
> Signed-off-by: James Morse <[email protected]>
...
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index c6f749f54765..0ff10468940b 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -165,10 +165,12 @@ struct rdt_resource {
> * @list: Member of resctrl's schema list
> * @conf_type: Whether this entry is for code/data/both
> * @res: The rdt_resource for this entry
> + * @num_closid: Number of CLOSIDs available for this resource
It is not clear what "this resource" is. This description is essentially
the same as its copy found in rdt_hw_resource. Could you please improve
these descriptions to capture the differences between the two num_closid
values?
Thank you
Reinette