[PATCH v15 32/34] fs/resctrl: Disable BMEC event configuration when mbm_event mode is enabled

Babu Moger posted 34 patches 3 months ago
[PATCH v15 32/34] fs/resctrl: Disable BMEC event configuration when mbm_event mode is enabled
Posted by Babu Moger 3 months ago
The BMEC (Bandwidth Monitoring Event Configuration) feature enables
per-domain event configuration. With BMEC the MBM events are configured
using the mbm_total_bytes_config or mbm_local_bytes_config files in
/sys/fs/resctrl/info/L3_MON/ and the per-domain event configuration affects
all monitor resource groups.

The mbm_event counter assignment mode enables counters to be assigned to
RMID (i.e a monitor resource group), event pairs, with potentially unique
event configurations associated with every counter.

There may be systems that support both BMEC and mbm_event counter
assignment mode, but resctrl supporting both concurrently will present a
conflicting interface to the user with both per-domain and per RMID, event
configurations active at the same time.

The mbm_event counter assignment provides most flexibility to user space
and aligns with Arm's counter support. On systems that support both,
disable BMEC event configuration when mbm_event mode is enabled by hiding
the mbm_total_bytes_config or mbm_local_bytes_config files when mbm_event
mode is enabled. Ensure mon_features always displays accurate information
about monitor features.

Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v15: Updated the changelog.
     Moved resctrl_bmec_files_show() inside rdtgroup_mkdir_info_resdir().
     Removed the unnecessary kernfs_get() call.

v14: Updated the changelog for change in mbm_assign_modes.
     Added check in rdt_mon_features_show to hide bmec related feature.

v13: New patch to hide BMEC related files.
---
 fs/resctrl/rdtgroup.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index b26baca389bb..d73057b96d1d 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -1152,7 +1152,8 @@ static int rdt_mon_features_show(struct kernfs_open_file *of,
 		if (mevt->rid != r->rid || !mevt->enabled)
 			continue;
 		seq_printf(seq, "%s\n", mevt->name);
-		if (mevt->configurable)
+		if (mevt->configurable &&
+		    !resctrl_arch_mbm_cntr_assign_enabled(r))
 			seq_printf(seq, "%s_config\n", mevt->name);
 	}
 
@@ -1801,6 +1802,36 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
 	return ret ?: nbytes;
 }
 
+/*
+ * resctrl_bmec_files_show() — Controls the visibility of BMEC-related resctrl
+ * files. When @show is true, the files are displayed; when false, the files
+ * are hidden.
+ */
+static void resctrl_bmec_files_show(struct rdt_resource *r, bool show)
+{
+	struct kernfs_node *kn_config, *l3_mon_kn;
+	char name[32];
+
+	sprintf(name, "%s_MON", r->name);
+	l3_mon_kn = kernfs_find_and_get(kn_info, name);
+	if (!l3_mon_kn)
+		return;
+
+	kn_config = kernfs_find_and_get(l3_mon_kn, "mbm_total_bytes_config");
+	if (kn_config) {
+		kernfs_show(kn_config, show);
+		kernfs_put(kn_config);
+	}
+
+	kn_config = kernfs_find_and_get(l3_mon_kn, "mbm_local_bytes_config");
+	if (kn_config) {
+		kernfs_show(kn_config, show);
+		kernfs_put(kn_config);
+	}
+
+	kernfs_put(l3_mon_kn);
+}
+
 static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of,
 					struct seq_file *s, void *v)
 {
@@ -2659,6 +2690,12 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
 			ret = resctrl_mkdir_event_configs(r, kn_subdir);
 			if (ret)
 				return ret;
+			/*
+			 * Hide BMEC related files if mbm_event mode
+			 * is enabled.
+			 */
+			if (resctrl_arch_mbm_cntr_assign_enabled(r))
+				resctrl_bmec_files_show(r, false);
 		}
 	}
 
-- 
2.34.1

Re: [PATCH v15 32/34] fs/resctrl: Disable BMEC event configuration when mbm_event mode is enabled
Posted by Reinette Chatre 2 months, 3 weeks ago
Hi Babu,

On 7/8/25 3:17 PM, Babu Moger wrote:
>  
> +/*
> + * resctrl_bmec_files_show() — Controls the visibility of BMEC-related resctrl
> + * files. When @show is true, the files are displayed; when false, the files
> + * are hidden.
> + */
> +static void resctrl_bmec_files_show(struct rdt_resource *r, bool show)

The "void" return is unexpected since many of the calls can fail. It looks to me
that this is indeed intentional since there is no BMEC feature checking, but instead
the existence of the resctrl files are implicitly used as feature check. Doing so enables
this code to be called on any system whether BMEC is supported or not and thus a failure
should not be considered an actual failure. If this is the case, then it is subtle so please
add this information to the function's comments. If this is not the case, could you
please explain the void return?

> +{
> +	struct kernfs_node *kn_config, *l3_mon_kn;
> +	char name[32];
> +
> +	sprintf(name, "%s_MON", r->name);
> +	l3_mon_kn = kernfs_find_and_get(kn_info, name);
> +	if (!l3_mon_kn)
> +		return;
> +
> +	kn_config = kernfs_find_and_get(l3_mon_kn, "mbm_total_bytes_config");
> +	if (kn_config) {
> +		kernfs_show(kn_config, show);
> +		kernfs_put(kn_config);
> +	}
> +
> +	kn_config = kernfs_find_and_get(l3_mon_kn, "mbm_local_bytes_config");
> +	if (kn_config) {
> +		kernfs_show(kn_config, show);
> +		kernfs_put(kn_config);
> +	}
> +
> +	kernfs_put(l3_mon_kn);
> +}
> +
>  static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of,
>  					struct seq_file *s, void *v)
>  {
> @@ -2659,6 +2690,12 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
>  			ret = resctrl_mkdir_event_configs(r, kn_subdir);
>  			if (ret)
>  				return ret;
> +			/*
> +			 * Hide BMEC related files if mbm_event mode
> +			 * is enabled.
> +			 */
> +			if (resctrl_arch_mbm_cntr_assign_enabled(r))
> +				resctrl_bmec_files_show(r, false);

Looks like the kernfs_find_and_get(kn_info, name) in resctrl_bmec_files_show()
can be avoided by providing the kn as parameter. I think you may be doing it like
this because of how resctrl_bmec_files_show() is used in resctrl_mbm_assign_mode_write()
in the next patch. I think the kn can still be provided as parameter but 
resctrl_mbm_assign_mode_write() will set it to NULL and only then does 
resctrl_bmec_files_show() need to figure it out itself.

>  		}
>  	}
>  

Reinette
Re: [PATCH v15 32/34] fs/resctrl: Disable BMEC event configuration when mbm_event mode is enabled
Posted by Moger, Babu 2 months, 2 weeks ago
Hi Reinette,

On 7/17/25 23:02, Reinette Chatre wrote:
> Hi Babu,
> 
> On 7/8/25 3:17 PM, Babu Moger wrote:
>>  
>> +/*
>> + * resctrl_bmec_files_show() — Controls the visibility of BMEC-related resctrl
>> + * files. When @show is true, the files are displayed; when false, the files
>> + * are hidden.
>> + */
>> +static void resctrl_bmec_files_show(struct rdt_resource *r, bool show)
> 
> The "void" return is unexpected since many of the calls can fail. It looks to me
> that this is indeed intentional since there is no BMEC feature checking, but instead
> the existence of the resctrl files are implicitly used as feature check. Doing so enables
> this code to be called on any system whether BMEC is supported or not and thus a failure
> should not be considered an actual failure. If this is the case, then it is subtle so please
> add this information to the function's comments. If this is not the case, could you
> please explain the void return?

Yes. Returned void because in some cases one of the config files may not
be available. Added a comment now.

"Don't treat kernfs_find_and_get failure as an error, since this function
may be called regardless of whether BMEC is supported or the event is
enabled."


> 
>> +{
>> +	struct kernfs_node *kn_config, *l3_mon_kn;
>> +	char name[32];
>> +
>> +	sprintf(name, "%s_MON", r->name);
>> +	l3_mon_kn = kernfs_find_and_get(kn_info, name);
>> +	if (!l3_mon_kn)
>> +		return;
>> +
>> +	kn_config = kernfs_find_and_get(l3_mon_kn, "mbm_total_bytes_config");
>> +	if (kn_config) {
>> +		kernfs_show(kn_config, show);
>> +		kernfs_put(kn_config);
>> +	}
>> +
>> +	kn_config = kernfs_find_and_get(l3_mon_kn, "mbm_local_bytes_config");
>> +	if (kn_config) {
>> +		kernfs_show(kn_config, show);
>> +		kernfs_put(kn_config);
>> +	}
>> +
>> +	kernfs_put(l3_mon_kn);
>> +}
>> +
>>  static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of,
>>  					struct seq_file *s, void *v)
>>  {
>> @@ -2659,6 +2690,12 @@ static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
>>  			ret = resctrl_mkdir_event_configs(r, kn_subdir);
>>  			if (ret)
>>  				return ret;
>> +			/*
>> +			 * Hide BMEC related files if mbm_event mode
>> +			 * is enabled.
>> +			 */
>> +			if (resctrl_arch_mbm_cntr_assign_enabled(r))
>> +				resctrl_bmec_files_show(r, false);
> 
> Looks like the kernfs_find_and_get(kn_info, name) in resctrl_bmec_files_show()
> can be avoided by providing the kn as parameter. I think you may be doing it like
> this because of how resctrl_bmec_files_show() is used in resctrl_mbm_assign_mode_write()
> in the next patch. I think the kn can still be provided as parameter but 
> resctrl_mbm_assign_mode_write() will set it to NULL and only then does 
> resctrl_bmec_files_show() need to figure it out itself.

Sure. Passed kernfs_node now. If the node is NULL then use
kernfs_find_and_get() to get the pointer to the node.

> 
>>  		}
>>  	}
>>  
> 
> Reinette
> 

-- 
Thanks
Babu Moger