On Wed, Mar 11, 2026 at 08:43:05PM +0100, Peter Zijlstra wrote:
> On Wed, Mar 11, 2026 at 09:50:18AM -0300, Wander Lairson Costa wrote:
> > +#define local_irq_enable()                         \
> > +   do {                                            \
> > +           if (tracepoint_enabled(irq_enable))     \
> > +                   trace_local_irq_enable();       \
> 
> I'm thinking you didn't even look at the assembly generated :/

Yes, I did. But I was more concerned with the hot path, making sure it
only adds the NOP instruction generated by the static_key machinery.

> 
> Otherwise you would have written this like:
> 
>               if (tracepoint_enabled(irq_enable))
>                       __do_trace_local_irq_enable();
> 
> > +           raw_local_irq_enable();                 \
> > +   } while (0)
> 

Steve already opposed to using internal functions. When trace invoke
public API is available, we can always come back and replace the call.

> Again, this was one instruction, and you clearly didn't bother looking
> at the mess you've generated :/
> 

I might have missed something (as was the case for preempt_enable), but
I spent quite some time looking at the assembly code. I did my best, but
there lots of people far more skilled than me. And patch submission is
the way to connect to these people.


Reply via email to