From: "Arnd Bergmann" <arnd@arndb.de>
To: "Gabriele Monaco" <gmonaco@redhat.com>,
"Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Eduard Zingerman" <eddyz87@gmail.com>,
"Kumar Kartikeya Dwivedi" <memxor@gmail.com>,
bpf@vger.kernel.org, Linux-Arch <linux-arch@vger.kernel.org>,
linux-kernel@vger.kernel.org
Cc: "Peter Zijlstra" <peterz@infradead.org>,
"Ingo Molnar" <mingo@redhat.com>, "Will Deacon" <will@kernel.org>,
"Boqun Feng" <boqun@kernel.org>,
"Waiman Long" <longman@redhat.com>
Subject: Re: [PATCH] rqspinlock: Fix order in raw_res_spin_(un)lock_irq to allow schedule
Date: Tue, 09 Jun 2026 13:22:35 +0200 [thread overview]
Message-ID: <d40ba64d-78d9-45f5-99b9-4bfb1fc27f6c@app.fastmail.com> (raw)
In-Reply-To: <20260609094941.56122-1-gmonaco@redhat.com>
On Tue, Jun 9, 2026, at 11:49, Gabriele Monaco wrote:
> raw_res_spin_unlock_irqrestore() calls raw_res_spin_unlock() and then
> restores interrupts, this means preemption is enabled when interrupts
> are still disabled (as part of raw_res_spin_unlock()) so this cannot
> trigger an actual preemption.
> This is inconsistent with other spinlock implementations
> (raw_spin_unlock_irqrestore() and bpf_res_spin_unlock_irqrestore()
> itself).
>
> Adjust the macro to ensure interrupts are enabled before enabling
> preemption, allowing to schedule at that point. Make the same
> modification in the error path of raw_res_spin_lock_irqsave().
>
> Fixes: 101acd2e78b1 ("rqspinlock: Add macros for rqspinlock usage")
Should this be Cc:stable@vger.kernel.org to get backported?
Did you see this cause measurable performance problems,
or did you find it through inspection?
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de> # asm-generic
This should probably get merged through the BPF tree, but I've
added the kernel/locking maintainers to Cc as well, since I
feel it's more useful to have them look at it than me.
Maybe it would be good to update (as a separate patch) the
MAINTAINERS file so the locking subsystem also includes the
headers currently missing:
arch/*/include/asm/*spinlock*.h
arch/*/include/asm/*rwlock*.h
include/asm-generic/*spinlock*.h
include/asm-generic/*rwlock*.h
Arnd
(full patch quoted below)
> ---
> include/asm-generic/rqspinlock.h | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/include/asm-generic/rqspinlock.h
> b/include/asm-generic/rqspinlock.h
> index 151d267a49..4d46643f46 100644
> --- a/include/asm-generic/rqspinlock.h
> +++ b/include/asm-generic/rqspinlock.h
> @@ -243,12 +243,20 @@ static __always_inline void
> res_spin_unlock(rqspinlock_t *lock)
> ({ \
> int __ret; \
> local_irq_save(flags); \
> - __ret = raw_res_spin_lock(lock); \
> - if (__ret) \
> + preempt_disable(); \
> + __ret = res_spin_lock(lock); \
> + if (__ret) { \
> local_irq_restore(flags); \
> + preempt_enable(); \
> + } \
> __ret; \
> })
>
> -#define raw_res_spin_unlock_irqrestore(lock, flags) ({
> raw_res_spin_unlock(lock); local_irq_restore(flags); })
> +#define raw_res_spin_unlock_irqrestore(lock, flags) \
> + ({ \
> + res_spin_unlock(lock); \
> + local_irq_restore(flags); \
> + preempt_enable(); \
> + })
>
> #endif /* __ASM_GENERIC_RQSPINLOCK_H */
>
> base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
> --
> 2.54.0
next prev parent reply other threads:[~2026-06-09 11:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-09 9:49 Gabriele Monaco
2026-06-09 11:22 ` Arnd Bergmann [this message]
2026-06-09 13:04 ` Gabriele Monaco
2026-06-09 13:08 ` Arnd Bergmann
2026-06-09 14:42 ` Kumar Kartikeya Dwivedi
2026-06-09 16:17 ` Gabriele Monaco
2026-06-09 16:57 ` Alexei Starovoitov
2026-06-09 14:35 ` Peter Zijlstra
2026-06-10 0:06 ` Waiman Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d40ba64d-78d9-45f5-99b9-4bfb1fc27f6c@app.fastmail.com \
--to=arnd@arndb.de \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=boqun@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=gmonaco@redhat.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=memxor@gmail.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®