Re: [PATCH v3 07/12] fs/resctrl: Add info/kernel_mode for kernel-mode policy introspection

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

Hi Reinette,

On 6/16/26 18:38, Reinette Chatre wrote:
> Hi Babu,
> 
> How should "introspection" as used in subject be interpreted? This just
> displays the supported and active kernel modes to user space, no?

Yes. Will change it.

> 
> On 4/30/26 4:24 PM, Babu Moger wrote:
>> There is no user-visible way today to see which kernel-mode CLOSID/RMID
>> policies the running kernel supports, which one is active, or which
>> resctrl group currently owns the kernel CLOSID/RMID.
> 
> Why should there be? This is a new feature being added in this series.
> No need to write this as a bugfix.
> 

Sure.

>>
>> Add a read-only top-level sysfs file, info/kernel_mode.  It emits one
>> line per mode advertised in resctrl_kcfg.kmode, in stable lowercase
>> spelling derived from enum resctrl_kernel_modes, e.g.:
> 
> All these changelogs feel so strange ... as though they are written by
> somebody who simultaneously has no and full knowledge of resctrl.
> These verbatim descriptions of what the code does is not necessary. Please
> start with why the patch is needed.

Sure. My bad. Will re-write it.

> 
>>
>>    [inherit_ctrl_and_mon:group=//]
> 
> This is unexpected. There should be no group associated with this default mode.
> This is how I interpreted our previous discussion ending:
> https://lore.kernel.org/lkml/6709398b-269d-47b5-9b41-084f410bb1a6@amd.com/

Ack.

> 
>>    global_assign_ctrl_inherit_mon_per_cpu:group=none
>>    global_assign_ctrl_assign_mon_per_cpu:group=none
>>
>> The effective policy (resctrl_kcfg.kmode_cur) is wrapped in square
> 
> (needs imperative - please check all changelogs)

Sure.

> 
>> brackets and its :group= suffix names the resctrl group currently
>> bound to the kernel CLOSID/RMID (resctrl_kcfg.k_rdtgrp), formatted as
>> <ctrl>/<mon>/ with empty components left blank.  Inactive modes are
>> reported as :group=none.
>>
>> rdtgroup_mutex is held while printing, matching other info/ show paths.
> 
> No need to describe details that can be seen from patch.

ok.

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v3: New patch to handle the changed interface file info/kernel_mode.
>>      Changed the group name to "none" if kmode binding is not done.
>>      Reinette suggested "uninitialized". "none" seemed more relevent.
>> ---
>>   fs/resctrl/rdtgroup.c | 74 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index a7bfc74897cc..9cdcfa64c4a2 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -988,6 +988,73 @@ static int rdt_last_cmd_status_show(struct kernfs_open_file *of,
>>   	return 0;
>>   }
>>   
>> +/* Sysfs lines for info/kernel_mode; indexed by &enum resctrl_kernel_modes */
>> +static const char * const resctrl_mode_str[] = {
>> +	[INHERIT_CTRL_AND_MON]			= "inherit_ctrl_and_mon",
>> +	[GLOBAL_ASSIGN_CTRL_INHERIT_MON_PER_CPU] = "global_assign_ctrl_inherit_mon_per_cpu",
>> +	[GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU]	= "global_assign_ctrl_assign_mon_per_cpu",
> 
> Please make alignment consistent.
> 

Sure.

>> +};
>> +
>> +static_assert(ARRAY_SIZE(resctrl_mode_str) == RESCTRL_NUM_KERNEL_MODES);
>> +
>> +/**
>> + * resctrl_kernel_mode_show() - Enumerate supported and effective kernel-mode policies
> 
> "Enumerate" -> "Display"?

sure.

> 
>> + * @of: kernfs open file
>> + * @seq: output seq_file
>> + * @v: unused
>> + *
>> + * Emits one line per mode advertised in resctrl_kcfg.kmode (each mode is one
>> + * BIT(index) per &enum resctrl_kernel_modes).  Every line carries a
> 
> Above is clear from the code. Please instead describe what this means.

Sure.
> 
>> + * ":group=<name>" suffix:
>> + *
>> + *   - The effective policy (whose BIT matches resctrl_kcfg.kmode_cur) is
>> + *     wrapped in square brackets and <name> is the resctrl group that
>> + *     currently owns the kernel CLOSID/RMID (resctrl_kcfg.k_rdtgrp),
>> + *     formatted as "<ctrl>/<mon>/".  A component is left empty when it
>> + *     does not apply: an RDTCTRL_GROUP emits "<ctrl>//", an RDTMON_GROUP
>> + *     under the default control group emits "/<mon>/", and an RDTMON_GROUP
>> + *     under a named control group emits "<ctrl>/<mon>/".
>> + *
>> + *   - Other supported but inactive modes are emitted without brackets and
>> + *     <name> is reported as "none".
>> + *
>> + * Context: Called under rdtgroup_mutex like other resctrl sysfs show paths.
> 
> This does not look accurate since it is not called with mutex held but instead
> takes the mutex itself. Also no need to refer to what other code does.

ok.

> 
>> + */
>> +static int resctrl_kernel_mode_show(struct kernfs_open_file *of,
>> +				    struct seq_file *seq, void *v)
>> +{
>> +	struct rdtgroup *rdtgrp;
>> +	const char *ctrl, *mon;
>> +	int i;
>> +
>> +	mutex_lock(&rdtgroup_mutex);
>> +	for (i = 0; i < RESCTRL_NUM_KERNEL_MODES; i++) {
>> +		if (!(resctrl_kcfg.kmode & BIT(i)))
>> +			continue;
>> +
>> +		if (resctrl_kcfg.kmode_cur != BIT(i)) {
>> +			seq_printf(seq, "%s:group=none\n",
>> +				   resctrl_mode_str[i]);
>> +			continue;
>> +		}
>> +
>> +		rdtgrp = resctrl_kcfg.k_rdtgrp;
>> +		ctrl = "";
>> +		mon = "";
>> +		if (rdtgrp->type == RDTMON_GROUP) {
>> +			if (rdtgrp->mon.parent != &rdtgroup_default)
>> +				ctrl = rdtgrp->mon.parent->kn->name;
> 
> Isn't default group's kn->name is initialized correctly via
> rdtgroup_setup_root()->kernfs_create_root()->__kernfs_new_node(root, NULL, "", ...) ?

Yes. that is correct. I will remove the check.

> 
>> +			mon = rdtgrp->kn->name;
>> +		} else {
>> +			ctrl = rdtgrp->kn->name;
>> +		}
> 
> Can the names not just be initialized directly from kn->name?

Yes. I think so. But I need to know if this is a control group or mon 
group to make it generic. Let me see if I can optimize this section.

> 
> 
>> +		seq_printf(seq, "[%s:group=%s/%s/]\n",
>> +			   resctrl_mode_str[i], ctrl, mon);
> 
> This is not where I understood our discussion landed. I expected that the display will
> reflect what can/should be assigned in a mode. For example, mode "inherit_ctrl_and_mon"
> does not have an associated resource group and should thus not display one,

Correct.

> "global_assign_ctrl_inherit_mon_per_cpu" can only be assigned a control group and
> should thus not display a monitor group also.

Yes. True. In that case "mon" is empty. It will print correctly.  Let me 
see if I can optimize this section.

Thanks
Babu