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

[Babu Moger]

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@xxxxxxx>
> > ---
>
> ...
>
> > ---
> >   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