Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Tony,

On 1/7/26 2:27 PM, Luck, Tony wrote:
> On Wed, Jan 07, 2026 at 02:09:35PM -0800, Reinette Chatre wrote:
>> Hi Tony,
>>> If these DO_ONCE macros are ever used heavily in run-time code, it might
>>> be better for once_lock and once_mutex to be statically defined in each
>>> invocation of the DO_ONCE() and DO_ONCE_SLEEPABLE() macros. But the fact
>>> that the static key protects the spinlock/mutex from being called may
>>> mean that it is practically hard to hit problems.
>>
>> Which problems do you have in mind? One problem I see is that since these "once"
>> functions are globally forced to be serialized this may cause unnecessary delays,
>> for example during initialization. I do not think this impacts the resctrl intended
>> usage since resctrl_arch_pre_mount() is not called during initialization and is
>> already ok with delays (it is on a "slow" path).
> 
> Reinette
> 
> Yes. Unnecessary delays due to serialization. But that only happens if
> the first call to a DO_ONCE*() instance overlaps with another first
> call. It might be quite hard to hit that during boot unless there are
> many uses of DO_ONCE*()
> 
> Looking at this some more, DO_ONCE() is overkill for mounting resctrl. The
> static key part is there so that DO_ONCE*() can be safely used in some
> hot code path without adding overhead of checking some "bool done" type
> variable and branching around it.  I don't see anyone except validation
> executing resctrl mounts at multiple times per second.
> 
> But it does make the code easier to read with a single line with obvious
> meaning instead of multiple lines with declarations, initializations,
> and if () conditions.

I am ok with using DO_ONCE_SLEEPABLE(). The next question (perhaps nitpicking?) is
if it is resctrl fs or the arch's decision to use this. That is, whether the flow is
something like below where the arch decides:
	arch/x86/kernel/cpu/resctrl/core.c:
		void resctrl_arch_pre_mount(void)
		{
			DO_ONCE_SLEEPABLE(aet_specific_call);
		}

	fs/resctrl/rdtgroup.c:
		static int rdt_get_tree(struct fs_context *fc)
		{
			...
			resctrl_arch_pre_mount();
			...
		}

or something like below where resctrl fs dictates the function can only be called once:

	arch/x86/kernel/cpu/resctrl/core.c:
		void resctrl_arch_pre_mount(void)
		{
			/* AET specific code */
		}

	fs/resctrl/rdtgroup.c:
		static int rdt_get_tree(struct fs_context *fc)
		{
			...
			DO_ONCE_SLEEPABLE(resctrl_arch_pre_mount);
			...
		}

It looks to me as though the first option creates opportunity for better isolation
of AET code into arch/x86/kernel/cpu/resctrl/intel_aet.c, specifically, it needs fewer AET
stubs in arch/x86/kernel/cpu/resctrl/internal.h. I do not envision resctrl fs needing
to call resctrl_arch_pre_mount() multiple times but the safe pattern appears to be to
place DO_ONCE* in a helper function to ensure that only one static key is ever created.

While the first option allows more flexibility to the arch that should not be a reason though
since this is internal and we can always change to better accommodate arch requirements.
The question here is just what is best for AET support. What do you think?

Reinette