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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cb41f4bf-7eb4-4a59-b46e-8bc4ae18e6c6@intel.com>
Date: Wed, 7 Jan 2026 14:09:35 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>
CC: Borislav Petkov <bp@...en8.de>, Fenghua Yu <fenghuay@...dia.com>,
	"Wieczor-Retman, Maciej" <maciej.wieczor-retman@...el.com>, Peter Newman
	<peternewman@...gle.com>, James Morse <james.morse@....com>, Babu Moger
	<babu.moger@....com>, Drew Fustini <dfustini@...libre.com>, Dave Martin
	<Dave.Martin@....com>, "Chen, Yu C" <yu.c.chen@...el.com>, "x86@...nel.org"
	<x86@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "patches@...ts.linux.dev"
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v17 13/32] x86,fs/resctrl: Add an architectural hook
 called for each mount

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