Re: [PATCH v2 01/16] fs/resctrl: Add kernel mode (kmode) data structures and arch hook

Babu Moger
2026-03-26T18:41:17Z

Hi Reinette,

On 3/24/26 17:51, Reinette Chatre wrote:
> Hi Babu,
>
> On 3/12/26 1:36 PM, Babu Moger wrote:
>> Add resctrl_kmode, resctrl_kmode_cfg, kernel mode bit defines, and
>> resctrl_arch_get_kmode_cfg() for resctrl kernel mode (e.g. PLZA) support.
> We should not have to start every series from scratch.
> Documentation/process/maintainer-tip.rst. Always.

Sure. Yea. I did not focus on that aspect of patch submission in this
series. Will do next revision.

>
>> ---
>> include/linux/resctrl.h | 10 ++++++++++
>> include/linux/resctrl_types.h | 30 ++++++++++++++++++++++++++++++
>> 2 files changed, 40 insertions(+)
>>
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index 006e57fd7ca5..2c36d1ac392f 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -699,6 +699,16 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
>> */
>> bool resctrl_arch_get_io_alloc_enabled(struct rdt_resource *r);
>>
>> +/**
>> + * resctrl_arch_get_kmode_cfg() - Get resctrl kernel mode configuration
>> + * @kcfg: Filled with current kernel mode config (kmode, kmode_cur, k_rdtgrp).
>> + *
>> + * Used by the arch (e.g. x86) to report which kernel mode is active and,
>> + * when a global assign mode is in use, which rdtgroup is assigned to
>> + * kernel work.
>> + */
>> +void resctrl_arch_get_kmode_cfg(struct resctrl_kmode_cfg *kcfg);
> This interface does not look right. Would it not be resctrl fs that determines
> which resource group is assigned? This cannot be set by arch. Why does arch decide
> which mode is active? Is this not also resctrl fs? Should arch not just tell
> resctrl fs what it supports?
Yes. Sure. Let the arch tell what is supported.  Let fs decide what is
default.
>
>> +
>> extern unsigned int resctrl_rmid_realloc_threshold;
>> extern unsigned int resctrl_rmid_realloc_limit;
>>
>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
>> index a5f56faa18d2..6b78b08eab29 100644
>> --- a/include/linux/resctrl_types.h
>> +++ b/include/linux/resctrl_types.h
>> @@ -65,7 +65,37 @@ enum resctrl_event_id {
>> QOS_NUM_EVENTS,
>> };
>>
>> +/**
>> + * struct resctrl_kmode - Resctrl kernel mode descriptor
>> + * @name: Human-readable name of the kernel mode.
>> + * @val: Bitmask value for the kernel mode (e.g. INHERIT_CTRL_AND_MON).
>> + */
>> +struct resctrl_kmode {
>> + char name[32];
>> + u32 val;
>> +};
> There is no reason why this needs to be in a central header exposed to archs. Could
> this not be a static within the only function that uses it? Something like
> rdt_mode_str[]?
Yes. I think so.
>
>> +
>> +/**
>> + * struct resctrl_kmode_cfg - Resctrl kernel mode configuration
>> + * @kmode: Requested kernel mode.
>> + * @kmode_cur: Currently active kernel mode.
>> + * @k_rdtgrp: Resource control structure in use, or NULL otherwise.
>> + */
>> +struct resctrl_kmode_cfg {
>> + u32 kmode;
>> + u32 kmode_cur;
>> + struct rdtgroup *k_rdtgrp;
>> +};
>> +
>> #define QOS_NUM_L3_MBM_EVENTS (QOS_L3_MBM_LOCAL_EVENT_ID - QOS_L3_MBM_TOTAL_EVENT_ID + 1)
>> #define MBM_STATE_IDX(evt) ((evt) - QOS_L3_MBM_TOTAL_EVENT_ID)
>>
>> +/* Resctrl kernel mode bits (e.g. for PLZA). */
>> +#define INHERIT_CTRL_AND_MON BIT(0) /* Kernel uses same CLOSID/RMID as user. */
>> +/* One CLOSID for all kernel work; RMID inherited from user. */
>> +#define GLOBAL_ASSIGN_CTRL_INHERIT_MON BIT(1)
>> +/* One resource group (CLOSID+RMID) for all kernel work. */
>> +#define GLOBAL_ASSIGN_CTRL_ASSIGN_MON BIT(2)
>> +#define RESCTRL_KERNEL_MODES_NUM 3
> I think it will make the code much easier to understand if the different modes are described by an
> enum. For example,
Yes. Sure.
>
> enum resctrl_kernel_modes {
> INHERIT_CTRL_AND_MON,
> GLOBAL_ASSIGN_CTRL_INHERIT_MON,
> GLOBAL_ASSIGN_CTRL_ASSIGN_MON,
> RESCTRL_KMODE_LAST = GLOBAL_ASSIGN_CTRL_ASSIGN_MON
> };
> #define RESCTRL_NUM_KERNEL_MODES (RESCTRL_KMODE_LAST + 1)
>
> The supported kernel modes can still be managed as a bitmap with intuitive API using the
> enum that will make the code easier to read. For example, __set_bit(INHERIT_CTRL_AND_MON, ...)
> or BIT(INHERIT_CTRL_AND_MON). The naming is awkward at the moment though, we should improve here.
>

Sure.  Yes. We need to think about naming.. Let me think about it.

Thanks

Babu