Re: [PATCH v3 1/4] tracing/preemptirq: Optimize preempt_disable/enable() tracepoint overhead

[Wander Lairson Costa <wander@redhat.com>]

On Wed, Mar 11, 2026 at 08:35:03PM +0100, Peter Zijlstra wrote:
> On Wed, Mar 11, 2026 at 09:50:15AM -0300, Wander Lairson Costa wrote:
> 
> > +extern void __trace_preempt_on(void);
> > +extern void __trace_preempt_off(void);
> > +
> > +DECLARE_TRACEPOINT(preempt_enable);
> > +DECLARE_TRACEPOINT(preempt_disable);
> > +
> > +#define __preempt_trace_enabled(type, val) \
> > +	(tracepoint_enabled(preempt_##type) && preempt_count() == (val))
> > +
> > +static __always_inline void preempt_count_add(int val)
> > +{
> > +	__preempt_count_add(val);
> > +
> > +	if (__preempt_trace_enabled(disable, val))
> > +		__trace_preempt_off();
> > +}
> > +
> > +static __always_inline void preempt_count_sub(int val)
> > +{
> > +	if (__preempt_trace_enabled(enable, val))
> > +		__trace_preempt_on();
> > +
> > +	__preempt_count_sub(val);
> > +}
> >  #else
> >  #define preempt_count_add(val)	__preempt_count_add(val)
> >  #define preempt_count_sub(val)	__preempt_count_sub(val)
> >  #define preempt_count_dec_and_test() __preempt_count_dec_and_test()
> >  #endif
> >  
> > +#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
> > +#define preempt_count_dec_and_test() \
> > +	({ preempt_count_sub(1); should_resched(0); })
> > +#endif
> 
> Why!?!
> 
> Why can't you simply have:
> 
> static __always_inline bool preempt_count_dec_and_test(void)
> {
> 	if (__preempt_trace_enabled(enable, 1))
> 		__trace_preempt_on();
> 
> 	return __preempt_count_dec_and_test();
> }

Indeed it improved the generated code. Thanks a lot.

> 
> Also, given how !x86 architectures were just complaining about how
> terrible their preempt_emable() is, I'm really not liking this much at
> all.
> 

I will look deeper in arm64 and riscv generated code. If there are other
specific architectures that concerns you, please let me know.

> Currently the x86 preempt_disable() is _1_ instruction and
> preempt_enable() is all of 3. Adding in these tracepoints will bloat
> every single such site by at least another 4-5.
> 

Yes, there is a tradeoff. As I said before, I am looking at the hot path
(tracepoint no activated). Furthermore, when CONFIG_TRACE_PREEMPT_TOGGLE
and CONFIG_TRACE_IRQFLAGS_TOGGLE are off (default config), the code
generated is the same, byte by byte, of the stock kernel.

> That's significant bloat, for really very little gain. Realistically
> nobody is going to need these.
> 

Of course, I can't speak for others, but more than once I debugged issues
that those tracepoints had made my life far easier. Those cases convinced
me that such a feature would be worth it. But if you don't see
value and will reject the patches no matter what, nothing can be done,
and I will have to accept defeat.