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: <aV7BIWu7cSSgCv1v@agluck-desk3>
Date: Wed, 7 Jan 2026 12:25:05 -0800
From: "Luck, Tony" <tony.luck@...el.com>
To: Reinette Chatre <reinette.chatre@...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

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