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.
