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

[Boqun Feng]

On Tue, Aug 04, 2026 at 11:54:34PM -0700, Boqun Feng wrote:
> 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)
> 

Ok, now I remember the issue of the preempt_count_add() implemented
__preempt_count_nmi_enter(), considering this:

 	// outermost NMI handler
 	// nmi_nesting == 0
 
 	nmi_enter():
	  __preempt_count_nmi_enter():
	    unsigned int _o = NMI_MASK + HARDIRQ_OFFSET;
	    ...
	    __this_cpu_inc(nmi_nesting);
	    // ^ nmi_nesting == 1
	    _o -= (preempt_count() & NMI_MASK);	
	    // ^ _o == NMI_MASK + HARDIRQ_OFFSET because the NMI_MASK bit was not set
	    <NMI start>
	    nmi_enter();
	    // ^ nmi_nesting == 2 and NMI_MASK is set.
	    nmi_exti();
	    // ^ nmi_nesting == 1 so NMI_MASK is *still* set
	    <NMI end>
	    __preempt_count_add(_o);	
	    // ^ NMI_MASK overflows because of the addition.

Make sense?

But maybe there are clever ways that I don't know. Please do tell!
	    
Regards,
Boqun

> 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)