Re: [RESEND PATCH v4 06/15] fs/resctrl: Introduce resctrl_set_kmode_support() to initialize supported modes

["Moger, Babu" <bmoger@amd.com>]

Hi Reinette,

On 8/10/2026 10:16 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 7/7/26 2:50 PM, Babu Moger wrote:
>> Architectures need a way to advertise which kernel-mode resctrl policies
> 
> "kernel-mode resctrl policies" -> "kernel modes"?

Sure.

> 
>> they support before generic resctrl exposes those policies through the
> 
> "policies" -> "modes"
> (this is related to the earlier "use consistent terminology comment" I
> will stop highlighting instances)

Sure.

> 
>> filesystem.
>>
>> Add the `struct resctrl_kmode_cfg` to define the default mode statically,
>> and introduce `resctrl_set_kmode_support()` so architecture code can
>> register supported kernel-mode policies during resctrl initialization. The
>> `INHERIT_CTRL_AND_MON` policy is supported by default, so architectures
>> only need to register any additional modes.
> 
> Please keep changelog high level and not describe the code that can be seen from patch.
> When doing so, please write in imperative tone.

Yes.

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v4: New patch to initialize supported kernel-mode's.
>>      https://lore.kernel.org/lkml/737a4228-52fb-4583-ac64-8efe79c107e6@intel.com/
>>      Changed the kmode_cur to enum resctrl_kernel_mode in resctrl_kmode_cfg.
>>      Moved the resctrl_kmode_cfg to filesystem code.
>> ---
>>   fs/resctrl/internal.h   | 12 ++++++++++++
>>   fs/resctrl/rdtgroup.c   | 23 +++++++++++++++++++++++
>>   include/linux/resctrl.h | 10 ++++++++++
>>   3 files changed, 45 insertions(+)
>>
>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>> index 48af75b9dc85..178126bb2da5 100644
>> --- a/fs/resctrl/internal.h
>> +++ b/fs/resctrl/internal.h
>> @@ -314,6 +314,18 @@ struct mbm_state {
>>   	u32	prev_bw;
>>   };
>>   
>> +/**
>> + * struct resctrl_kmode_cfg - Kernel-mode policy snapshot from architecture
> "policy snapshot"? what does this mean? This is not just from architecture either.

  struct resctrl_kmode_cfg - Resctrl kernel mode support and active state

> 
>> + * @kmode:	Bitmap of supported &enum resctrl_kernel_mode values. Each
>> + *		supported mode is represented by BIT(mode).
>> + * @kmode_cur:	Currently selected kernel-mode policy.
> 
> How about "Active kernel mode"?

Sure.

> 
>> + * @k_rdtgrp:	Resource group backing global-assign modes when applicable;
> 
> (stray semi-colon)

Ack.

> 
>> + */
>> +struct resctrl_kmode_cfg {
>> +	unsigned long			kmode;
> 
> How about "kmode_sup" to complement the "kmode_cur"?

Sure.

> 
> To make it obvious this is a bitmap you can use something like:
> 	DECLARE_BITMAP(kmode_sup, RESCTRL_NUM_KERNEL_MODES)

Sure. If the code remains same.

> 
> 
>> +	enum resctrl_kernel_mode	kmode_cur;
>> +	struct rdtgroup			*k_rdtgrp;
>> +};
>>   extern struct mutex rdtgroup_mutex;
>>   
>>   static inline const char *rdt_kn_name(const struct kernfs_node *kn)
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index cc9966ff6cdf..0ee0dfaf9065 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -78,6 +78,16 @@ static void mon_put_kn_priv(void);
>>   
>>   struct dentry *debugfs_resctrl;
>>   
>> +/*
>> + * Global kernel-mode resctrl policy. Architectures add supported modes before
>> + * resctrl is mounted; @kmode_cur tracks the active policy after user selection.
> 
> ah - above it is referred to as "active". I find "active" to be more descriptive than
> "selected" as highlighted above. The "after user selection" seems distracting.
> "@kmode_cur is the active kernel mode" is all that is necessary, no?

Sure.

> 
>> + */
>> +static struct resctrl_kmode_cfg resctrl_kcfg = {
>> +	.kmode = BIT(INHERIT_CTRL_AND_MON),
>> +	.kmode_cur = INHERIT_CTRL_AND_MON,
>> +	.k_rdtgrp = NULL
>> +};
> 
> This initialization needs to be reconsidered to take into account that system may not
> support allocation or monitoring.
> 

Yes. We may need to change it to dynamic initialization than static.

Thanks
Babu