All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: David Laight <david.laight.linux@gmail.com>
Cc: "Lyude Paul" <lyude@redhat.com>,
	rust-for-linux@vger.kernel.org,
	"Thomas Gleixner" <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"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 23:48:24 -0700	[thread overview]
Message-ID: <aPHmuA9kXSn438v5@tardis.local> (raw)
In-Reply-To: <20251016222421.512ca8d1@pumpkin>

On Thu, Oct 16, 2025 at 10:24:21PM +0100, David Laight wrote:
> On Mon, 13 Oct 2025 11:48:07 -0400
> Lyude Paul <lyude@redhat.com> wrote:
> 
> > From: Boqun Feng <boqun.feng@gmail.com>
> > 
> > Currently the nested interrupt disabling and enabling is present by
> > _irqsave() and _irqrestore() APIs, which are relatively unsafe, for
> > example:
> > 
> > 	<interrupts are enabled as beginning>
> > 	spin_lock_irqsave(l1, flag1);
> > 	spin_lock_irqsave(l2, flag2);
> > 	spin_unlock_irqrestore(l1, flags1);
> > 	<l2 is still held but interrupts are enabled>
> > 	// accesses to interrupt-disable protect data will cause races.
> 
> To do this right you have to correctly 'nest' the flags even though
> the locks are chained.
> So you should have:
> 	spin_unlock_irqrestore(l1, flags2);
> Which is one reason why schemes that save the interrupt state in the
> lock are completely broken.
> 
> Did you consider a scheme where the interrupt disable count is held in a
> per-cpu variable (rather than on-stack)?
> It might have to be the same per-cpu variable that is used for disabling
> pre-emption.
> If you add (say) 256 to disable interrupts and do the hardware disable
> when the count ends up between 256 and 511 and the enable on the opposite
> transition I think it should work.
> An interrupt after the increment will be fine - it can't do a process
> switch.
> 

This patch is exactly about using percpu (in this case it's the preempt
count) to track interrupt disabling nested level and enabling interrupts
when the count reaches to 0 ;-)

Regards,
Boqun

> The read-add-write doesn't even need to be atomic.
> The problem is a process switch and that can only happen when the only
> value is zero - so it doesn't matter it is can from a different cpu!
> 
> I know some systems (I think including x86) have only incremented such a
> counter instead of doing the hardware interrupt disable.
> When an interrupt happens they realise it shouldn't have, block the IRQ,
> remember there is a deferred interrupt, and return from the ISR.
> This is good for very short disables - because the chance of an IRQ
> is low.
> 
> 	David
>  

  reply	other threads:[~2025-10-17  6:48 UTC|newest]

Thread overview: 49+ 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-10-13 15:48   ` Lyude Paul
2025-11-04 12:30   ` Andreas Hindborg
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-14  4:45   ` kernel test robot
2025-10-15 20:54   ` Bart Van Assche
2025-10-16  8:15     ` Peter Zijlstra
2025-10-17  6:44       ` Boqun Feng
2025-10-16 21:24   ` David Laight
2025-10-17  6:48     ` Boqun Feng [this message]
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=aPHmuA9kXSn438v5@tardis.local \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bigeasy@linutronix.de \
    --cc=bjorn3_gh@protonmail.com \
    --cc=bsegall@google.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=david.laight.linux@gmail.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=peterz@infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.