Re: [PATCH v9 2/9] x86/resctrl: Prepare for per-ctrl_mon group mba_MBps control

Moger, Babu
2024-11-15T16:20:40Z

Hi Tony,

On 11/13/2024 6:17 PM, Tony Luck wrote:
> 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.

How about this?

Provide users with the ability to select the input event independently
for each ctrl_mon group.

>
> Signed-off-by: Tony Luck <tony.luck@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 | 6 ++++++
> 4 files changed, 13 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;

Any reason to separate this patch and patch 8? I feel it can be combined.

> +
> 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 2b198ef95e1e..a8022bddf9f7 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -65,6 +65,8 @@ static void rdtgroup_destroy_root(void);
>
> struct dentry *debugfs_resctrl;
>
> +enum resctrl_event_id mba_mbps_default_event;
> +
> static bool resctrl_debug;
>
> void rdt_last_cmd_clear(void)
> @@ -3611,6 +3613,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;
> @@ -3970,6 +3974,8 @@ static void __init rdtgroup_setup_default(void)
> rdtgroup_default.closid = RESCTRL_RESERVED_CLOSID;
> rdtgroup_default.mon.rmid = RESCTRL_RESERVED_RMID;
> rdtgroup_default.type = RDTCTRL_GROUP;
> + if (supports_mba_mbps())
> + rdtgroup_default.mba_mbps_event = mba_mbps_default_event;
> INIT_LIST_HEAD(&rdtgroup_default.mon.crdtgrp_list);
>
> list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);

--
- Babu Moger