Re: [PATCH v2 02/16] fs, x86/resctrl: Add architecture routines for kernel mode initialization

[Babu Moger]

Hi Reinette,

On 3/24/26 17:53, Reinette Chatre wrote:
> Hi Babu,
>
> On 3/12/26 1:36 PM, Babu Moger wrote:
> > Implement the resctrl kernel mode (kmode) arch initialization.
> >
> > - Add resctrl_arch_get_kmode_cfg() to fill the default kernel mode
> >    (INHERIT_CTRL_AND_MON). This can be extended later (e.g. for PLZA) to set
> >    additional modes.
> I do not think this is something that the architecture should set, at least
> at this time. Every mode has different requirements and this just lets the arch set
> it without any support for what configurations it implies. For example, if
> arch sets a different default mode than INHERIT_CTRL_AND_MON then PQR_PLZA_ASSOC
> needs to be programmed as the CPUs come online and this does not seem to
> accommodate this. This implementation appears to have significant assumptions on
> what architecture will end up setting since it is only considering PLZA.

Sure.  Let the arch report what is supported. Will change it to set the 
default in fs code.

Users can change change modes from FS code.


> > - Add global resctrl_kcfg and resctrl_kmode_init() to initialize default
> >    values.
> >
> > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > ---
> > v2: New patch to handle PLZA interfaces with /sys/fs/resctrl/info/ directory.
> >      https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@xxxxxxxxx/
> > ---
> >   arch/x86/kernel/cpu/resctrl/core.c |  7 +++++++
> >   fs/resctrl/rdtgroup.c              | 10 ++++++++++
> >   2 files changed, 17 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 7667cf7c4e94..4c3ab2d93909 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -892,6 +892,13 @@ bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
> >   	}
> >   }
> >   
> > +void resctrl_arch_get_kmode_cfg(struct resctrl_kmode_cfg *kcfg)
> > +{
> > +	kcfg->kmode = INHERIT_CTRL_AND_MON;
> > +	kcfg->kmode_cur = INHERIT_CTRL_AND_MON;
> > +	kcfg->k_rdtgrp = NULL;
> > +}
> I already commented on the arch vs filesystem settings.
>
> When using an arch helper this forces all architectures to support this helper. Is a
> helper required? Is it perhaps possible for arch to set a property instead? For example,
> how enumeration is handled?
> I think the assumption here is that INHERIT_CTRL_AND_MON is the default and expected to
> be supported by all architectures. I do not see why arch should set this as default but
> instead this should be from resctrl fs. At the same time it is expected that the
> architecture supports this mode so there needs to be a failure if an architecture does
> not support this mode?

I will change. Arch sets the supported modes.  FS sets the default. 
Users can change it to required mode later.


> I'm going to stop here. I think the comments so far may result in major changes already
> making further detailed review of patches unnecessary.


Based on my comments below you may need to re-look at the some of the 
patches.

https://lore.kernel.org/lkml/47c0db32-d0e0-4c53-90bd-b74863d233dc@xxxxxxx/

I am fine otherwise also. Let continue that discussion.

Thanks

Babu


> Reinette