Re: [PATCH v4 10/17] preempt: Introduce HAS_SEPARATE_PREEMPT_RESCHED_BITS

On Wed, Aug 05, 2026 at 01:41:48AM +0530, Shrikanth Hegde wrote:
> Hi Boqun,
> 
> On 8/4/26 9:44 PM, Boqun Feng wrote:
> > With the changes that enable preempt count to track IRQ disabling
> > nesting, we don't have enough bits in 32-bit preempt count
> > implementation, as a result we move NMI nesting bits out of the 32-bit
> > preempt count. However on the architectures that can support 64-bit
> > preempt count implementation, we can keep the NMI nesting bits in the
> > 32-bit preempt count and avoid maintaining NMI nesting bits outside of
> > the same cache line.
> > 
> 
> [...]
> 
> > --- a/include/linux/hardirq.h
> > +++ b/include/linux/hardirq.h
> > @@ -10,8 +10,6 @@
> >   #include <linux/vtime.h>
> >   #include <asm/hardirq.h>
> > -DECLARE_PER_CPU(unsigned int, nmi_nesting);
> > -
> >   extern void synchronize_irq(unsigned int irq);
> >   extern bool synchronize_hardirq(unsigned int irq);
> > @@ -94,6 +92,37 @@ void irq_exit_rcu(void);
> >   #define arch_nmi_exit()		do { } while (0)
> >   #endif
> > +#ifdef CONFIG_HAS_SEPARATE_PREEMPT_RESCHED_BITS
> > +static __always_inline void __preempt_count_nmi_enter(void)
> > +{
> > +	__preempt_count_add(NMI_OFFSET + HARDIRQ_OFFSET);
> > +}
> > +
> > +static __always_inline void __preempt_count_nmi_exit(void)
> > +{
> > +	__preempt_count_sub(NMI_OFFSET + HARDIRQ_OFFSET);
> > +}
> > +#else
> > +DECLARE_PER_CPU(unsigned int, nmi_nesting);
> > +
> > +#define __preempt_count_nmi_enter()				\
> > +	do {							\
> > +		__preempt_count_add(HARDIRQ_OFFSET);		\
> 
> nit: This limit is because to have the same behavior as other case when
> NMI_BITS=4 right?
> It is not easy to infer that from comment.
> 

It's sort of design by implementation IIUC, previously because of
NMI_BITS=4, we could only support nesting level being 15. And here we
just want to keep the same behavior here.

If your question is why 15 was a good number before this change, I guess
would be it's just a number that is neither too big or too small.

> > +		/* Maximum NMI nesting is 15. */		\
> > +		BUG_ON(__this_cpu_read(nmi_nesting) >= 15);	\
> > +		__this_cpu_inc(nmi_nesting);			\
> > +		preempt_count_set(preempt_count() | NMI_MASK);  \
> 
> 
> Is there a reason preempt count updates are split rather than
> folded into a single preempt_count update?
> 

Keeping the implementation simple is one reason, most architectures
could utilize the HAS_SEPARATE_PREEMPT_RESCHED_BITS for better
performance. So that reduces the importance of having something
complicated but saves one access here. But if you see an optimization
that can be done here, please do share!

Peter had proposed one optimization here:

	#define __preempt_count_nmi_enter()				\
		do {							\
			unsigned int _o = NMI_MASK + HARDIRQ_OFFSET;	\
			/* Maximum NMI nesting is 15. */		\
			BUG_ON(__this_cpu_read(nmi_nesting) >= 15);	\
			__this_cpu_inc(nmi_nesting);			\
			_o -= (preempt_count() & NMI_MASK);		\
			__preempt_count_add(_o);			\
		} while (0)
	
	#define __preempt_count_nmi_exit()				\
		do {							\
			unsigned int _o = HARDIRQ_OFFSET;		\
			if (!__this_cpu_dec_return(nmi_nesting))	\
				_o += NMI_MASK;				\
			__preempt_count_sub(_o);			\
		} while (0)

but it has a problem considering this:

	// outermost NMI handler
	// nmi_nesting == 0

	nmi_enter();
	// ^ nmi_nesting == 1 and NMI_MASK is set.
	...
	nmi_exit():
	  if (!__this_cpu_dec_return(nmi_nesting)) // return true
	    _o += NMI_MASK;
	  <NMI start>
	  nmi_enter();
	  // ^ nmi_nesting == 1 and NMI_MASK is set.
	  nmi_exit();
	  // ^ nmi_nesting == 0 and NMI_MASK is *unset*.
	  <NMI end>

	  preempt_count_sub(_o); // _o == HARDIRQ_OFFSET + NMI_MASK,
	  			 // underflow

(Now think about this, the __preempt_count_nmi_enter() does seems
fine, maybe we can keep that, too tired to remember whether there is any
subtly here... will take another look tomorrow)

Regards,
Boqun


> > +	} while (0)
> > +
> > +#define __preempt_count_nmi_exit()				\
> > +	do {							\
> > +		__preempt_count_sub(HARDIRQ_OFFSET);		\
> > +		if (!__this_cpu_dec_return(nmi_nesting))	\
> > +			preempt_count_set(preempt_count() & ~NMI_MASK); \
> > +	} while (0)
> > +
> > +#endif
> > +
> >   /*
> >    * NMI vs Tracing
> >    * --------------
> > @@ -110,18 +139,14 @@ void irq_exit_rcu(void);
> >   	do {							\
> >   		lockdep_off();					\
> >   		arch_nmi_enter();				\
> > -		/* Maximum NMI nesting is 15. */		\
> > -		BUG_ON(__this_cpu_read(nmi_nesting) >= 15);	\
> > -		__this_cpu_inc(nmi_nesting);			\
> > -		__preempt_count_add(HARDIRQ_OFFSET);		\
> > -		preempt_count_set(preempt_count() | NMI_MASK);	\
> > +		__preempt_count_nmi_enter();			\
> >   	} while (0)
> >   #define nmi_enter()						\
> >   	do {							\
> >   		__nmi_enter();					\
> >   		lockdep_hardirq_enter();			\
> > -		ct_nmi_enter();				\
> > +		ct_nmi_enter();					\
> >   		instrumentation_begin();			\
> >   		ftrace_nmi_enter();				\
> >   		instrumentation_end();				\
> > @@ -129,12 +154,8 @@ void irq_exit_rcu(void);
> >   #define __nmi_exit()						\
> >   	do {							\
> > -		unsigned int nesting;				\
> >   		BUG_ON(!in_nmi());				\
> > -		__preempt_count_sub(HARDIRQ_OFFSET);		\
> > -		nesting = __this_cpu_dec_return(nmi_nesting);	\
> > -		if (!nesting)					\
> > -			preempt_count_set(preempt_count() & ~NMI_MASK);	\
> > +		__preempt_count_nmi_exit();			\
> >   		arch_nmi_exit();				\
> >   		lockdep_on();					\
> >   	} while (0)