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

Boqun Feng
2025-10-17T06:48:29Z

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
>