[PATCH v3 0/2] tracing/preemptirq: Optimize disabled tracepoint overhead

[PATCH v3 0/2] tracing/preemptirq: Optimize disabled tracepoint overhead

[Wander Lairson Costa]

This series addresses unnecessary overhead introduced by the
preempt/irq tracepoints when they are compiled into the kernel
but are not actively enabled (i.e., when tracing is disabled).

These optimizations ensure that when tracing is inactive, the kernel
can largely bypass operations that would otherwise incur a passive
performance penalty. This makes the impact of disabled preemptirq
IRQ and preempt tracing negligible in performance-sensitive environments.

---
Performance Measurements

Measurements were taken using a specialized kernel module [1] to benchmark
`local_irq_disable/enable()` and `preempt_disable/enable()` call pairs.
The kernel used for benchmarking was version 6.16.0-rc2. "Max Average"
represents the average of the 1000 highest samples, used to reduce noise
from single highest samples.

Each benchmark run collected 10^7 samples in parallel from each CPU
for each call pair (used for average, max_avg, and median calculations).
The 99th percentile was measured in a separate benchmark run, focused
on a single CPU.

The results show that compiling with tracers (Kernel Build:
`-master-trace`) introduced significant overhead compared to a base
kernel without tracers (Kernel Build: `-master`). After applying these
patches (Kernel Build: `-patched-trace`), the overhead is
substantially reduced, approaching the baseline.

x86-64 Metrics

Tests were run on a system equipped with an Intel(R) Xeon(R) Silver 4310 CPU.

IRQ Metrics

Combined Metric            average  max_avg  median  percentile
Kernel Build
6.16.0-rc2-master               28     5587      29          23
6.16.0-rc2-master-trace         46     7895      48          32
6.16.0-rc2-patched-trace        30     6030      31          27

Preempt Metrics

Combined Metric            average  max_avg  median  percentile
Kernel Build
6.16.0-rc2-master               26     5748      27          20
6.16.0-rc2-master-trace         45     7526      48          26
6.16.0-rc2-patched-trace        27     5479      27          21

AArch64 Metrics

Tests were also conducted on an AArch64 platform.

IRQ Metrics

Combined Metric             average  max_avg  median  percentile
Kernel Build
aarch64-6.16.0-rc2-master        28     3298      32          64
aarch64-6.16.0-rc2-master-trace 105     5769      96         128
aarch64-6.16.0-rc2-patched-trace 29     3192      32          64

Preempt Metrics

Combined Metric             average  max_avg  median  percentile
Kernel Build
aarch64-6.16.0-rc2-master        27     3371      32          32
aarch64-6.16.0-rc2-master-trace  32     3000      32          64
aarch64-6.16.0-rc2-patched-trace 28     3132      32          64

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Clark Williams <williams@redhat.com>
Cc: Gabriele Monaco <gmonaco@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>

---
References:
[1] https://github.com/walac/tracer-benchmark

--
Changes:
v1: Initial version of the patch.
v2: Enabled IRQ tracing automatically when CONFIG_PROVE_LOCKING is active.
v3: Resolved a build failure on the 32-bit ARM architecture.

Wander Lairson Costa (2):
  trace/preemptirq: reduce overhead of irq_enable/disable tracepoints
  tracing/preemptirq: Optimize preempt_disable/enable() tracepoint
    overhead

 include/linux/irqflags.h        | 30 +++++++++++++++++++---------
 include/linux/preempt.h         | 35 ++++++++++++++++++++++++++++++---
 include/linux/tracepoint-defs.h |  1 -
 include/linux/tracepoint.h      |  1 +
 kernel/sched/core.c             | 12 +----------
 kernel/trace/trace_preemptirq.c | 22 +++++++++++++++++++++
 6 files changed, 77 insertions(+), 24 deletions(-)

-- 
2.50.0

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

[Wander Lairson Costa]

The irqsoff tracer is rarely enabled in production systems due to the
non-negligible overhead it introduces—even when unused. This is caused
by how trace_hardirqs_on/off() are always invoked in
local_irq_enable/disable(), evaluate the tracepoint static key.

This patch reduces the overhead in the common case where the tracepoint
is disabled by performing the static key check earlier, avoiding the
call to trace_hardirqs_on/off() entirely.

This makes the impact of disabled preemptirq IRQ tracing negligible in
performance-sensitive environments.

We also move the atomic.h include from tracepoint-defs.h to tracepoint.h
due a circular dependency when building 32 bits ARM.

The failure occurs because the new logic in <linux/irqflags.h> calls
tracepoint_enabled(), which requires the tracepoint-defs.h header file.
This header, in turn, includes <linux/atomic.h>. On ARM32, the include
path for kernel/bounds.c creates a circular dependency:
atomic.h -> cmpxchg.h -> irqflags.h -> tracepoint.h -> atomic.h

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Clark Williams <williams@redhat.com>
Cc: Gabriele Monaco <gmonaco@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
---
 include/linux/irqflags.h        | 30 +++++++++++++++++++++---------
 include/linux/tracepoint-defs.h |  1 -
 include/linux/tracepoint.h      |  1 +
 kernel/trace/trace_preemptirq.c |  3 +++
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 57b074e0cfbb..40e456fa3d10 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -17,6 +17,7 @@
 #include <linux/cleanup.h>
 #include <asm/irqflags.h>
 #include <asm/percpu.h>
+#include <linux/tracepoint-defs.h>
 
 struct task_struct;
 
@@ -197,9 +198,17 @@ extern void warn_bogus_irq_restore(void);
  */
 #ifdef CONFIG_TRACE_IRQFLAGS
 
+DECLARE_TRACEPOINT(irq_enable);
+DECLARE_TRACEPOINT(irq_disable);
+
+#define __trace_enabled(tp)				\
+	(IS_ENABLED(CONFIG_PROVE_LOCKING) ||		\
+	 tracepoint_enabled(tp))
+
 #define local_irq_enable()				\
 	do {						\
-		trace_hardirqs_on();			\
+		if (__trace_enabled(irq_enable))	\
+			trace_hardirqs_on();		\
 		raw_local_irq_enable();			\
 	} while (0)
 
@@ -207,31 +216,34 @@ extern void warn_bogus_irq_restore(void);
 	do {						\
 		bool was_disabled = raw_irqs_disabled();\
 		raw_local_irq_disable();		\
-		if (!was_disabled)			\
+		if (__trace_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 (__trace_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 (__trace_enabled(irq_enable) &&	\
+		    !raw_irqs_disabled_flags(flags))	\
 			trace_hardirqs_on();		\
 		raw_local_irq_restore(flags);		\
 	} while (0)
 
-#define safe_halt()				\
-	do {					\
-		trace_hardirqs_on();		\
-		raw_safe_halt();		\
+#define safe_halt()					\
+	do {						\
+		if (__trace_enabled(irq_enable))	\
+			trace_hardirqs_on();		\
+		raw_safe_halt();			\
 	} while (0)
 
-
 #else /* !CONFIG_TRACE_IRQFLAGS */
 
 #define local_irq_enable()	do { raw_local_irq_enable(); } while (0)
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index aebf0571c736..cb1f15a4e43f 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -8,7 +8,6 @@
  * trace_print_flags{_u64}. Otherwise linux/tracepoint.h should be used.
  */
 
-#include <linux/atomic.h>
 #include <linux/static_key.h>
 
 struct static_call_key;
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 826ce3f8e1f8..2fd91ef49b7f 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -20,6 +20,7 @@
 #include <linux/rcupdate_trace.h>
 #include <linux/tracepoint-defs.h>
 #include <linux/static_call.h>
+#include <linux/atomic.h>
 
 struct module;
 struct tracepoint;
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
-- 
2.50.0

[PATCH v3 2/2] tracing/preemptirq: Optimize preempt_disable/enable() tracepoint overhead

[Wander Lairson Costa]

Similar to the IRQ tracepoint, the preempt tracepoints are typically
disabled in production systems due to the significant overhead they
introduce even when not in use.

The overhead primarily comes from two sources: First, when tracepoints
are compiled into the kernel, preempt_count_add() and preempt_count_sub()
become external function calls rather than inlined operations. Second,
these functions perform unnecessary preempt_count() checks even when the
tracepoint itself is disabled.

This optimization introduces an early check of the tracepoint static key,
which allows us to skip both the function call overhead and the redundant
preempt_count() checks when tracing is disabled. The change maintains all
existing functionality when tracing is active while significantly
reducing overhead for the common case where tracing is inactive.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Clark Williams <williams@redhat.com>
Cc: Gabriele Monaco <gmonaco@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
---
 include/linux/preempt.h         | 35 ++++++++++++++++++++++++++++++---
 kernel/sched/core.c             | 12 +----------
 kernel/trace/trace_preemptirq.c | 19 ++++++++++++++++++
 3 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index b0af8d4ef6e6..d13c755cd934 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -10,6 +10,7 @@
 #include <linux/linkage.h>
 #include <linux/cleanup.h>
 #include <linux/types.h>
+#include <linux/tracepoint-defs.h>
 
 /*
  * We put the hardirq and softirq counter into the preemption
@@ -191,17 +192,45 @@ static __always_inline unsigned char interrupt_context_level(void)
  */
 #define in_atomic_preempt_off() (preempt_count() != PREEMPT_DISABLE_OFFSET)
 
-#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
+#if defined(CONFIG_DEBUG_PREEMPT)
 extern void preempt_count_add(int val);
 extern void preempt_count_sub(int val);
-#define preempt_count_dec_and_test() \
-	({ preempt_count_sub(1); should_resched(0); })
+#elif defined(CONFIG_TRACE_PREEMPT_TOGGLE)
+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) \
+	(tracepoint_enabled(preempt_##type) && preempt_count() == val)
+
+static inline void preempt_count_add(int val)
+{
+	__preempt_count_add(val);
+
+	if (__preempt_trace_enabled(disable))
+		__trace_preempt_off();
+}
+
+static inline void preempt_count_sub(int val)
+{
+	if (__preempt_trace_enabled(enable))
+		__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
+
 #define __preempt_count_inc() __preempt_count_add(1)
 #define __preempt_count_dec() __preempt_count_sub(1)
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8988d38d46a3..4feba4738d79 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5840,8 +5840,7 @@ static inline void sched_tick_start(int cpu) { }
 static inline void sched_tick_stop(int cpu) { }
 #endif
 
-#if defined(CONFIG_PREEMPTION) && (defined(CONFIG_DEBUG_PREEMPT) || \
-				defined(CONFIG_TRACE_PREEMPT_TOGGLE))
+#if defined(CONFIG_PREEMPTION) && defined(CONFIG_DEBUG_PREEMPT)
 /*
  * If the value passed in is equal to the current preempt count
  * then we just disabled preemption. Start timing the latency.
@@ -5850,30 +5849,24 @@ static inline void preempt_latency_start(int val)
 {
 	if (preempt_count() == val) {
 		unsigned long ip = get_lock_parent_ip();
-#ifdef CONFIG_DEBUG_PREEMPT
 		current->preempt_disable_ip = ip;
-#endif
 		trace_preempt_off(CALLER_ADDR0, ip);
 	}
 }
 
 void preempt_count_add(int val)
 {
-#ifdef CONFIG_DEBUG_PREEMPT
 	/*
 	 * Underflow?
 	 */
 	if (DEBUG_LOCKS_WARN_ON((preempt_count() < 0)))
 		return;
-#endif
 	__preempt_count_add(val);
-#ifdef CONFIG_DEBUG_PREEMPT
 	/*
 	 * Spinlock count overflowing soon?
 	 */
 	DEBUG_LOCKS_WARN_ON((preempt_count() & PREEMPT_MASK) >=
 				PREEMPT_MASK - 10);
-#endif
 	preempt_latency_start(val);
 }
 EXPORT_SYMBOL(preempt_count_add);
@@ -5891,7 +5884,6 @@ static inline void preempt_latency_stop(int val)
 
 void preempt_count_sub(int val)
 {
-#ifdef CONFIG_DEBUG_PREEMPT
 	/*
 	 * Underflow?
 	 */
@@ -5903,14 +5895,12 @@ void preempt_count_sub(int val)
 	if (DEBUG_LOCKS_WARN_ON((val < PREEMPT_MASK) &&
 			!(preempt_count() & PREEMPT_MASK)))
 		return;
-#endif
 
 	preempt_latency_stop(val);
 	__preempt_count_sub(val);
 }
 EXPORT_SYMBOL(preempt_count_sub);
 NOKPROBE_SYMBOL(preempt_count_sub);
-
 #else
 static inline void preempt_latency_start(int val) { }
 static inline void preempt_latency_stop(int val) { }
diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
index 90ee65db4516..deb2428b34a2 100644
--- a/kernel/trace/trace_preemptirq.c
+++ b/kernel/trace/trace_preemptirq.c
@@ -118,6 +118,25 @@ EXPORT_TRACEPOINT_SYMBOL(irq_enable);
 
 #ifdef CONFIG_TRACE_PREEMPT_TOGGLE
 
+#if !defined(CONFIG_DEBUG_PREEMPT)
+EXPORT_SYMBOL(__tracepoint_preempt_disable);
+EXPORT_SYMBOL(__tracepoint_preempt_enable);
+
+void __trace_preempt_on(void)
+{
+	trace_preempt_on(CALLER_ADDR0, get_lock_parent_ip());
+}
+EXPORT_SYMBOL(__trace_preempt_on);
+NOKPROBE_SYMBOL(__trace_preempt_on);
+
+void __trace_preempt_off(void)
+{
+	trace_preempt_off(CALLER_ADDR0, get_lock_parent_ip());
+}
+EXPORT_SYMBOL(__trace_preempt_off);
+NOKPROBE_SYMBOL(__trace_preempt_off);
+#endif /* !CONFIG_DEBUG_PREEMPT */
+
 void trace_preempt_on(unsigned long a0, unsigned long a1)
 {
 	trace(preempt_enable, TP_ARGS(a0, a1));
-- 
2.50.0

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

[kernel test robot]

Hi Wander,

kernel test robot noticed the following build errors:

[auto build test ERROR on trace/for-next]
[also build test ERROR on tip/sched/core linus/master v6.16-rc4 next-20250704]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wander-Lairson-Costa/trace-preemptirq-reduce-overhead-of-irq_enable-disable-tracepoints/20250705-011058
base:   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link:    https://lore.kernel.org/r/20250704170748.97632-2-wander%40redhat.com
patch subject: [PATCH v3 1/2] trace/preemptirq: reduce overhead of irq_enable/disable tracepoints
config: nios2-randconfig-r123-20250706 (https://download.01.org/0day-ci/archive/20250706/202507061226.y7g5eBuq-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 8.5.0
reproduce: (https://download.01.org/0day-ci/archive/20250706/202507061226.y7g5eBuq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507061226.y7g5eBuq-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from ./arch/nios2/include/generated/asm/cmpxchg.h:1,
                    from include/asm-generic/atomic.h:12,
                    from ./arch/nios2/include/generated/asm/atomic.h:1,
                    from include/linux/atomic.h:7,
                    from include/linux/jump_label.h:257,
                    from include/linux/static_key.h:1,
                    from include/linux/tracepoint-defs.h:11,
                    from include/linux/irqflags.h:20,
                    from include/asm-generic/bitops.h:14,
                    from ./arch/nios2/include/generated/asm/bitops.h:1,
                    from include/linux/bitops.h:68,
                    from include/linux/log2.h:12,
                    from kernel/bounds.c:13:
   include/asm-generic/cmpxchg.h: In function '__generic_xchg':
>> include/asm-generic/cmpxchg.h:33:3: error: implicit declaration of function 'local_irq_save'; did you mean 'arch_local_irq_save'? [-Werror=implicit-function-declaration]
      local_irq_save(flags);
      ^~~~~~~~~~~~~~
      arch_local_irq_save
>> include/asm-generic/cmpxchg.h:36:3: error: implicit declaration of function 'local_irq_restore'; did you mean 'arch_local_irq_restore'? [-Werror=implicit-function-declaration]
      local_irq_restore(flags);
      ^~~~~~~~~~~~~~~~~
      arch_local_irq_restore
   In file included from include/asm-generic/cmpxchg.h:89,
                    from ./arch/nios2/include/generated/asm/cmpxchg.h:1,
                    from include/asm-generic/atomic.h:12,
                    from ./arch/nios2/include/generated/asm/atomic.h:1,
                    from include/linux/atomic.h:7,
                    from include/linux/jump_label.h:257,
                    from include/linux/static_key.h:1,
                    from include/linux/tracepoint-defs.h:11,
                    from include/linux/irqflags.h:20,
                    from include/asm-generic/bitops.h:14,
                    from ./arch/nios2/include/generated/asm/bitops.h:1,
                    from include/linux/bitops.h:68,
                    from include/linux/log2.h:12,
                    from kernel/bounds.c:13:
   include/asm-generic/cmpxchg-local.h: In function '__generic_cmpxchg_local':
>> include/asm-generic/cmpxchg-local.h:26:2: error: implicit declaration of function 'raw_local_irq_save'; did you mean 'arch_local_irq_save'? [-Werror=implicit-function-declaration]
     raw_local_irq_save(flags);
     ^~~~~~~~~~~~~~~~~~
     arch_local_irq_save
>> include/asm-generic/cmpxchg-local.h:47:2: error: implicit declaration of function 'raw_local_irq_restore'; did you mean 'arch_local_irq_restore'? [-Werror=implicit-function-declaration]
     raw_local_irq_restore(flags);
     ^~~~~~~~~~~~~~~~~~~~~
     arch_local_irq_restore
   cc1: some warnings being treated as errors
   make[3]: *** [scripts/Makefile.build:98: kernel/bounds.s] Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1274: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +33 include/asm-generic/cmpxchg.h

b4816afa3986704 David Howells 2012-03-28  22  
b4816afa3986704 David Howells 2012-03-28  23  static inline
82b993e8249ae3c Mark Rutland  2021-05-25  24  unsigned long __generic_xchg(unsigned long x, volatile void *ptr, int size)
b4816afa3986704 David Howells 2012-03-28  25  {
b4816afa3986704 David Howells 2012-03-28  26  	unsigned long ret, flags;
b4816afa3986704 David Howells 2012-03-28  27  
b4816afa3986704 David Howells 2012-03-28  28  	switch (size) {
b4816afa3986704 David Howells 2012-03-28  29  	case 1:
b4816afa3986704 David Howells 2012-03-28  30  #ifdef __xchg_u8
b4816afa3986704 David Howells 2012-03-28  31  		return __xchg_u8(x, ptr);
b4816afa3986704 David Howells 2012-03-28  32  #else
b4816afa3986704 David Howells 2012-03-28 @33  		local_irq_save(flags);
b4816afa3986704 David Howells 2012-03-28  34  		ret = *(volatile u8 *)ptr;
656e9007ef58627 Arnd Bergmann 2023-03-02  35  		*(volatile u8 *)ptr = (x & 0xffu);
b4816afa3986704 David Howells 2012-03-28 @36  		local_irq_restore(flags);
b4816afa3986704 David Howells 2012-03-28  37  		return ret;
b4816afa3986704 David Howells 2012-03-28  38  #endif /* __xchg_u8 */
b4816afa3986704 David Howells 2012-03-28  39  
b4816afa3986704 David Howells 2012-03-28  40  	case 2:
b4816afa3986704 David Howells 2012-03-28  41  #ifdef __xchg_u16
b4816afa3986704 David Howells 2012-03-28  42  		return __xchg_u16(x, ptr);
b4816afa3986704 David Howells 2012-03-28  43  #else
b4816afa3986704 David Howells 2012-03-28  44  		local_irq_save(flags);
b4816afa3986704 David Howells 2012-03-28  45  		ret = *(volatile u16 *)ptr;
656e9007ef58627 Arnd Bergmann 2023-03-02  46  		*(volatile u16 *)ptr = (x & 0xffffu);
b4816afa3986704 David Howells 2012-03-28  47  		local_irq_restore(flags);
b4816afa3986704 David Howells 2012-03-28  48  		return ret;
b4816afa3986704 David Howells 2012-03-28  49  #endif /* __xchg_u16 */
b4816afa3986704 David Howells 2012-03-28  50  
b4816afa3986704 David Howells 2012-03-28  51  	case 4:
b4816afa3986704 David Howells 2012-03-28  52  #ifdef __xchg_u32
b4816afa3986704 David Howells 2012-03-28  53  		return __xchg_u32(x, ptr);
b4816afa3986704 David Howells 2012-03-28  54  #else
b4816afa3986704 David Howells 2012-03-28  55  		local_irq_save(flags);
b4816afa3986704 David Howells 2012-03-28  56  		ret = *(volatile u32 *)ptr;
656e9007ef58627 Arnd Bergmann 2023-03-02  57  		*(volatile u32 *)ptr = (x & 0xffffffffu);
b4816afa3986704 David Howells 2012-03-28  58  		local_irq_restore(flags);
b4816afa3986704 David Howells 2012-03-28  59  		return ret;
b4816afa3986704 David Howells 2012-03-28  60  #endif /* __xchg_u32 */
b4816afa3986704 David Howells 2012-03-28  61  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Wander,

kernel test robot noticed the following build errors:

[auto build test ERROR on trace/for-next]
[also build test ERROR on tip/sched/core linus/master v6.16-rc5 next-20250704]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wander-Lairson-Costa/trace-preemptirq-reduce-overhead-of-irq_enable-disable-tracepoints/20250705-011058
base:   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link:    https://lore.kernel.org/r/20250704170748.97632-3-wander%40redhat.com
patch subject: [PATCH v3 2/2] tracing/preemptirq: Optimize preempt_disable/enable() tracepoint overhead
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250707/202507071800.OViM30Fr-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250707/202507071800.OViM30Fr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507071800.OViM30Fr-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/irqflags.h:18,
                    from arch/m68k/include/asm/atomic.h:6,
                    from include/linux/atomic.h:7,
                    from include/linux/jump_label.h:257,
                    from include/linux/static_key.h:1,
                    from include/linux/tracepoint-defs.h:11,
                    from include/linux/preempt.h:13,
                    from arch/m68k/include/asm/processor.h:11,
                    from include/linux/sched.h:13,
                    from arch/m68k/kernel/asm-offsets.c:15:
   arch/m68k/include/asm/irqflags.h: In function 'arch_local_irq_enable':
>> arch/m68k/include/asm/irqflags.h:44:29: error: implicit declaration of function 'hardirq_count' [-Wimplicit-function-declaration]
      44 |         if (MACH_IS_Q40 || !hardirq_count())
         |                             ^~~~~~~~~~~~~
   make[3]: *** [scripts/Makefile.build:98: arch/m68k/kernel/asm-offsets.s] Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1274: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +/hardirq_count +44 arch/m68k/include/asm/irqflags.h

df9ee29270c11d David Howells 2010-10-07  31  
df9ee29270c11d David Howells 2010-10-07  32  static inline void arch_local_irq_enable(void)
df9ee29270c11d David Howells 2010-10-07  33  {
df9ee29270c11d David Howells 2010-10-07  34  #if defined(CONFIG_COLDFIRE)
df9ee29270c11d David Howells 2010-10-07  35  	asm volatile (
df9ee29270c11d David Howells 2010-10-07  36  		"move	%/sr,%%d0	\n\t"
df9ee29270c11d David Howells 2010-10-07  37  		"andi.l	#0xf8ff,%%d0	\n\t"
df9ee29270c11d David Howells 2010-10-07  38  		"move	%%d0,%/sr	\n"
df9ee29270c11d David Howells 2010-10-07  39  		: /* no outputs */
df9ee29270c11d David Howells 2010-10-07  40  		:
df9ee29270c11d David Howells 2010-10-07  41  		: "cc", "%d0", "memory");
df9ee29270c11d David Howells 2010-10-07  42  #else
df9ee29270c11d David Howells 2010-10-07  43  # if defined(CONFIG_MMU)
df9ee29270c11d David Howells 2010-10-07 @44  	if (MACH_IS_Q40 || !hardirq_count())
df9ee29270c11d David Howells 2010-10-07  45  # endif
df9ee29270c11d David Howells 2010-10-07  46  		asm volatile (
df9ee29270c11d David Howells 2010-10-07  47  			"andiw %0,%%sr"
df9ee29270c11d David Howells 2010-10-07  48  			:
df9ee29270c11d David Howells 2010-10-07  49  			: "i" (ALLOWINT)
df9ee29270c11d David Howells 2010-10-07  50  			: "memory");
df9ee29270c11d David Howells 2010-10-07  51  #endif
df9ee29270c11d David Howells 2010-10-07  52  }
df9ee29270c11d David Howells 2010-10-07  53  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Peter Zijlstra]

On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> Similar to the IRQ tracepoint, the preempt tracepoints are typically
> disabled in production systems due to the significant overhead they
> introduce even when not in use.
> 
> The overhead primarily comes from two sources: First, when tracepoints
> are compiled into the kernel, preempt_count_add() and preempt_count_sub()
> become external function calls rather than inlined operations. Second,
> these functions perform unnecessary preempt_count() checks even when the
> tracepoint itself is disabled.
> 
> This optimization introduces an early check of the tracepoint static key,
> which allows us to skip both the function call overhead and the redundant
> preempt_count() checks when tracing is disabled. The change maintains all
> existing functionality when tracing is active while significantly
> reducing overhead for the common case where tracing is inactive.
> 

This one in particular I worry about the code gen impact. There are a
*LOT* of preempt_{dis,en}able() sites in the kernel and now they all get
this static branch and call crud on.

We spend significant effort to make preempt_{dis,en}able() as small as
possible.

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

[Peter Zijlstra]

On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> +#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
> +#define preempt_count_dec_and_test() \
> +	({ preempt_count_sub(1); should_resched(0); })
> +#endif

Also this is terrible. Surely you can do better.

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

[Wander Lairson Costa]

O Mon, Jul 07, 2025 at 01:20:03PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> > Similar to the IRQ tracepoint, the preempt tracepoints are typically
> > disabled in production systems due to the significant overhead they
> > introduce even when not in use.
> > 
> > The overhead primarily comes from two sources: First, when tracepoints
> > are compiled into the kernel, preempt_count_add() and preempt_count_sub()
> > become external function calls rather than inlined operations. Second,
> > these functions perform unnecessary preempt_count() checks even when the
> > tracepoint itself is disabled.
> > 
> > This optimization introduces an early check of the tracepoint static key,
> > which allows us to skip both the function call overhead and the redundant
> > preempt_count() checks when tracing is disabled. The change maintains all
> > existing functionality when tracing is active while significantly
> > reducing overhead for the common case where tracing is inactive.
> > 
> 
> This one in particular I worry about the code gen impact. There are a
> *LOT* of preempt_{dis,en}able() sites in the kernel and now they all get
> this static branch and call crud on.
> 
> We spend significant effort to make preempt_{dis,en}able() as small as
> possible.
> 

Thank you for the feedback, it's much appreciated. I just want to make sure
I'm on the right track. If I understand your concern correctly, it revolves
around the overhead this patch might introduce—specifically to the binary
size and its effect on the iCache—when the kernel is built with preempt
tracepoints enabled. Is that an accurate summary?

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

[Wander Lairson Costa]

On Mon, Jul 07, 2025 at 01:26:22PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> > +#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
> > +#define preempt_count_dec_and_test() \
> > +	({ preempt_count_sub(1); should_resched(0); })
> > +#endif
> 
> Also this is terrible. Surely you can do better.
> 

Thank you for pointing this out. I'm not sure I've fully understood the
concern here. My understanding was that this logic was pre-existing and
my patch only reorganized it.

I'm clearly missing something. Could you please elaborate a bit on the
issue you've spotted?

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

[Peter Zijlstra]

On Tue, Jul 08, 2025 at 10:09:45AM -0300, Wander Lairson Costa wrote:
> On Mon, Jul 07, 2025 at 01:26:22PM +0200, Peter Zijlstra wrote:
> > On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> > > +#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
> > > +#define preempt_count_dec_and_test() \
> > > +	({ preempt_count_sub(1); should_resched(0); })
> > > +#endif
> > 
> > Also this is terrible. Surely you can do better.
> > 
> 
> Thank you for pointing this out. I'm not sure I've fully understood the
> concern here. My understanding was that this logic was pre-existing and
> my patch only reorganized it.
> 
> I'm clearly missing something. Could you please elaborate a bit on the
> issue you've spotted?

The normal (!DEBUG) case uses __preempt_count_dec_and_test(), which is
significantly better.

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

[Peter Zijlstra]

On Tue, Jul 08, 2025 at 09:54:06AM -0300, Wander Lairson Costa wrote:
> O Mon, Jul 07, 2025 at 01:20:03PM +0200, Peter Zijlstra wrote:
> > On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> > > Similar to the IRQ tracepoint, the preempt tracepoints are typically
> > > disabled in production systems due to the significant overhead they
> > > introduce even when not in use.
> > > 
> > > The overhead primarily comes from two sources: First, when tracepoints
> > > are compiled into the kernel, preempt_count_add() and preempt_count_sub()
> > > become external function calls rather than inlined operations. Second,
> > > these functions perform unnecessary preempt_count() checks even when the
> > > tracepoint itself is disabled.
> > > 
> > > This optimization introduces an early check of the tracepoint static key,
> > > which allows us to skip both the function call overhead and the redundant
> > > preempt_count() checks when tracing is disabled. The change maintains all
> > > existing functionality when tracing is active while significantly
> > > reducing overhead for the common case where tracing is inactive.
> > > 
> > 
> > This one in particular I worry about the code gen impact. There are a
> > *LOT* of preempt_{dis,en}able() sites in the kernel and now they all get
> > this static branch and call crud on.
> > 
> > We spend significant effort to make preempt_{dis,en}able() as small as
> > possible.
> > 
> 
> Thank you for the feedback, it's much appreciated. I just want to make sure
> I'm on the right track. If I understand your concern correctly, it revolves
> around the overhead this patch might introduce???specifically to the binary
> size and its effect on the iCache???when the kernel is built with preempt
> tracepoints enabled. Is that an accurate summary?

Yes, specifically:

preempt_disable()
	incl	%gs:__preempt_count



preempt_enable()
	decl	%gs:__preempt_count
	jz	do_schedule
1:	...

do_schedule:
	call	__SCT__preemptible_schedule
	jmp	1


your proposal adds significantly to this.

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

[Wander Lairson Costa]

On Tue, Jul 8, 2025 at 3:47 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
Sorry for the late reply. A mix of PTO + higher priority backlog items.

> On Tue, Jul 08, 2025 at 10:09:45AM -0300, Wander Lairson Costa wrote:
> > On Mon, Jul 07, 2025 at 01:26:22PM +0200, Peter Zijlstra wrote:
> > > On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> > > > +#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
> > > > +#define preempt_count_dec_and_test() \
> > > > + ({ preempt_count_sub(1); should_resched(0); })
> > > > +#endif
> > >
> > > Also this is terrible. Surely you can do better.
> > >
> >
> > Thank you for pointing this out. I'm not sure I've fully understood the
> > concern here. My understanding was that this logic was pre-existing and
> > my patch only reorganized it.
> >
> > I'm clearly missing something. Could you please elaborate a bit on the
> > issue you've spotted?
>
> The normal (!DEBUG) case uses __preempt_count_dec_and_test(), which is
> significantly better.
>
Maybe I am missing something, but my understanding is that this behavior didn't
change. When DEBUG_PREEMPT and TRACE_PREEMPT_TOGGLE are not defined,
__preempt_count_dec_and_test() is used.

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

[Wander Lairson Costa]

On Tue, Jul 8, 2025 at 3:54 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, Jul 08, 2025 at 09:54:06AM -0300, Wander Lairson Costa wrote:
> > O Mon, Jul 07, 2025 at 01:20:03PM +0200, Peter Zijlstra wrote:
> > > On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> > > > Similar to the IRQ tracepoint, the preempt tracepoints are typically
> > > > disabled in production systems due to the significant overhead they
> > > > introduce even when not in use.
> > > >
> > > > The overhead primarily comes from two sources: First, when tracepoints
> > > > are compiled into the kernel, preempt_count_add() and preempt_count_sub()
> > > > become external function calls rather than inlined operations. Second,
> > > > these functions perform unnecessary preempt_count() checks even when the
> > > > tracepoint itself is disabled.
> > > >
> > > > This optimization introduces an early check of the tracepoint static key,
> > > > which allows us to skip both the function call overhead and the redundant
> > > > preempt_count() checks when tracing is disabled. The change maintains all
> > > > existing functionality when tracing is active while significantly
> > > > reducing overhead for the common case where tracing is inactive.
> > > >
> > >
> > > This one in particular I worry about the code gen impact. There are a
> > > *LOT* of preempt_{dis,en}able() sites in the kernel and now they all get
> > > this static branch and call crud on.
> > >
> > > We spend significant effort to make preempt_{dis,en}able() as small as
> > > possible.
> > >
> >
> > Thank you for the feedback, it's much appreciated. I just want to make sure
> > I'm on the right track. If I understand your concern correctly, it revolves
> > around the overhead this patch might introduce???specifically to the binary
> > size and its effect on the iCache???when the kernel is built with preempt
> > tracepoints enabled. Is that an accurate summary?
>
> Yes, specifically:
>
> preempt_disable()
>         incl    %gs:__preempt_count
>
>
>
> preempt_enable()
>         decl    %gs:__preempt_count
>         jz      do_schedule
> 1:      ...
>
> do_schedule:
>         call    __SCT__preemptible_schedule
>         jmp     1
>
>
> your proposal adds significantly to this.
>

Here is a breakdown of the patch's behavior under the different kernel
configurations:
* When DEBUG_PREEMPT is defined, the behavior is identical to the
current implementation, with calls to preempt_count_add/sub().
* When both DEBUG_PREEMPT and TRACE_PREEMPT_TOGGLE are disabled, the
generated code is also unchanged.
* The primary change occurs when only TRACE_PREEMPT_TOGGLE is defined.
In this case, the code uses a static key test instead of a function
call. As the benchmarks show, this approach is faster when the
tracepoints are disabled.
The main trade-off is that enabling or disabling these tracepoints
will require the kernel to patch more code locations due to the use of
static keys.

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

[Wander Lairson Costa]

Resending in case people missed the previous message.

On Fri, Aug 1, 2025 at 10:30 AM Wander Lairson Costa <wander@redhat.com> wrote:
>
> On Tue, Jul 8, 2025 at 3:54 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Tue, Jul 08, 2025 at 09:54:06AM -0300, Wander Lairson Costa wrote:
> > > O Mon, Jul 07, 2025 at 01:20:03PM +0200, Peter Zijlstra wrote:
> > > > On Fri, Jul 04, 2025 at 02:07:43PM -0300, Wander Lairson Costa wrote:
> > > > > Similar to the IRQ tracepoint, the preempt tracepoints are typically
> > > > > disabled in production systems due to the significant overhead they
> > > > > introduce even when not in use.
> > > > >
> > > > > The overhead primarily comes from two sources: First, when tracepoints
> > > > > are compiled into the kernel, preempt_count_add() and preempt_count_sub()
> > > > > become external function calls rather than inlined operations. Second,
> > > > > these functions perform unnecessary preempt_count() checks even when the
> > > > > tracepoint itself is disabled.
> > > > >
> > > > > This optimization introduces an early check of the tracepoint static key,
> > > > > which allows us to skip both the function call overhead and the redundant
> > > > > preempt_count() checks when tracing is disabled. The change maintains all
> > > > > existing functionality when tracing is active while significantly
> > > > > reducing overhead for the common case where tracing is inactive.
> > > > >
> > > >
> > > > This one in particular I worry about the code gen impact. There are a
> > > > *LOT* of preempt_{dis,en}able() sites in the kernel and now they all get
> > > > this static branch and call crud on.
> > > >
> > > > We spend significant effort to make preempt_{dis,en}able() as small as
> > > > possible.
> > > >
> > >
> > > Thank you for the feedback, it's much appreciated. I just want to make sure
> > > I'm on the right track. If I understand your concern correctly, it revolves
> > > around the overhead this patch might introduce???specifically to the binary
> > > size and its effect on the iCache???when the kernel is built with preempt
> > > tracepoints enabled. Is that an accurate summary?
> >
> > Yes, specifically:
> >
> > preempt_disable()
> >         incl    %gs:__preempt_count
> >
> >
> >
> > preempt_enable()
> >         decl    %gs:__preempt_count
> >         jz      do_schedule
> > 1:      ...
> >
> > do_schedule:
> >         call    __SCT__preemptible_schedule
> >         jmp     1
> >
> >
> > your proposal adds significantly to this.
> >
>
Here is a breakdown of the patch's behavior under the different kernel
configurations:
* When DEBUG_PREEMPT is defined, the behavior is identical to the
current implementation, with calls to preempt_count_add/sub().
* When both DEBUG_PREEMPT and TRACE_PREEMPT_TOGGLE are disabled, the
generated code is also unchanged.
* The primary change occurs when only TRACE_PREEMPT_TOGGLE is defined.
In this case, the code uses a static key test instead of a function
call. As the benchmarks show, this approach is faster when the
tracepoints are disabled.
The main trade-off is that enabling or disabling these tracepoints
will require the kernel to patch more code locations due to the use of
static keys.