Re: [PATCH 1/2] trace/preemptirq: reduce overhead of irq_enable/disable tracepoints

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 26 Jun 2025 11:20:09 -0300
Wander Lairson Costa <wander@redhat.com> wrote:


> @@ -197,9 +198,13 @@ extern void warn_bogus_irq_restore(void);
>   */
>  #ifdef CONFIG_TRACE_IRQFLAGS
>  
> +DECLARE_TRACEPOINT(irq_enable);
> +DECLARE_TRACEPOINT(irq_disable);
> +
>  #define local_irq_enable()				\
>  	do {						\
> -		trace_hardirqs_on();			\
> +		if (tracepoint_enabled(irq_enable))	\
> +			trace_hardirqs_on();		\

If you have both this and lockdep enabled, then this has to be called
regardless if tracing is enabled or not. So this should be:

		if (IS_ENABLED(CONFIG_PROVE_LOCKING) || \
		    tracepoint_enabled(irq_enable))


>  		raw_local_irq_enable();			\
>  	} while (0)
>  
> @@ -207,28 +212,32 @@ extern void warn_bogus_irq_restore(void);
>  	do {						\
>  		bool was_disabled = raw_irqs_disabled();\
>  		raw_local_irq_disable();		\
> -		if (!was_disabled)			\
> +		if (tracepoint_enabled(irq_disable) &&	\
> +		    !was_disabled)			\

And this should be:

		if (IS_ENABLED(CONFIG_PROVE_LOCKING) || \
		    (tracepoint_enabled(irq_disable) && \
		     !was_disabled))


>  			trace_hardirqs_off();		\
>  	} while (0)
>  
>  #define local_irq_save(flags)				\
>  	do {						\
>  		raw_local_irq_save(flags);		\
> -		if (!raw_irqs_disabled_flags(flags))	\
> +		if (tracepoint_enabled(irq_disable) &&	\
> +		    !raw_irqs_disabled_flags(flags))	\
>  			trace_hardirqs_off();		\
>  	} while (0)
>  
>  #define local_irq_restore(flags)			\
>  	do {						\
> -		if (!raw_irqs_disabled_flags(flags))	\
> +		if (tracepoint_enabled(irq_enable) &&	\
> +		    !raw_irqs_disabled_flags(flags))	\
>  			trace_hardirqs_on();		\

Same with these.

-- Steve

>  		raw_local_irq_restore(flags);		\
>  	} while (0)
>  
> -#define safe_halt()				\
> -	do {					\
> -		trace_hardirqs_on();		\
> -		raw_safe_halt();		\
> +#define safe_halt()					\
> +	do {						\
> +		if (tracepoint_enabled(irq_enable))	\
> +			trace_hardirqs_on();		\
> +		raw_safe_halt();			\
>  	} while (0)
>  
>  
> diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
> index 0c42b15c3800..90ee65db4516 100644
> --- a/kernel/trace/trace_preemptirq.c
> +++ b/kernel/trace/trace_preemptirq.c
> @@ -111,6 +111,9 @@ void trace_hardirqs_off(void)
>  }
>  EXPORT_SYMBOL(trace_hardirqs_off);
>  NOKPROBE_SYMBOL(trace_hardirqs_off);
> +
> +EXPORT_TRACEPOINT_SYMBOL(irq_disable);
> +EXPORT_TRACEPOINT_SYMBOL(irq_enable);
>  #endif /* CONFIG_TRACE_IRQFLAGS */
>  
>  #ifdef CONFIG_TRACE_PREEMPT_TOGGLE