Re: [PATCH v6 11/18] fs/resctrl: Introduce kmode_cpus/kmode_cpus_list per rdtgroup

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

Hi Babu,

On 10/1/26 8:10 AM, Babu Moger wrote:
> The RESCTRL_GLOBAL_ENABLE_PER_CPU kernel mode allows tasks to use different
> allocation and monitoring associations when transitioning from user mode to

Could "different allocation and monitoring associations" just be "different
CLOSID and RMID" (in places where this is what it refers to)? Being specific
and consistent with terms would make this so much easier to understand.

> kernel mode. The mode is enabled/disabled on a per-CPU basis.
> 
> Add kmode_cpus and kmode_cpus_list to display the CPUs associated with a
> kernel mode. These files are exposed only when the global_enable_per_cpu


"a kernel mode" -> this is specific to RESCTRL_GLOBAL_ENABLE_PER_CPU kernel
mode node?

This switches from "RESCTRL_GLOBAL_ENABLE_PER_CPU kernel mode" to 
"global_enable_per_cpu mode" mid changelog.

> mode is active.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

...> ---
>  fs/resctrl/internal.h |  2 ++
>  fs/resctrl/rdtgroup.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> index 986b9c942fb2..6b8d14c4bd83 100644
> --- a/fs/resctrl/internal.h
> +++ b/fs/resctrl/internal.h
> @@ -216,6 +216,7 @@ struct mongroup {
>   * @mon:			mongroup related data
>   * @mode:			mode of resource group
>   * @mba_mbps_event:		input monitoring event id when mba_sc is enabled
> + * @kmode_cpu_mask:		CPUs in this group's kernel-mode association

Could there be a caveat that the value is only valid when the resource group
is backing the RESCTRL_GLOBAL_ENABLE_PER_CPU kernel mode?

>   * @plr:			pseudo-locked region
>   */
>  struct rdtgroup {
> @@ -229,6 +230,7 @@ struct rdtgroup {
>  	struct mongroup			mon;
>  	enum rdtgrp_mode		mode;
>  	enum resctrl_event_id		mba_mbps_event;
> +	struct cpumask			kmode_cpu_mask;
>  	struct pseudo_lock_region	*plr;
>  };
>  
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 46b2cafd9411..0e638dc654c1 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -385,6 +385,31 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
>  	return ret;
>  }
>  
> +/*
> + * Display CPU masks for the kernel-mode associated resource group.

nit: "masks for" -> "mask of"?

> + * Supports both "kmode_cpus" (bitmap format) and "kmode_cpus_list"
> + * (range list format); the output format is selected accordingly.
> + *
> + * Returns -ENOENT on error.
> + */
> +static int rdtgroup_kmode_cpus_show(struct kernfs_open_file *of,
> +				    struct seq_file *s, void *v)
> +{
> +	struct rdtgroup *rdtgrp;
> +	int ret = 0;
> +
> +	rdtgrp = rdtgroup_kn_lock_live(of->kn);
> +
> +	if (!rdtgrp || rdtgrp != resctrl_kmode.rdtgrp)

Could the changelog please help reader understand the patches better?
For example, the additional "rdtgrp != resctrl_kmode.rdtgrp" check is
subtle but very important to maintain synchronization, but it is
never mentioned why this is used. 

> +		ret = -ENOENT;
> +	else
> +		seq_printf(s, is_cpu_list(of) ? "%*pbl\n" : "%*pb\n",
> +			   cpumask_pr_args(&rdtgrp->kmode_cpu_mask));
> +	rdtgroup_kn_unlock(of->kn);
> +
> +	return ret;
> +}
> +
>  /*
>   * Update the PGR_ASSOC MSR on all cpus in @cpu_mask,
>   *
> @@ -2256,6 +2281,23 @@ static struct rftype res_common_files[] = {
>  		.flags		= RFTYPE_FLAGS_CPUS_LIST,
>  		.fflags		= RFTYPE_BASE,
>  	},
> +	{
> +		.name		= "kmode_cpus",
> +		.mode		= 0444,
> +		.hidden		= true,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.seq_show	= rdtgroup_kmode_cpus_show,
> +		.fflags		= RFTYPE_BASE,
> +	},
> +	{
> +		.name		= "kmode_cpus_list",
> +		.mode		= 0444,
> +		.hidden		= true,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.seq_show	= rdtgroup_kmode_cpus_show,
> +		.flags		= RFTYPE_FLAGS_CPUS_LIST,
> +		.fflags		= RFTYPE_BASE,
> +	},
>  	{
>  		.name		= "tasks",
>  		.mode		= 0644,

Reinette