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

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

Hi Babu,

How should "introspection" as used in subject be interpreted? This just
displays the supported and active kernel modes to user space, no?

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.

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

> 
>   [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/

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

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

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

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

"Enumerate" -> "Display"?

> + * @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.

> + * ":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.

> + */
> +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, "", ...) ?

> +			mon = rdtgrp->kn->name;
> +		} else {
> +			ctrl = rdtgrp->kn->name;
> +		}

Can the names not just be initialized directly from kn->name?


> +		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, 
"global_assign_ctrl_inherit_mon_per_cpu" can only be assigned a control group and
should thus not display a monitor group also.

> +	}
> +	mutex_unlock(&rdtgroup_mutex);
> +	return 0;
> +}
> +
>  void *rdt_kn_parent_priv(struct kernfs_node *kn)
>  {
>  	/*
> @@ -1891,6 +1958,13 @@ static struct rftype res_common_files[] = {
>  		.seq_show	= rdt_last_cmd_status_show,
>  		.fflags		= RFTYPE_TOP_INFO,
>  	},
> +	{
> +		.name		= "kernel_mode",
> +		.mode		= 0444,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.seq_show	= resctrl_kernel_mode_show,
> +		.fflags		= RFTYPE_TOP_INFO,
> +	},
>  	{
>  		.name		= "mbm_assign_on_mkdir",
>  		.mode		= 0644,

Reinette