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

Tony Luck posted 32 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Tony Luck 1 month, 3 weeks ago
Enumeration of Intel telemetry events is an asynchronous process involving
several mutually dependent drivers added as auxiliary devices during
the device_initcall() phase of Linux boot. The process finishes after
the probe functions of these drivers completes. But this happens after
resctrl_arch_late_init() is executed.

Tracing the enumeration process shows that it does complete a full seven
seconds before the earliest possible mount of the resctrl file system (when
included in /etc/fstab for automatic mount by systemd).

Add a hook at the beginning of the mount code that will be used to check
for telemetry events and initialize if any are found.

Call the hook on every attempted mount. Expectations are that most actions
(like enumeration) will only need to be performed on the first call.

resctrl filesystem calls the hook with no locks held. Architecture code is
responsible for any required locking.

Signed-off-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
---
 include/linux/resctrl.h            | 6 ++++++
 arch/x86/kernel/cpu/resctrl/core.c | 9 +++++++++
 fs/resctrl/rdtgroup.c              | 2 ++
 3 files changed, 17 insertions(+)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index c43526cdf304..dc148b7feb71 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -514,6 +514,12 @@ void resctrl_offline_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *h
 void resctrl_online_cpu(unsigned int cpu);
 void resctrl_offline_cpu(unsigned int cpu);
 
+/*
+ * Architecture hook called at beginning of each file system mount attempt.
+ * No locks are held.
+ */
+void resctrl_arch_pre_mount(void);
+
 /**
  * resctrl_arch_rmid_read() - Read the eventid counter corresponding to rmid
  *			      for this resource and domain.
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 9222eee7ce07..2dd48b59ba9b 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -726,6 +726,15 @@ static int resctrl_arch_offline_cpu(unsigned int cpu)
 	return 0;
 }
 
+void resctrl_arch_pre_mount(void)
+{
+	static atomic_t only_once = ATOMIC_INIT(0);
+	int old = 0;
+
+	if (!atomic_try_cmpxchg(&only_once, &old, 1))
+		return;
+}
+
 enum {
 	RDT_FLAG_CMT,
 	RDT_FLAG_MBM_TOTAL,
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 771e40f02ba6..b20d104ea0c9 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -2785,6 +2785,8 @@ static int rdt_get_tree(struct fs_context *fc)
 	struct rdt_resource *r;
 	int ret;
 
+	resctrl_arch_pre_mount();
+
 	cpus_read_lock();
 	mutex_lock(&rdtgroup_mutex);
 	/*
-- 
2.52.0
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Borislav Petkov 1 month ago
On Wed, Dec 17, 2025 at 09:21:00AM -0800, Tony Luck wrote:
> +void resctrl_arch_pre_mount(void)
> +{
> +	static atomic_t only_once = ATOMIC_INIT(0);
> +	int old = 0;
> +
> +	if (!atomic_try_cmpxchg(&only_once, &old, 1))
> +		return;
> +}

There's

#define DO_ONCE(func, ...)

Can't use that?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Luck, Tony 1 month ago
On Mon, Jan 05, 2026 at 08:17:11PM +0100, Borislav Petkov wrote:
> On Wed, Dec 17, 2025 at 09:21:00AM -0800, Tony Luck wrote:
> > +void resctrl_arch_pre_mount(void)
> > +{
> > +	static atomic_t only_once = ATOMIC_INIT(0);
> > +	int old = 0;
> > +
> > +	if (!atomic_try_cmpxchg(&only_once, &old, 1))
> > +		return;
> > +}
> 
> There's
> 
> #define DO_ONCE(func, ...)
> 
> Can't use that?

Learn something new every day. Yes, <linux/once.h> looks possible here.

Though I believe I'll need the DO_ONCE_SLEEPABLE() version since 
resctrl_arch_pre_mount() grabs a mutex and various called functions
allocate memory using kmalloc().

-Tony
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Borislav Petkov 1 month ago
On Mon, Jan 05, 2026 at 11:39:29AM -0800, Luck, Tony wrote:
> Learn something new every day. Yes, <linux/once.h> looks possible here.
> 
> Though I believe I'll need the DO_ONCE_SLEEPABLE() version since 
> resctrl_arch_pre_mount() grabs a mutex and various called functions
> allocate memory using kmalloc().

Aha.

Ok, if it works and passes testing, I could wait for you to send me an updated
patch and drop this one.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
RE: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Luck, Tony 1 month ago
> Ok, if it works and passes testing, I could wait for you to send me an updated
> patch and drop this one.

Building and testing now.

Reinette: When originally developing this you suggested that rdt_get_tree()
should call resctrl_arch_pre_mount() on *every* mount (to make it generally
useful should future changes need something to be done in architecture code
on each mount).

That flexibility isn't needed for enumerating telemetry events. Boris' suggestion
to use DO_ONCE_SLEEPABLE() would revert to what I had in some earlier
version where rdt_get_tree() only calls this hook on first mount.

Are you OK with this? Or do you still think that the hook should be called on
every mount?

-Tony
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Reinette Chatre 1 month ago
Hi Tony,

On 1/5/26 12:15 PM, Luck, Tony wrote:
>> Ok, if it works and passes testing, I could wait for you to send me an updated
>> patch and drop this one.
> 
> Building and testing now.
> 
> Reinette: When originally developing this you suggested that rdt_get_tree()
> should call resctrl_arch_pre_mount() on *every* mount (to make it generally
> useful should future changes need something to be done in architecture code
> on each mount).

I'm digging through the history just to refresh on why I made that comment. From what I can
tell this work always called the AET init on every mount attempt. One difference is that during
v2 it did so by taking some extra locks before doing so, but still did the AET init before
resctrl's "resctrl_mounted" check. The move to current spot (before extra locks) was made in v3,
and looking at v2 comments I could just find a request to use a generic resctrl_arch_* helper in
fs code instead of the arch specific rdt_get_intel_aet_mount() called from fs code.

> 
> That flexibility isn't needed for enumerating telemetry events. Boris' suggestion
> to use DO_ONCE_SLEEPABLE() would revert to what I had in some earlier
> version where rdt_get_tree() only calls this hook on first mount.

I think I am missing something here - even the original RFC calls the AET init on
every mount. Which version are you referring to? I am also missing why DO_ONCE_SLEEPABLE()
requires a flow change.

> 
> Are you OK with this? Or do you still think that the hook should be called on
> every mount?

To be specific, the current implementation calls the resctrl_arch_pre_mount() hook on
every mount *attempt*. For the hook to be called on every mount it should be after the
resctrl_mounted check. This would change resctrl_arch_pre_mount() to be called with
rdtgroup_mutex held though but that seems trouble since resctrl_arch_pre_mount() currently
follows lock ordering of domain_list_lock then rdtgroup_mutex to match lock ordering
during resctrl init.

Reinette
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Luck, Tony 1 month ago
On Wed, Jan 07, 2026 at 09:29:27AM -0800, Reinette Chatre wrote:
> Hi Tony,
> 
> On 1/5/26 12:15 PM, Luck, Tony wrote:
> >> Ok, if it works and passes testing, I could wait for you to send me an updated
> >> patch and drop this one.
> > 
> > Building and testing now.
> > 
> > Reinette: When originally developing this you suggested that rdt_get_tree()
> > should call resctrl_arch_pre_mount() on *every* mount (to make it generally
> > useful should future changes need something to be done in architecture code
> > on each mount).
> 
> I'm digging through the history just to refresh on why I made that comment. From what I can
> tell this work always called the AET init on every mount attempt. One difference is that during
> v2 it did so by taking some extra locks before doing so, but still did the AET init before
> resctrl's "resctrl_mounted" check. The move to current spot (before extra locks) was made in v3,
> and looking at v2 comments I could just find a request to use a generic resctrl_arch_* helper in
> fs code instead of the arch specific rdt_get_intel_aet_mount() called from fs code.

I should stop relying on my memory and check the actual history. You are
right. The call from rdt_get_tree() has moved, and changed name, but all
versions called it every time.

> > 
> > That flexibility isn't needed for enumerating telemetry events. Boris' suggestion
> > to use DO_ONCE_SLEEPABLE() would revert to what I had in some earlier
> > version where rdt_get_tree() only calls this hook on first mount.
> 
> I think I am missing something here - even the original RFC calls the AET init on
> every mount. Which version are you referring to? I am also missing why DO_ONCE_SLEEPABLE()
> requires a flow change.
> 
> > 
> > Are you OK with this? Or do you still think that the hook should be called on
> > every mount?
> 
> To be specific, the current implementation calls the resctrl_arch_pre_mount() hook on
> every mount *attempt*. For the hook to be called on every mount it should be after the
> resctrl_mounted check. This would change resctrl_arch_pre_mount() to be called with
> rdtgroup_mutex held though but that seems trouble since resctrl_arch_pre_mount() currently
> follows lock ordering of domain_list_lock then rdtgroup_mutex to match lock ordering
> during resctrl init.

Yes, the call was moved before any locks obtained because of lock
ordering issues with domain_list_lock.

A better summary of the change is that the "only once" logic is being
moved from open-coded using atomic operations in resctrl_arch_pre_mount()
to using DO_ONCE_SLEEPABLE() in rdt_get_tree().
> 
> Reinette

-Tony
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Reinette Chatre 1 month ago
Hi Tony,

On 1/7/26 10:05 AM, Luck, Tony wrote:
> On Wed, Jan 07, 2026 at 09:29:27AM -0800, Reinette Chatre wrote:
>> Hi Tony,
>>
>> On 1/5/26 12:15 PM, Luck, Tony wrote:
>>>> Ok, if it works and passes testing, I could wait for you to send me an updated
>>>> patch and drop this one.
>>>
>>> Building and testing now.
>>>
>>> Reinette: When originally developing this you suggested that rdt_get_tree()
>>> should call resctrl_arch_pre_mount() on *every* mount (to make it generally
>>> useful should future changes need something to be done in architecture code
>>> on each mount).
>>
>> I'm digging through the history just to refresh on why I made that comment. From what I can
>> tell this work always called the AET init on every mount attempt. One difference is that during
>> v2 it did so by taking some extra locks before doing so, but still did the AET init before
>> resctrl's "resctrl_mounted" check. The move to current spot (before extra locks) was made in v3,
>> and looking at v2 comments I could just find a request to use a generic resctrl_arch_* helper in
>> fs code instead of the arch specific rdt_get_intel_aet_mount() called from fs code.
> 
> I should stop relying on my memory and check the actual history. You are
> right. The call from rdt_get_tree() has moved, and changed name, but all
> versions called it every time.
> 
>>>
>>> That flexibility isn't needed for enumerating telemetry events. Boris' suggestion
>>> to use DO_ONCE_SLEEPABLE() would revert to what I had in some earlier
>>> version where rdt_get_tree() only calls this hook on first mount.
>>
>> I think I am missing something here - even the original RFC calls the AET init on
>> every mount. Which version are you referring to? I am also missing why DO_ONCE_SLEEPABLE()
>> requires a flow change.
>>
>>>
>>> Are you OK with this? Or do you still think that the hook should be called on
>>> every mount?
>>
>> To be specific, the current implementation calls the resctrl_arch_pre_mount() hook on
>> every mount *attempt*. For the hook to be called on every mount it should be after the
>> resctrl_mounted check. This would change resctrl_arch_pre_mount() to be called with
>> rdtgroup_mutex held though but that seems trouble since resctrl_arch_pre_mount() currently
>> follows lock ordering of domain_list_lock then rdtgroup_mutex to match lock ordering
>> during resctrl init.
> 
> Yes, the call was moved before any locks obtained because of lock
> ordering issues with domain_list_lock.
> 
> A better summary of the change is that the "only once" logic is being
> moved from open-coded using atomic operations in resctrl_arch_pre_mount()
> to using DO_ONCE_SLEEPABLE() in rdt_get_tree().

One thing about DO_ONCE_SLEEPABLE() that is unexpected to me is that it disables the static
key in a workqueue that seems unnecessary. Original motivation for the workqueue on which DO_ONCE()
is based (per commit a48e42920ff3 ("net: introduce new macro net_get_random_once")) was to support
calling code from atomic sections. Looks like DO_ONCE_SLEEPABLE() copied this implementation. It switched
the spinlock to mutex but the changelog (62c07983bef9 ("once: add DO_ONCE_SLOW() for sleepable contexts")) does
not mention revisiting the workqueue. Looks like deferring it to workqueue does make it easier to not have
to worry whether helper is called with hotplug lock held or not though.

Do you see any issue with deferring the disable of the static key in the resctrl usage? Since
resctrl_arch_pre_mount() is called without any locks in resctrl control it now relies on fs code
to not have rdt_get_tree() called concurrently and thus risk resctrl_arch_pre_mount() called before
static key is disabled? I just want to make sure here since from what I can tell this makes resctrl the
first user of this helper apart from code for which this helper was created and there may be implicit
assumptions that resctrl does not adhere to.

Reinette
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Luck, Tony 1 month ago
On Wed, Jan 07, 2026 at 11:33:26AM -0800, Reinette Chatre wrote:
> > A better summary of the change is that the "only once" logic is being
> > moved from open-coded using atomic operations in resctrl_arch_pre_mount()
> > to using DO_ONCE_SLEEPABLE() in rdt_get_tree().
> 
> One thing about DO_ONCE_SLEEPABLE() that is unexpected to me is that it disables the static
> key in a workqueue that seems unnecessary. Original motivation for the workqueue on which DO_ONCE()
> is based (per commit a48e42920ff3 ("net: introduce new macro net_get_random_once")) was to support
> calling code from atomic sections. Looks like DO_ONCE_SLEEPABLE() copied this implementation. It switched
> the spinlock to mutex but the changelog (62c07983bef9 ("once: add DO_ONCE_SLOW() for sleepable contexts")) does
> not mention revisiting the workqueue. Looks like deferring it to workqueue does make it easier to not have
> to worry whether helper is called with hotplug lock held or not though.
> 
> Do you see any issue with deferring the disable of the static key in the resctrl usage? Since
> resctrl_arch_pre_mount() is called without any locks in resctrl control it now relies on fs code
> to not have rdt_get_tree() called concurrently and thus risk resctrl_arch_pre_mount() called before
> static key is disabled? I just want to make sure here since from what I can tell this makes resctrl the
> first user of this helper apart from code for which this helper was created and there may be implicit
> assumptions that resctrl does not adhere to.

Reinette,

The deferred reset of the static key does seem unnecessary.

But it looks like DO_ONCE_SLEEPABLE() is still correct. If there are
multiple parallel calls before the static key is reset, then the 2nd and
subsequent instance will block on mutex_lock(&once_mutex) in
__do_once_sleepable_start(). When it is their turn, they will find that
"*done" is true, so resctrl_arch_pre_mount() will not be called again.

Side note: This global "once_mutex" means that any other subsystem using
DO_ONCE_SLEEPABLE() would be blocked waiting for resctrl_arch_pre_mount()
to complete. Same is true for DO_ONCE() where parallel calls from
different subsystems would be serialized by the "once_lock" spinlock.

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.

-Tony
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Reinette Chatre 1 month ago
Hi Tony,

On 1/7/26 12:25 PM, Luck, Tony wrote:
> On Wed, Jan 07, 2026 at 11:33:26AM -0800, Reinette Chatre wrote:
>>> A better summary of the change is that the "only once" logic is being
>>> moved from open-coded using atomic operations in resctrl_arch_pre_mount()
>>> to using DO_ONCE_SLEEPABLE() in rdt_get_tree().
>>
>> One thing about DO_ONCE_SLEEPABLE() that is unexpected to me is that it disables the static
>> key in a workqueue that seems unnecessary. Original motivation for the workqueue on which DO_ONCE()
>> is based (per commit a48e42920ff3 ("net: introduce new macro net_get_random_once")) was to support
>> calling code from atomic sections. Looks like DO_ONCE_SLEEPABLE() copied this implementation. It switched
>> the spinlock to mutex but the changelog (62c07983bef9 ("once: add DO_ONCE_SLOW() for sleepable contexts")) does
>> not mention revisiting the workqueue. Looks like deferring it to workqueue does make it easier to not have
>> to worry whether helper is called with hotplug lock held or not though.
>>
>> Do you see any issue with deferring the disable of the static key in the resctrl usage? Since
>> resctrl_arch_pre_mount() is called without any locks in resctrl control it now relies on fs code
>> to not have rdt_get_tree() called concurrently and thus risk resctrl_arch_pre_mount() called before
>> static key is disabled? I just want to make sure here since from what I can tell this makes resctrl the
>> first user of this helper apart from code for which this helper was created and there may be implicit
>> assumptions that resctrl does not adhere to.
> 
> Reinette,
> 
> The deferred reset of the static key does seem unnecessary.
> 
> But it looks like DO_ONCE_SLEEPABLE() is still correct. If there are
> multiple parallel calls before the static key is reset, then the 2nd and
> subsequent instance will block on mutex_lock(&once_mutex) in
> __do_once_sleepable_start(). When it is their turn, they will find that
> "*done" is true, so resctrl_arch_pre_mount() will not be called again.

Ah - I see. Thank you. So in addition to the static key there is the "done"
variable that is protected with the mutex and protects against a second call
before static key can be disabled.

> 
> Side note: This global "once_mutex" means that any other subsystem using
> DO_ONCE_SLEEPABLE() would be blocked waiting for resctrl_arch_pre_mount()
> to complete. Same is true for DO_ONCE() where parallel calls from
> different subsystems would be serialized by the "once_lock" spinlock.

oh, good catch.

> 
> 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
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Luck, Tony 1 month ago
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.

-Tony
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Reinette Chatre 1 month ago
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
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Luck, Tony 1 month ago
On Wed, Jan 07, 2026 at 03:09:24PM -0800, Reinette Chatre wrote:
> 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);
> 		}

The AET code in resctrl_arch_pre_mount() includes building the domains.
That needs the domain_list_lock mutex and domain_add_cpu_mon() which are
both static in core.c. So either they need to be unstatic'd and added
to "internal.h", or that part of the code needs to stay in core.c

Opinion on making these available to intel_aet.c? I'm not a fan.

Keeping it in core.c means finding out if intel_aet_get_events()
succeeded or not. DO_ONCE_SLEEPABLE() doesn't return the return value
of the called function. It just returns true/false to say if it called
the function.

So with this approach I have:

void resctrl_arch_pre_mount(void)
{
	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
	int cpu;

	if (!DO_ONCE_SLEEPABLE(intel_aet_get_events))
		return;

	// intel_aet_get_events() sets mon_capable if it succeeds
	if (!r->mon_capable)
		return;

	/*
	 * Late discovery of telemetry events means the domains for the
	 * resource were not built. Do that now.
	 */
	cpus_read_lock();
	mutex_lock(&domain_list_lock);
	rdt_mon_capable = true;
	for_each_online_cpu(cpu)
		domain_add_cpu_mon(cpu, r);
	mutex_unlock(&domain_list_lock);
	cpus_read_unlock();
}

It does reduce by one the number of stubs. intel_aet_add_debugfs() can
be static in intel_aet.c

> 	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 */
This is the minimal change from my current series. So my laziness factor
leans toward it.

> 		}
> 
> 	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?

The current usage for resctrl_arch_pre_mount() is that it only needs to
be called once. As you say, that could be changed if a new requirement
appears. But the simpler approach today is to put the
DO_ONCE_SLEEPABLE() into rdt_get_tree()

> Reinette

-Tony
Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook called for each mount
Posted by Reinette Chatre 1 month ago
Hi Tony,

On 1/7/26 4:16 PM, Luck, Tony wrote:
> On Wed, Jan 07, 2026 at 03:09:24PM -0800, Reinette Chatre wrote:
>> 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);
>> 		}
> 
> The AET code in resctrl_arch_pre_mount() includes building the domains.
> That needs the domain_list_lock mutex and domain_add_cpu_mon() which are
> both static in core.c. So either they need to be unstatic'd and added
> to "internal.h", or that part of the code needs to stay in core.c
> 
> Opinion on making these available to intel_aet.c? I'm not a fan.

ok, that is fair.

> 
> Keeping it in core.c means finding out if intel_aet_get_events()
> succeeded or not. DO_ONCE_SLEEPABLE() doesn't return the return value
> of the called function. It just returns true/false to say if it called
> the function.
> 
> So with this approach I have:
> 
> void resctrl_arch_pre_mount(void)
> {
> 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
> 	int cpu;
> 
> 	if (!DO_ONCE_SLEEPABLE(intel_aet_get_events))
> 		return;
> 

Thank you for considering. This is getting difficult to read.

> 	// intel_aet_get_events() sets mon_capable if it succeeds
> 	if (!r->mon_capable)
> 		return;
> 
> 	/*
> 	 * Late discovery of telemetry events means the domains for the
> 	 * resource were not built. Do that now.
> 	 */
> 	cpus_read_lock();
> 	mutex_lock(&domain_list_lock);
> 	rdt_mon_capable = true;
> 	for_each_online_cpu(cpu)
> 		domain_add_cpu_mon(cpu, r);
> 	mutex_unlock(&domain_list_lock);
> 	cpus_read_unlock();
> }
> 
> It does reduce by one the number of stubs. intel_aet_add_debugfs() can
> be static in intel_aet.c
> 
>> 	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 */
> This is the minimal change from my current series. So my laziness factor
> leans toward it.
> 
>> 		}
>>
>> 	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?
> 
> The current usage for resctrl_arch_pre_mount() is that it only needs to
> be called once. As you say, that could be changed if a new requirement
> appears. But the simpler approach today is to put the
> DO_ONCE_SLEEPABLE() into rdt_get_tree()

Thank you for considering the options. Placing DO_ONCE_SLEEPABLE() in
rdt_get_tree() is fine by me.

Reinette