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

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

Hi Reinette,

On 10/15/26 10:46, Reinette Chatre wrote:
> 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.

"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 a resource group backs the global per-CPU 
kernel mode. Hide the files when the group is no longer associated with 
the mode.

This avoids exposing kernel mode configuration files in resource groups 
where they do not apply."


> 
> 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.
> 

Sure.

> 
>>
>> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> 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@intel.com/
>> ---
>>   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)

Sure. Will do.

Thanks
Babu