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

["Moger, Babu" <bmoger@amd.com>]

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