Re: [PATCH v6 04/18] fs/resctrl: Introduce kernel mode enum

[Babu Moger <babu.moger@amd.com>]

Hi Reinette,

On 10/15/26 09:44, Reinette Chatre wrote:
> Hi Babu,
> 
> On 10/1/26 8:10 AM, Babu Moger wrote:
>> Kernel mode work can use different allocation and monitoring associations
> 
> Here I think "different allocation and monitoring associations" means
> different CLOSID/RMID, while other places "different allocation and monitoring
> associations" means different MSR_IA32_PQR_PLZA_ASSOC.closid_en and
> MSR_IA32_PQR_PLZA_ASSOC.rmid_en?

That is correct. Hope text here is fine.

> 
>> than the originating user task. On x86, Privilege Level Zero Association
>> (PLZA) enables the kernel to switch to a different CLOSID and/or RMID when
>> entering kernel mode.
>>
>> Architectures need a common way to name kernel modes before resctrl
>> can report what is active or what the platform supports.
>>
>> Introduce enum resctrl_kernel_mode:
>>    - RESCTRL_INHERIT_USER: Kernel mode work inherits allocation and
>>      monitoring from the user task.
>>    - RESCTRL_GLOBAL_ENABLE_PER_CPU: Kernel mode work uses allocation and/or
>>      monitoring associations that differ from the user task.
> 
> As written this is not accurate because there is nothing that prevents
> the "kernel mode group" from being the same as a task's own group?

Yes. That can happen.

  - RESCTRL_GLOBAL_ENABLE_PER_CPU: Kernel mode work may use allocation 
and/or monitoring associations that differ from the user task.

> 
> Could the description of RESCTRL_GLOBAL_ENABLE_PER_CPU be made more specific
> to include that (a) it is a single (global) association that applies to *all*
> user space tasks, and (b) it can be enabled/disabled on each CPU? With these
> two additional details reader will more easily understand why the name of this
> mode is "RESCTRL_GLOBAL_ENABLE_PER_CPU".

Sure.

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
> 
> ...
> 
>> ---
>>   include/linux/resctrl.h | 29 +++++++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index 10dfdca7f4bf..f6adff7f4e1f 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -723,6 +723,35 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
>>    */
>>   bool resctrl_arch_get_io_alloc_enabled(struct rdt_resource *r);
>>   
>> +/**
>> + * enum resctrl_kernel_mode - Kernel mode control and monitoring association.
>> + *
>> + * @RESCTRL_INHERIT_USER:
>> + *     Kernel mode inherits both allocation and monitoring associations
>> + *     from the user-space task. On x86, the kernel uses the same CLOSID and
>> + *     RMID as the user-space task.
>> + *
>> + * @RESCTRL_GLOBAL_ENABLE_PER_CPU:
>> + *     All kernel mode work uses the same allocation and/or monitoring
>> + *     association via an assigned CTRL_MON or MON group. The mode can
>> + *     be enabled/disabled per-CPU. When the mode is disabled on a CPU
>> + *     then the kernel work on that CPU inherits the allocation and
>> + *     monitoring association from the user-space task, effectively the
>> + *     kernel mode becomes RESCTRL_INHERIT_USER on that CPU.
>> + *
>> + *     All online CPUs are included by default. A subset may be selected
>> + *     through the resctrl group interface, and a CTRL_MON or MON group
>> + *     may be assigned to this mode. On x86, CLOSID and RMID are configured
> 
> The sentence starting with "A subset ..." repeats what is described in the
> first paragraph so it could just be dropped? This leaves the first sentence
> of second paragraph out of place, how about merging it into first paragraph
> like:
> 
> 	The mode can be enabled/disabled per-CPU, all online CPUs are enabled
> 	by default.

Sure.
Thanks
Babu