Re: [PATCH v6 14/18] fs/resctrl: Control visibility of global per-CPU kernel mode group files

[Reinette Chatre]

Hi Babu,

On 10/1/26 8:10 AM, Babu Moger wrote:
> The kmode_cpus and kmode_cpus_list files are specific to the
> RESCTRL_GLOBAL_ENABLE_PER_CPU kernel mode. These files are created in a
> hidden state when an rdtgroup is created.
> 
> Introduce resctrl_kmode_global_files_show() to manage the visibility of
> these files based on whether the global per-CPU kernel mode is enabled or
> disabled.

This does not sound right and does not match the patch. The visibility of files
is per group, not globally whether the mode is enabled or not, right?

It is ok to have cryptic changelogs when changes are small and simple but these
changes are not small and simple.

Sidenote (on topic of consistent terminology): Please stick to "activate"/"deactivate"
for the mode itself and use "enable"/"disable" when mode is enabled/disabled per-CPU.


> 
> Suggested-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> ---
> v6: Helper named for the global per-CPU files instead of unhiding every
>     hidden file on the group.
>     https://lore.kernel.org/lkml/34aa4824-810b-4c81-8188-f2f37e1dc277@xxxxxxxxx/
> ---
>  fs/resctrl/rdtgroup.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 0cdc868a55b1..98451b417fe3 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -1125,6 +1125,24 @@ static int resctrl_kernel_mode_show(struct kernfs_open_file *of,
>  	return 0;
>  }
>  
> +/* Show or hide kmode_cpus and kmode_cpus_list on @kn. */
> +static void resctrl_kmode_global_files_show(struct kernfs_node *kn, bool show)

I think this can be simplified by changing the signature to:
	 rdtgroup_kmode_global_files_show(struct rdtgroup *rdtgrp, bool show)

> +{
> +	struct kernfs_node *kn_file;
> +
> +	kn_file = kernfs_find_and_get(kn, "kmode_cpus");
> +	if (kn_file) {
> +		kernfs_show(kn_file, show);
> +		kernfs_put(kn_file);
> +	}
> +
> +	kn_file = kernfs_find_and_get(kn, "kmode_cpus_list");
> +	if (kn_file) {
> +		kernfs_show(kn_file, show);
> +		kernfs_put(kn_file);
> +	}
> +}
> +
>  /*
>   * resctrl_kmode_global_online_cpu() - Program kernel mode association for @cpu
>   * @cpu: CPU that has just been brought online
> @@ -1175,6 +1193,7 @@ static void rdtgroup_kmode_global_deactivate(struct rdtgroup *rdtgrp)
>  {
>  	resctrl_configure_kmode_global(&rdtgrp->kmode_cpu_mask, rdtgrp, false);
>  	cpumask_clear(&rdtgrp->kmode_cpu_mask);
> +	resctrl_kmode_global_files_show(rdtgrp->kn, false);
>  }
>  
>  /*

Reinette