Re: [PATCH v13 05/17] irq & spin_lock: Add counted interrupt disabling/enabling

[David Laight <david.laight.linux@gmail.com>]

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.

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