Re: [PATCH 01/24] x86/resctrl: Split struct rdt_resource
Hi Reinette,
On 17/11/2020 19:20, Reinette Chatre wrote:
> On 10/30/2020 9:10 AM, James Morse wrote:
>> Splitting rdt_domain up in a similar way happens in the next patch.
>> No change in behaviour, this patch just moves types around.
>
> Please remove the "this patch" term.
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c
>> b/arch/x86/kernel/cpu/resctrl/core.c
>> index e5f4ee8f4c3b..470661f2eb68 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -912,9 +938,14 @@ static __init bool get_rdt_resources(void)
>> static __init void rdt_init_res_defs_intel(void)
>> {
>> + struct rdt_hw_resource *hw_res;
>> struct rdt_resource *r;
>> + int i;
>> +
>> + for (i = 0; i < RDT_NUM_RESOURCES; i++) {
>> + hw_res = &rdt_resources_all[i];
>> + r = &rdt_resources_all[i].resctrl;
>> - for_each_rdt_resource(r) {
>> if (r->rid == RDT_RESOURCE_L3 ||
>> r->rid == RDT_RESOURCE_L3DATA ||
>> r->rid == RDT_RESOURCE_L3CODE ||
>
> Could using for_each_rdt_resource() remain with the additional assignment of
> hw_res?
> Similar to the later usage of for_each_alloc_enabled_rdt_resource()?
Sure. I didn't do it here because of the back-to-back container_of(), even
though the
array is defined in the same file. But the compiler will remove all of that.
>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> index c877642e8a14..ab6e584c9d2d 100644
>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> @@ -284,10 +284,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 rdt_resource *r;
>> for_each_alloc_enabled_rdt_resource(r) {
>> - if (!strcmp(resname, r->name) && rdtgrp->closid < r->num_closid)
>> + hw_res = resctrl_to_arch_res(r);
>> + if (!strcmp(resname, r->name) && rdtgrp->closid <
>> hw_res->num_closid)
>> return parse_line(tok, r, rdtgrp);
>> }
> This is the usage of for_each_alloc_enabled_rdt_resource() I refer to earlier.
(if it helps, the difference in treatment is because this one is to do with the
filesystem
interface, the others were clearly arch code)
Thanks,
James
Re: [PATCH 01/24] x86/resctrl: Split struct rdt_resource
Hi James,
On 10/30/2020 9:10 AM, James Morse wrote:
resctrl is the defacto Linux ABI for SoC resource partitioning features.
To support it on another architecture, it needs to be abstracted from
Intel RDT, and moved it to /fs/.
Current support for AMD PQoS should also be considered.
s/and moved it to/and moved to/?
Start by splitting struct rdt_resource, (the name is kept to keep the noise
down), and add some type-trickery to keep the foreach helpers working.
Move everything that that is particular to resctrl into a new header
s/that that/that/
file, keeping the x86 hardware accessors where they are. resctrl code
paths touching a 'hw' struct indicates where an abstraction is needed.
Splitting rdt_domain up in a similar way happens in the next patch.
No change in behaviour, this patch just moves types around.
Please remove the "this patch" term.
Signed-off-by: James Morse
---
arch/x86/kernel/cpu/resctrl/core.c| 258 --
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 14 +-
arch/x86/kernel/cpu/resctrl/internal.h| 138 +++-
arch/x86/kernel/cpu/resctrl/monitor.c | 32 +--
arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 4 +-
arch/x86/kernel/cpu/resctrl/rdtgroup.c| 69 +++---
include/linux/resctrl.h | 117 ++
7 files changed, 362 insertions(+), 270 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c
b/arch/x86/kernel/cpu/resctrl/core.c
index e5f4ee8f4c3b..470661f2eb68 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
...
@@ -912,9 +938,14 @@ static __init bool get_rdt_resources(void)
static __init void rdt_init_res_defs_intel(void)
{
+ struct rdt_hw_resource *hw_res;
struct rdt_resource *r;
+ int i;
+
+ for (i = 0; i < RDT_NUM_RESOURCES; i++) {
+ hw_res = &rdt_resources_all[i];
+ r = &rdt_resources_all[i].resctrl;
- for_each_rdt_resource(r) {
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L3DATA ||
r->rid == RDT_RESOURCE_L3CODE ||
Could using for_each_rdt_resource() remain with the additional
assignment of hw_res? Similar to the later usage of
for_each_alloc_enabled_rdt_resource()?
@@ -924,17 +955,22 @@ static __init void rdt_init_res_defs_intel(void)
r->cache.arch_has_sparse_bitmaps = false;
r->cache.arch_has_empty_bitmaps = false;
} else if (r->rid == RDT_RESOURCE_MBA) {
- r->msr_base = MSR_IA32_MBA_THRTL_BASE;
- r->msr_update = mba_wrmsr_intel;
+ hw_res->msr_base = MSR_IA32_MBA_THRTL_BASE;
+ hw_res->msr_update = mba_wrmsr_intel;
}
}
}
static __init void rdt_init_res_defs_amd(void)
{
+ struct rdt_hw_resource *hw_res;
struct rdt_resource *r;
+ int i;
+
+ for (i = 0; i < RDT_NUM_RESOURCES; i++) {
+ hw_res = &rdt_resources_all[i];
+ r = &rdt_resources_all[i].resctrl;
- for_each_rdt_resource(r) {
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L3DATA ||
r->rid == RDT_RESOURCE_L3CODE ||
Same here (keep using for_each_rdt_resource()?).
@@ -944,8 +980,8 @@ static __init void rdt_init_res_defs_amd(void)
r->cache.arch_has_sparse_bitmaps = true;
r->cache.arch_has_empty_bitmaps = true;
} else if (r->rid == RDT_RESOURCE_MBA) {
- r->msr_base = MSR_IA32_MBA_BW_BASE;
- r->msr_update = mba_wrmsr_amd;
+ hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
+ hw_res->msr_update = mba_wrmsr_amd;
}
}
}
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index c877642e8a14..ab6e584c9d2d 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -284,10 +284,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 rdt_resource *r;
for_each_alloc_enabled_rdt_resource(r) {
- if (!strcmp(resname, r->name) && rdtgrp->closid < r->num_closid)
+ hw_res = resctrl_to_arch_res(r);
+ if (!strcmp(resname, r->name) && rdtgrp->closid <
hw_res->num_closid)
return parse_line(tok, r, rdtgrp);
}
This is the usage of for_each_alloc_enabled_rdt_resource() I refer to
earlier.
rdt_last_cmd_printf("Unknown or unsupported resource name '%s'\n",
resname);
...
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h
b/arch/x86/ker
[PATCH 01/24] x86/resctrl: Split struct rdt_resource
resctrl is the defacto Linux ABI for SoC resource partitioning features.
To support it on another architecture, it needs to be abstracted from
Intel RDT, and moved it to /fs/.
Start by splitting struct rdt_resource, (the name is kept to keep the noise
down), and add some type-trickery to keep the foreach helpers working.
Move everything that that is particular to resctrl into a new header
file, keeping the x86 hardware accessors where they are. resctrl code
paths touching a 'hw' struct indicates where an abstraction is needed.
Splitting rdt_domain up in a similar way happens in the next patch.
No change in behaviour, this patch just moves types around.
Signed-off-by: James Morse
---
arch/x86/kernel/cpu/resctrl/core.c| 258 --
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 14 +-
arch/x86/kernel/cpu/resctrl/internal.h| 138 +++-
arch/x86/kernel/cpu/resctrl/monitor.c | 32 +--
arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 4 +-
arch/x86/kernel/cpu/resctrl/rdtgroup.c| 69 +++---
include/linux/resctrl.h | 117 ++
7 files changed, 362 insertions(+), 270 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c
b/arch/x86/kernel/cpu/resctrl/core.c
index e5f4ee8f4c3b..470661f2eb68 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -57,120 +57,134 @@ static void
mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m,
struct rdt_resource *r);
-#define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].domains)
+#define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].resctrl.domains)
-struct rdt_resource rdt_resources_all[] = {
+struct rdt_hw_resource rdt_resources_all[] = {
[RDT_RESOURCE_L3] =
{
- .rid= RDT_RESOURCE_L3,
- .name = "L3",
- .domains= domain_init(RDT_RESOURCE_L3),
+ .resctrl = {
+ .rid= RDT_RESOURCE_L3,
+ .name = "L3",
+ .cache_level= 3,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 1,
+ .cbm_idx_offset = 0,
+ },
+ .domains= domain_init(RDT_RESOURCE_L3),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L3_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level= 3,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 1,
- .cbm_idx_offset = 0,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_L3DATA] =
{
- .rid= RDT_RESOURCE_L3DATA,
- .name = "L3DATA",
- .domains= domain_init(RDT_RESOURCE_L3DATA),
+ .resctrl = {
+ .rid= RDT_RESOURCE_L3DATA,
+ .name = "L3DATA",
+ .cache_level= 3,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 2,
+ .cbm_idx_offset = 0,
+ },
+ .domains=
domain_init(RDT_RESOURCE_L3DATA),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L3_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level= 3,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 2,
- .cbm_idx_offset = 0,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_L3CODE] =
{
- .rid= RDT_RESOURCE_L3CODE,
- .name = "L3CODE",
- .domains= domain_init(RDT_RESOURCE_L3CODE),
+ .resctrl = {
+ .rid= RDT_RESOU
