Re: [RESEND PATCH v4 06/15] fs/resctrl: Introduce resctrl_set_kmode_support() to initialize supported modes
"Moger, Babu" <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> >> --- >> v4: New patch to initialize supported kernel-mode's. >> https://lore.kernel.org/lkml/[email protected]/ >> 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