Re: [PATCH] locking: Revert switching guards to _irq_{disable,enable}()

[Boqun Feng <boqun@kernel.org>]

On Thu, Aug 27, 2026 at 10:29:10PM +0200, Thomas Gleixner wrote:
> On Thu, Aug 27 2026 at 10:30, Thomas Gleixner wrote:
> > On Tue, Aug 25 2026 at 16:28, Boqun Feng wrote:
> > So you can get away with:
> >
> > raw_local_irq_save(flags)
> > {
> > 	flags = count;
> > 	if (!count)
> >         	arch_local_irq_disable();
> >         count++;
> > }
> >
> > raw_local_irq_restore(flags)
> > {
> > 	if (!(count = flags))
> >         	arch_local_irq_enable();
> > }
> >
> > raw_local_irq_disable()
> > {
> >         arch_local_irq_disable();
> >         count = 1;
> > }
> >
> > raw_local_irq_enable()
> > {
> >         count = 0;
> >         arch_local_irq_enable();
> > }
> 
> Actually it can be done way simpler because the nasty case of
> 
>          scoped_guard(lock_irqsave, lock) {
>          	unlock_irq(lock);
>                 lock_irq(lock);
>          }
> 
> is only valid for a single lock guard, because if it's nested then the
> outer lock would lose the interrupt disabled protection. Anything else
> would be a bug on its own and would have long ago blown up in our face.
> 

Right.

> So we can completely ignore flags.
> 
> raw_local_irq_save(flags)
> {
>  	if (!count)
>          	arch_local_irq_disable();
>         count++;
> }
> 
> raw_local_irq_restore(flags)
> {
>  	if (!--count)
>          	arch_local_irq_enable();
> }
> 

These are just local_interrupt_{disable,enable}() (replacing
arch_local_irq_save() with arch_local_irq_disable()) :)

> raw_local_irq_disable()
> {
>          arch_local_irq_disable();
>          count++;
> }
> 
> raw_local_irq_enable()
> {
>          count--;
>          arch_local_irq_enable();
> }
> 
> with a copious amount of debug machinery to catch any oddballs.
> 

Ok, so brainstorm on the oddballs:

# 1: double disable

	local_irq_disable();
	local_irq_disable();
	local_irq_enable();

# 2: double enable

	local_irq_disable();
	local_irq_enable();
	local_irq_enable();

I think these mean we should probably do count = 1 and count = 0 in
irq_{enable,disable}() than count++ and count--?

# 3: only restore once

	local_irq_save(flag1);
	local_irq_save(flag2);
	local_irq_restore(flag1);

# 4: keep restoring

	local_irq_save(flag1);
	local_irq_restore(flag1);
	local_irq_restore(flag1);

these are a bit tricky, I guess we could only fix the users? But we
should not postpone the infrastructure because of these?

> Also note that this never uses irqsave/restore because historically that
> has been way slower than CLI/STI.
> 
> A decade+ ago this used to be up to 30%, but micro architectures
> optimized for it. Still on a SKL it's ~14% and on a Zen3 ~8% slower.
> 

Yes, if we go to the level to unify all irq disabling with counter
tracking then I think using arch_local_irq_disable() is possible and
makes a lot of senses.

Regards,
Boqun

> Thanks,
> 
>         tglx
>