[PATCH] tracing: Keep preemption disabled until the recursion bit is cleared

[PATCH] tracing: Keep preemption disabled until the recursion bit is cleared

[Karl Mehltretter]

Function tracing can drop calls in interrupt entry and exit code, such
as irq_enter_rcu(), because it wrongly detects recursion.

trace_test_and_set_recursion() sets the recursion bit before it
disables preemption, and trace_clear_recursion() enables preemption
before it clears the bit. If the task is preempted in either window,
the scheduler's traced functions find the bit still set and take the
transition bit meant for interrupt entry and exit code. An interrupt
that arrives during one of these callbacks then finds both bits taken.

Disable preemption before setting the bit and enable it after clearing
the bit. The preempt count helpers then run outside the recursion
protection, so they must not be traced.

Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Patch 12 of my larger RFC series [1] also moved the RCU check and the
trace_recursion read. This version only moves the preempt calls.

Assisted by an LLM.

No Fixes tag. Most callers had the same ordering before ce5e48036c9e
("ftrace: disable preemption when recursion locked") moved the preempt
calls into these helpers.

The preempt count helpers are __always_inline, except on arm64, where
inline implies notrace and Song Liu's series adds an explicit
notrace [2].

Tested on v7.4-rc2 in QEMU TCG with PREEMPT, PROVE_LOCKING and
FTRACE_RECORD_RECURSION, running the function and then the
function_graph tracer while forking processes. On x86_64 and arm64, a
test module also sent IPIs to a CPU inside an ftrace callback.

Calls dropped as recursion, two boots each (function / function_graph):

                 v7.4-rc2       + this patch
  x86_64         0-12 / 14      0 / 0
  arm64          4 / 6          0 / 0

This also reduces use of the transition bit, which Steven would like to
see removed [3]. The remaining users are mostly interrupt entry and exit
code, which the early-hardirq series [4] is meant to cover.

Functions on other architectures that needed the transition bit,
whether they got it or were dropped, counted by a test-only hunk:

                 v7.4-rc2     + this patch
  i386           123 / 123    13 / 14
  ARMv7          118 / -      3 / -
  riscv64        115 / 116    17 / 18
  ppc64le        84 / 90      6 / 8
  ppc32          67 / 89      3 / 5
  mips32         76 / -       7 / -
  mips64         87 / 107     5 / 7
  sparc64        70 / 87      3 / 5
  loongarch64    114 / 119    11 / 15
  s390x          15 / 19      3 / 4

FTRACE_STARTUP_TEST passed with this patch on x86_64 and arm64, and
gave the same results as v7.4-rc2 on i386, ARMv7, riscv64, ppc64le and
loongarch64.

No result on PA-RISC (crashes) and sh4 (does not boot), or for
function_graph on ARMv7 and mips32. Baseline v7.4-rc2 behaves the same.
s390x traced only scheduler and interrupt entry functions, because full
tracing stalls under TCG. csky, microblaze and xtensa were only built.

[1] https://lore.kernel.org/all/20261022051654.79224-13-kmehltretter@gmail.com/
[2] https://lore.kernel.org/all/20261023011049.1164625-1-song@kernel.org/
[3] https://lore.kernel.org/all/20261015084821.3fcf69bf@gandalf.local.home/
[4] https://lore.kernel.org/all/20261028033127.82193-1-kmehltretter@gmail.com/

 include/linux/trace_recursion.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
index e6ca052b2a85a..a7c66313bf360 100644
--- a/include/linux/trace_recursion.h
+++ b/include/linux/trace_recursion.h
@@ -132,6 +132,8 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
 	if (trace_warn_on_no_rcu(ip))
 		return -1;
 
+	preempt_disable_notrace();
+
 	bit = trace_get_context_bit() + start;
 	if (unlikely(val & (1 << bit))) {
 		/*
@@ -145,6 +147,7 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
 		bit = TRACE_CTX_TRANSITION + start;
 		if (val & (1 << bit)) {
 			do_ftrace_record_recursion(ip, pip);
+			preempt_enable_notrace();
 			return -1;
 		}
 	}
@@ -153,8 +156,6 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
 	current->trace_recursion = val;
 	barrier();
 
-	preempt_disable_notrace();
-
 	return bit;
 }
 
@@ -163,9 +164,9 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
  */
 static __always_inline void trace_clear_recursion(int bit)
 {
-	preempt_enable_notrace();
 	barrier();
 	trace_recursion_clear(bit);
+	preempt_enable_notrace();
 }
 
 /**
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb

-- 
2.39.5 (Apple Git-154)