Resctrl uses local memory bandwidth event as input to the feedback
loop when the mba_MBps mount option is used. This means that this
mount option cannot be used on systems that only support monitoring
of total bandwidth.
Prepare to allow users to choose the input event independently for
each CTRL_MON group by adding a global variable "mba_mbps_default_event"
used to set the default event for each CTRL_MON group, and a new
field "mba_mbps_event" in struct rdtgroup to track which event is
used for each CTRL_MON group.
Notes:
1) Both of these are only used when the user mounts the filesystem with
the "mba_MBps" option.
2) Only check for support of local bandwidth event when initializing
mba_mbps_default_event. Support for total bandwidth event can be added
after other routines in resctrl have been updated to handle total
bandwidth event.
Signed-off-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
---
include/linux/resctrl.h | 2 ++
arch/x86/kernel/cpu/resctrl/internal.h | 2 ++
arch/x86/kernel/cpu/resctrl/core.c | 3 +++
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 13 +++++++++++++
4 files changed, 20 insertions(+)
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index d94abba1c716..fd05b937e2f4 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -49,6 +49,8 @@ enum resctrl_event_id {
QOS_L3_MBM_LOCAL_EVENT_ID = 0x03,
};
+extern enum resctrl_event_id mba_mbps_default_event;
+
/**
* struct resctrl_staged_config - parsed configuration to be applied
* @new_ctrl: new ctrl value to be loaded
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index faaff9d64102..485800055a7d 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -283,6 +283,7 @@ struct pseudo_lock_region {
* monitor only or ctrl_mon group
* @mon: mongroup related data
* @mode: mode of resource group
+ * @mba_mbps_event: input monitoring event id when mba_sc is enabled
* @plr: pseudo-locked region
*/
struct rdtgroup {
@@ -295,6 +296,7 @@ struct rdtgroup {
enum rdt_group_type type;
struct mongroup mon;
enum rdtgrp_mode mode;
+ enum resctrl_event_id mba_mbps_event;
struct pseudo_lock_region *plr;
};
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index f3ee5859b69d..94bf559966d6 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -963,6 +963,9 @@ static __init bool get_rdt_mon_resources(void)
if (!rdt_mon_features)
return false;
+ if (is_mbm_local_enabled())
+ mba_mbps_default_event = QOS_L3_MBM_LOCAL_EVENT_ID;
+
return !rdt_get_mon_l3_config(r);
}
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index d333570e893d..8a52b25ce26b 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -65,6 +65,15 @@ static void rdtgroup_destroy_root(void);
struct dentry *debugfs_resctrl;
+/*
+ * Memory bandwidth monitoring event to use for the default CTRL_MON group
+ * and each new CTRL_MON group created by the user. Only relevant when
+ * the filesystem is mounted with the "mba_MBps" option so it does not
+ * matter that it remains uninitialized on systems that do not support
+ * the "mba_MBps" option.
+ */
+enum resctrl_event_id mba_mbps_default_event;
+
static bool resctrl_debug;
void rdt_last_cmd_clear(void)
@@ -2353,6 +2362,8 @@ static int set_mba_sc(bool mba_sc)
r->membw.mba_sc = mba_sc;
+ rdtgroup_default.mba_mbps_event = mba_mbps_default_event;
+
list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
for (i = 0; i < num_closid; i++)
d->mbps_val[i] = MBA_MAX_MBPS;
@@ -3611,6 +3622,8 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
rdt_last_cmd_puts("kernfs subdir error\n");
goto out_del_list;
}
+ if (is_mba_sc(NULL))
+ rdtgrp->mba_mbps_event = mba_mbps_default_event;
}
goto out_unlock;
--
2.47.0On Fri, Dec 06, 2024 at 08:31:42AM -0800, Tony Luck wrote:
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index d94abba1c716..fd05b937e2f4 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -49,6 +49,8 @@ enum resctrl_event_id {
> QOS_L3_MBM_LOCAL_EVENT_ID = 0x03,
> };
>
> +extern enum resctrl_event_id mba_mbps_default_event;
Any reason this extern isn't in
arch/x86/kernel/cpu/resctrl/internal.h
?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
> > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> > index d94abba1c716..fd05b937e2f4 100644
> > --- a/include/linux/resctrl.h
> > +++ b/include/linux/resctrl.h
> > @@ -49,6 +49,8 @@ enum resctrl_event_id {
> > QOS_L3_MBM_LOCAL_EVENT_ID = 0x03,
> > };
> >
> > +extern enum resctrl_event_id mba_mbps_default_event;
>
> Any reason this extern isn't in
>
> arch/x86/kernel/cpu/resctrl/internal.h
Trying to avoid making more work for the ARM folks as they split resctrl
into "file system" and "architecture specific" bits.
mba_mbps_default_event isn't architecture specific. The mba_MBps
feedback code could be implemented on any architecture that supports
both measurement and control of memory bandwidth.
-Tony
On Mon, Dec 09, 2024 at 10:05:43PM +0000, Luck, Tony wrote:
> mba_mbps_default_event isn't architecture specific. The mba_MBps
> feedback code could be implemented on any architecture that supports
> both measurement and control of memory bandwidth.
Yes, and it should be moved to that header then, right?
But not earlier.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
> > mba_mbps_default_event isn't architecture specific. The mba_MBps > > feedback code could be implemented on any architecture that supports > > both measurement and control of memory bandwidth. > > Yes, and it should be moved to that header then, right? > > But not earlier. If you feel strongly about it then go ahead and cut the line from <linux/rectrl.h> and paste it into <asm/resctrl.h> -Tony
Hi Tony, On 12/9/24 2:35 PM, Luck, Tony wrote: >>> mba_mbps_default_event isn't architecture specific. The mba_MBps >>> feedback code could be implemented on any architecture that supports >>> both measurement and control of memory bandwidth. >> >> Yes, and it should be moved to that header then, right? >> >> But not earlier. > > If you feel strongly about it then go ahead and cut the line from <linux/rectrl.h> > and paste it into <asm/resctrl.h> I am not sure about this ... I expect the code needing this initially will form part of the filesystem code so it may be more intuitive to have it be located in arch/x86/kernel/cpu/resctrl/internal.h as Boris suggested. As part of the arch/fs split it may then move to fs/resctrl/internal.h mba_mbps_default_event may even stay internal to the fs/resctrl code with an arch helper created later to initialize it. This is because I think the initialization of mba_mbps_default_event may move out of get_rdt_mon_resources() into resctrl_mon_resource_init() that is being created as part of the MPAM work [1]. An example of current fs initialization done in arch code that is moved to it can be found in [2]. Reinette [1] https://lore.kernel.org/all/20241004180347.19985-17-james.morse@arm.com/ [2] https://lore.kernel.org/all/20241004180347.19985-20-james.morse@arm.com/
On Mon, Dec 09, 2024 at 03:40:12PM -0800, Reinette Chatre wrote: > Hi Tony, > > On 12/9/24 2:35 PM, Luck, Tony wrote: > >>> mba_mbps_default_event isn't architecture specific. The mba_MBps > >>> feedback code could be implemented on any architecture that supports > >>> both measurement and control of memory bandwidth. > >> > >> Yes, and it should be moved to that header then, right? > >> > >> But not earlier. > > > > If you feel strongly about it then go ahead and cut the line from <linux/rectrl.h> > > and paste it into <asm/resctrl.h> > > I am not sure about this ... I expect the code needing this initially will > form part of the filesystem code so it may be more intuitive to have it > be located in arch/x86/kernel/cpu/resctrl/internal.h as Boris suggested. > As part of the arch/fs split it may then move to fs/resctrl/internal.h > > mba_mbps_default_event may even stay internal to the fs/resctrl code with an > arch helper created later to initialize it. This is because I think > the initialization of mba_mbps_default_event may move out of > get_rdt_mon_resources() into resctrl_mon_resource_init() that is being > created as part of the MPAM work [1]. An example of current fs initialization > done in arch code that is moved to it can be found in [2]. Reinette is right. The post-split home of this is not <linux/resctrl.h> but fs/resctrl/internal.h which doesn't exist yet. So Boris is right, this declaration should be added to arch/x86/kernel/cpu/resctrl/internal.h by this patch to be moved later. > > [1] https://lore.kernel.org/all/20241004180347.19985-17-james.morse@arm.com/ > [2] https://lore.kernel.org/all/20241004180347.19985-20-james.morse@arm.com/ -Tony
On Mon, Dec 09, 2024 at 04:00:54PM -0800, Luck, Tony wrote:
> Reinette is right. The post-split home of this is not <linux/resctrl.h>
> but fs/resctrl/internal.h which doesn't exist yet.
>
> So Boris is right, this declaration should be added to arch/x86/kernel/cpu/resctrl/internal.h
> by this patch to be moved later.
Done:
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 485800055a7d..542d01c055aa 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -510,6 +510,7 @@ extern struct mutex rdtgroup_mutex;
extern struct rdt_hw_resource rdt_resources_all[];
extern struct rdtgroup rdtgroup_default;
extern struct dentry *debugfs_resctrl;
+extern enum resctrl_event_id mba_mbps_default_event;
enum resctrl_res_level {
RDT_RESOURCE_L3,
@@ -653,5 +654,4 @@ void resctrl_file_fflags_init(const char *config, unsigned long fflags);
void rdt_staged_configs_clear(void);
bool closid_allocated(unsigned int closid);
int resctrl_find_cleanest_closid(void);
-
#endif /* _ASM_X86_RESCTRL_INTERNAL_H */
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index fd05b937e2f4..d94abba1c716 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -49,8 +49,6 @@ enum resctrl_event_id {
QOS_L3_MBM_LOCAL_EVENT_ID = 0x03,
};
-extern enum resctrl_event_id mba_mbps_default_event;
-
/**
* struct resctrl_staged_config - parsed configuration to be applied
* @new_ctrl: new ctrl value to be loaded
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi Boris,
On 12/10/24 2:13 AM, Borislav Petkov wrote:
> On Mon, Dec 09, 2024 at 04:00:54PM -0800, Luck, Tony wrote:
>> Reinette is right. The post-split home of this is not <linux/resctrl.h>
>> but fs/resctrl/internal.h which doesn't exist yet.
>>
>> So Boris is right, this declaration should be added to arch/x86/kernel/cpu/resctrl/internal.h
>> by this patch to be moved later.
>
> Done:
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 485800055a7d..542d01c055aa 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -510,6 +510,7 @@ extern struct mutex rdtgroup_mutex;
> extern struct rdt_hw_resource rdt_resources_all[];
> extern struct rdtgroup rdtgroup_default;
> extern struct dentry *debugfs_resctrl;
> +extern enum resctrl_event_id mba_mbps_default_event;
>
> enum resctrl_res_level {
> RDT_RESOURCE_L3,
> @@ -653,5 +654,4 @@ void resctrl_file_fflags_init(const char *config, unsigned long fflags);
> void rdt_staged_configs_clear(void);
> bool closid_allocated(unsigned int closid);
> int resctrl_find_cleanest_closid(void);
> -
> #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index fd05b937e2f4..d94abba1c716 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -49,8 +49,6 @@ enum resctrl_event_id {
> QOS_L3_MBM_LOCAL_EVENT_ID = 0x03,
> };
>
> -extern enum resctrl_event_id mba_mbps_default_event;
> -
> /**
> * struct resctrl_staged_config - parsed configuration to be applied
> * @new_ctrl: new ctrl value to be loaded
>
Thank you very much for catching the issue and taking the time to create
the fix.
The middle hunk looks to be an unrelated change. I squashed the first and
last hunk of this change into the original patch and all my tests pass.
I am still waiting the 0day test report though.
What would work best for you to have this fix included?
Reinette
On Tue, Dec 10, 2024 at 10:42:33AM -0800, Reinette Chatre wrote:
> Thank you very much for catching the issue and taking the time to create
> the fix.
For nothing. :)
> The middle hunk looks to be an unrelated change.
Yeah, I zapped the superfluous newline while at it.
> I squashed the first and
> last hunk of this change into the original patch and all my tests pass.
> I am still waiting the 0day test report though.
>
> What would work best for you to have this fix included?
Already folded into Tony's patchset.
Will push out once I've gone through the set.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
The following commit has been merged into the x86/cache branch of tip:
Commit-ID: 3b49c37a2f4657730dd38a050b9d221363889dea
Gitweb: https://git.kernel.org/tip/3b49c37a2f4657730dd38a050b9d221363889dea
Author: Tony Luck <tony.luck@intel.com>
AuthorDate: Fri, 06 Dec 2024 08:31:42 -08:00
Committer: Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Tue, 10 Dec 2024 11:13:48 +01:00
x86/resctrl: Prepare for per-CTRL_MON group mba_MBps control
Resctrl uses local memory bandwidth event as input to the feedback loop when
the mba_MBps mount option is used. This means that this mount option cannot be
used on systems that only support monitoring of total bandwidth.
Prepare to allow users to choose the input event independently for each
CTRL_MON group by adding a global variable "mba_mbps_default_event" used to
set the default event for each CTRL_MON group, and a new field
"mba_mbps_event" in struct rdtgroup to track which event is used for each
CTRL_MON group.
Notes:
1) Both of these are only used when the user mounts the filesystem with the
"mba_MBps" option.
2) Only check for support of local bandwidth event when initializing
mba_mbps_default_event. Support for total bandwidth event can be added
after other routines in resctrl have been updated to handle total bandwidth
event.
[ bp: Move mba_mbps_default_event extern into the arch header. ]
Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Tested-by: Babu Moger <babu.moger@amd.com>
Link: https://lore.kernel.org/r/20241206163148.83828-3-tony.luck@intel.com
---
arch/x86/kernel/cpu/resctrl/core.c | 3 +++
arch/x86/kernel/cpu/resctrl/internal.h | 4 +++-
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 13 +++++++++++++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index f3ee585..94bf559 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -963,6 +963,9 @@ static __init bool get_rdt_mon_resources(void)
if (!rdt_mon_features)
return false;
+ if (is_mbm_local_enabled())
+ mba_mbps_default_event = QOS_L3_MBM_LOCAL_EVENT_ID;
+
return !rdt_get_mon_l3_config(r);
}
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index faaff9d..542d01c 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -283,6 +283,7 @@ struct pseudo_lock_region {
* monitor only or ctrl_mon group
* @mon: mongroup related data
* @mode: mode of resource group
+ * @mba_mbps_event: input monitoring event id when mba_sc is enabled
* @plr: pseudo-locked region
*/
struct rdtgroup {
@@ -295,6 +296,7 @@ struct rdtgroup {
enum rdt_group_type type;
struct mongroup mon;
enum rdtgrp_mode mode;
+ enum resctrl_event_id mba_mbps_event;
struct pseudo_lock_region *plr;
};
@@ -508,6 +510,7 @@ extern struct mutex rdtgroup_mutex;
extern struct rdt_hw_resource rdt_resources_all[];
extern struct rdtgroup rdtgroup_default;
extern struct dentry *debugfs_resctrl;
+extern enum resctrl_event_id mba_mbps_default_event;
enum resctrl_res_level {
RDT_RESOURCE_L3,
@@ -651,5 +654,4 @@ void resctrl_file_fflags_init(const char *config, unsigned long fflags);
void rdt_staged_configs_clear(void);
bool closid_allocated(unsigned int closid);
int resctrl_find_cleanest_closid(void);
-
#endif /* _ASM_X86_RESCTRL_INTERNAL_H */
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index d333570..8a52b25 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -65,6 +65,15 @@ static void rdtgroup_destroy_root(void);
struct dentry *debugfs_resctrl;
+/*
+ * Memory bandwidth monitoring event to use for the default CTRL_MON group
+ * and each new CTRL_MON group created by the user. Only relevant when
+ * the filesystem is mounted with the "mba_MBps" option so it does not
+ * matter that it remains uninitialized on systems that do not support
+ * the "mba_MBps" option.
+ */
+enum resctrl_event_id mba_mbps_default_event;
+
static bool resctrl_debug;
void rdt_last_cmd_clear(void)
@@ -2353,6 +2362,8 @@ static int set_mba_sc(bool mba_sc)
r->membw.mba_sc = mba_sc;
+ rdtgroup_default.mba_mbps_event = mba_mbps_default_event;
+
list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
for (i = 0; i < num_closid; i++)
d->mbps_val[i] = MBA_MAX_MBPS;
@@ -3611,6 +3622,8 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
rdt_last_cmd_puts("kernfs subdir error\n");
goto out_del_list;
}
+ if (is_mba_sc(NULL))
+ rdtgrp->mba_mbps_event = mba_mbps_default_event;
}
goto out_unlock;© 2016 - 2025 Red Hat, Inc.