Re: [PATCH v8 1/7] x86/resctrl: Prepare for per-ctrl_mon group mba_MBps control

[Fenghua Yu <fenghua.yu@intel.com>]

Hi, Tony,

On 10/29/24 10:28, 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.
> 
> 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     | 5 +++++
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 6 ++++++
>   4 files changed, 15 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 955999aecfca..a6f051fb2e69 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 event id when mba_sc mode is active
>    * @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 b681c2e07dbf..5b55a7ac7013 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -958,6 +958,11 @@ static __init bool get_rdt_mon_resources(void)
>   	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
>   		rdt_mon_features |= (1 << QOS_L3_MBM_LOCAL_EVENT_ID);
>   
> +	if (rdt_mon_features & (1 << QOS_L3_MBM_LOCAL_EVENT_ID))

Please change this check to:

	if (is_mbm_local_enabled())

> +		mba_mbps_default_event = QOS_L3_MBM_LOCAL_EVENT_ID;
> +	else if (rdt_mon_features & (1 << QOS_L3_MBM_TOTAL_EVENT_ID))

and this check to:

	else if (is_mbm_total_enabled())

> +		mba_mbps_default_event = QOS_L3_MBM_TOTAL_EVENT_ID;
> +
>   	if (!rdt_mon_features)
>   		return false;
>   
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index d7163b764c62..dbfb9d11f3f8 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)
> @@ -2665,6 +2667,8 @@ static int rdt_get_tree(struct fs_context *fc)
>   	if (ret)
>   		goto out_schemata_free;
>   
> +	rdtgroup_default.mba_mbps_event = mba_mbps_default_event;
> +
>   	kernfs_activate(rdtgroup_default.kn);
>   
>   	ret = rdtgroup_create_info_dir(rdtgroup_default.kn);
> @@ -3624,6 +3628,8 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
>   		}
>   	}
>   
> +	rdtgrp->mba_mbps_event = mba_mbps_default_event;
> +
>   	goto out_unlock;
>   
>   out_del_list:

Thanks.

-Fenghua