Re: [PATCH v14 30/32] fs/resctrl: Hide the BMEC related files when mbm_event mode is enabled

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

Hi Reinette,


On 6/25/25 18:39, Reinette Chatre wrote:
> Hi Babu,
> 
> On 6/13/25 2:05 PM, Babu Moger wrote:
>> BMEC (Bandwidth Monitoring Event Configuration) and mbm_event mode do not
>> work simultaneously.
> 
> Could you please elaborate why they do not work simultaneously?

Changed the changelog.

When mbm_event counter assignment mode is enabled, events are configured
through the "event_filter" files under
/sys/fs/resctrl/info/L3_MON/event_configs/.

The default monitoring mode and with BMEC (Bandwidth Monitoring Event
Configuration) support, events are configured using the files
mbm_total_bytes_config or mbm_local_bytes_config in
/sys/fs/resctrl/info/L3_MON/.

To avoid the confusion, hide BMEC-related files when mbm_event counter
assignment mode is enabled and update the mon_features display accordingly.

> 
>>
>> When mbm_event mode is enabled, hide BMEC-related files to avoid confusion
>> and update the mon_features display accordingly.
>>
>> The files /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config and
>> /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config will not be visible
>> when mbm_event mode is enabled.
>>
>> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> 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 | 42 +++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index 92bb8f3adfae..8c67e0897f25 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -1164,7 +1164,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);
>>  	}
>>  
>> @@ -1813,6 +1814,38 @@ 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);
> 
> Similar to comment about resctrl_mkdir_counter_configs() (resctrl_mkdir_event_configs())
> I think this can be avoided by calling resctrl_bmec_files_show() from 
> rdtgroup_mkdir_info_resdir().

Sure.

> 
>> +	if (!l3_mon_kn)
>> +		return;
>> +
>> +	kn_config = kernfs_find_and_get(l3_mon_kn, "mbm_total_bytes_config");
>> +	if (kn_config) {
>> +		kernfs_get(kn_config);
> 
> Be careful ... kernfs_find_and_get() already took a reference. Additional
> kernfs_get() is not needed.

Sure.

> 
>> +		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_get(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)
>>  {
>> @@ -2808,6 +2841,13 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
>>  			ret = resctrl_mkdir_counter_configs(r, name);
>>  			if (ret)
>>  				goto out_destroy;
>> +
>> +			/*
>> +			 * Hide BMEC related files if mbm_event mode
>> +			 * is enabled.
>> +			 */
>> +			if (resctrl_arch_mbm_cntr_assign_enabled(r))
>> +				resctrl_bmec_files_show(r, false);
>>  		}
>>  	}
>>  
> 
> Reinette
> 

-- 
Thanks
Babu Moger