Re: [PATCH] locking: Revert switching guards to _irq_{disable,enable}()
From: Boqun Feng
Date: Sat Aug 29 2026 - 16:53:42 EST
On Sat, Aug 29, 2026 at 10:44:28PM +0200, Thomas Gleixner wrote:
> On Fri, Aug 28 2026 at 17:45, Boqun Feng wrote:
> > On Sat, Aug 29, 2026 at 01:11:56AM +0200, Thomas Gleixner wrote:
> >> +static __always_inline void __raw_local_irq_restore(unsigned long cnt)
> >> +{
> >> + debug_assert((preempt_count() & HARDIRQ_DISABLE_MASK) == (cnt + HARDIRQ_DISABLE_OFFSET));
> >> +
> >> + if (!(__preempt_count_sub_return(HARDIRQ_DISABLE_OFFSET) & HARDIRQ_DISABLE_MASK))
> >> + arch_local_irq_enable();
> >> +}
> >> +
> >
> > So we can change the semantics of local_irq_disable(),
> > local_irq_enable(), local_irq_save() and local_irq_restore()? Nice!
> > local_irq_{en,dis}able() are no longer idempotent, and
> > local_irq_{save,restore}() have to pair with each other or a
> > local_irq_{en,dis}able(). This would make things much easier. And TBH, I
> > never think this is an option because there could be so much code not
> > obeying this (at least not on day 1). Now I see your point on getting
> > the design correct at the first place :D
>
> Compared to the insanities I had to handle almost 20 years ago when I
> tried that this has become much easier because lockdep and RT made quite
> some of the nastier lock/local_irq games go away.
>
> There are probably a few other places in cpu idle drivers or in dark
> half maintained driver implementations which might need some care, but I
> expect the overall fallout to be managable. The debug mechanisms should
> help to identify them quickly.
>
Yeah, that's something I was missing, I was too afraid to change API
semantics, but with reasonable debug mechanisms, at least users would
have a pointer to resolve the use case issues. Lesson learned.
I will fix the Rust's side compile error.
Regards,
Boqun