mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Bart Van Assche <bvanassche@acm.org>
Cc: "Lyude Paul" <lyude@redhat.com>,
	rust-for-linux@vger.kernel.org,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	linux-kernel@vger.kernel.org,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Juri Lelli" <juri.lelli@redhat.com>,
	"Vincent Guittot" <vincent.guittot@linaro.org>,
	"Dietmar Eggemann" <dietmar.eggemann@arm.com>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Ben Segall" <bsegall@google.com>, "Mel Gorman" <mgorman@suse.de>,
	"Valentin Schneider" <vschneid@redhat.com>,
	"Will Deacon" <will@kernel.org>,
	"Waiman Long" <longman@redhat.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"David Woodhouse" <dwmw@amazon.co.uk>,
	"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Ryo Takakura" <ryotkkr98@gmail.com>,
	"K Prateek Nayak" <kprateek.nayak@amd.com>
Subject: Re: [PATCH v13 05/17] irq & spin_lock: Add counted interrupt disabling/enabling
Date: Thu, 16 Oct 2025 10:15:13 +0200	[thread overview]
Message-ID: <20251016081513.GB3289052@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <7a98eb42-bc54-4f22-bc85-0f6d41f39fc7@acm.org>

On Wed, Oct 15, 2025 at 01:54:05PM -0700, Bart Van Assche wrote:

> > This also makes the wrapper of interrupt-disabling locks on Rust easier
> > to design.
> 
> Is a new counter really required to fix the issues that exist in the
> above examples? Has it been considered to remove the spin_lock_irqsave()
> and spin_lock_irqrestore() definitions, to introduce a macro that saves
> the interrupt state in a local variable and restores the interrupt state
> from the same local variable? With this new macro, the first two examples
> above would be changed into the following (this code has not been tested
> in any way):

So the thing is that actually frobbing the hardware interrupt state is
relatively expensive. On x86 things like PUSHF/POPF/CLI/STI are
definitely on the 'nice to avoid' side of things.

Various people have written patches to avoid them on x86, and while none
of them have made it, they did show benefit (notably PowerPC already
does something tricky because for them it is *really* expensive).

So in that regard, keeping this counter allows us to strictly reduce the
places where we have to touch IF. The interface is nicer too, so a win
all-round.

My main objection to all this has been that they add to the interface
instead of replace the interface. Ideally this would implement
spin_lock_irq() and spin_lock_irqsave() in terms of the new
spin_lock_irq_disable() and then we go do tree wide cleanups over a few
cycles.

The problem is that that requires non-trivial per architecture work and
they've so far tried to avoid this...

  reply	other threads:[~2025-10-16  8:16 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13 15:48 [PATCH v13 00/17] Refcounted interrupts, SpinLockIrq for rust Lyude Paul
2025-10-13 15:48 ` [PATCH v13 01/17] preempt: Track NMI nesting to separate per-CPU counter Lyude Paul
2025-10-13 16:19   ` Lyude Paul
2025-10-13 16:32     ` Miguel Ojeda
2025-10-13 20:00   ` Peter Zijlstra
2025-10-13 21:27     ` Joel Fernandes
2025-10-14  8:25       ` Peter Zijlstra
2025-10-14 17:59         ` Joel Fernandes
2025-10-14 19:37           ` Peter Zijlstra
2025-10-14 10:48   ` Peter Zijlstra
2025-10-14 17:55     ` Joel Fernandes
2025-10-14 19:43       ` Peter Zijlstra
2025-10-14 22:05         ` Joel Fernandes
2025-10-20 20:44         ` Joel Fernandes
2025-10-30 22:56           ` Joel Fernandes
2025-10-13 15:48 ` [PATCH v13 02/17] preempt: Reduce NMI_MASK to single bit and restore HARDIRQ_BITS Lyude Paul
2025-11-04 12:15   ` Andreas Hindborg
2025-10-13 15:48 ` [PATCH v13 03/17] preempt: Introduce HARDIRQ_DISABLE_BITS Lyude Paul
2025-10-31 14:59   ` Andreas Hindborg
2025-10-31 19:59     ` Boqun Feng
2025-10-13 15:48 ` [PATCH v13 04/17] preempt: Introduce __preempt_count_{sub, add}_return() Lyude Paul
2025-11-04 12:30   ` Andreas Hindborg
2025-10-13 15:48 ` [PATCH v13 05/17] irq & spin_lock: Add counted interrupt disabling/enabling Lyude Paul
2025-10-15 20:54   ` Bart Van Assche
2025-10-16  8:15     ` Peter Zijlstra [this message]
2025-10-17  6:44       ` Boqun Feng
2025-10-16 21:24   ` David Laight
2025-10-17  6:48     ` Boqun Feng
2025-11-04 12:45   ` Andreas Hindborg
2025-11-19 21:47     ` Lyude Paul
2025-10-13 15:48 ` [PATCH v13 06/17] irq: Add KUnit test for refcounted interrupt enable/disable Lyude Paul
2025-10-13 15:48 ` [PATCH v13 07/17] rust: Introduce interrupt module Lyude Paul
2025-11-04 12:55   ` Andreas Hindborg
2025-10-13 15:48 ` [PATCH v13 08/17] rust: helper: Add spin_{un,}lock_irq_{enable,disable}() helpers Lyude Paul
2025-11-04 12:56   ` Andreas Hindborg
2025-10-13 15:48 ` [PATCH v13 09/17] rust: sync: Add SpinLockIrq Lyude Paul
2025-11-04 13:09   ` Andreas Hindborg
2025-10-13 15:48 ` [PATCH v13 10/17] rust: sync: Introduce lock::Backend::Context Lyude Paul
2025-10-13 15:48 ` [PATCH v13 11/17] rust: sync: lock: Add `Backend::BackendInContext` Lyude Paul
2025-11-05 12:45   ` Andreas Hindborg
2025-10-13 15:48 ` [PATCH v13 12/17] rust: sync: lock/global: Rename B to G in trait bounds Lyude Paul
2025-10-13 15:48 ` [PATCH v13 13/17] rust: sync: Add a lifetime parameter to lock::global::GlobalGuard Lyude Paul
2025-10-13 15:48 ` [PATCH v13 14/17] rust: sync: Expose lock::Backend Lyude Paul
2025-10-13 15:48 ` [PATCH v13 15/17] rust: sync: lock/global: Add Backend parameter to GlobalGuard Lyude Paul
2025-10-13 15:48 ` [PATCH v13 16/17] rust: sync: lock/global: Add BackendInContext support to GlobalLock Lyude Paul
2025-10-13 15:48 ` [PATCH v13 17/17] locking: Switch to _irq_{disable,enable}() variants in cleanup guards Lyude Paul

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=20251016081513.GB3289052@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bigeasy@linutronix.de \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=bsegall@google.com \
    --cc=bvanassche@acm.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=dwmw@amazon.co.uk \
    --cc=gary@garyguo.net \
    --cc=joelagnelf@nvidia.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=ryotkkr98@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=tmgross@umich.edu \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --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®