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

[Reinette Chatre <reinette.chatre@intel.com>]

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