Re: [PATCH RFC v3 05/13] ftrace: Mark modules hosting direct-call trampolines for Tasks RCU
flat view
From: bot+bpf-ci@kernel.org
Date: 2026-09-15 14:17:52
Also in:
bpf, lkml, rcu, xen-devel
quoted hunk ↗ jump to hunk
diff --git a/include/linux/module.h b/include/linux/module.h index 96cc98568eea5..28488687cb01f 100644 --- a/include/linux/module.h +++ b/include/linux/module.h@@ -521,6 +521,13 @@ struct module { unsigned int num_ftrace_callsites; unsigned long *ftrace_callsites; #endif +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS + /* + * An ftrace direct-call trampoline lives in this module's text; see + * rcu_tasks_trampoline_text(). Sticky once set. + */ + bool ftrace_direct_tramp; +#endif
This isn't a bug, but could the field and ftrace_direct_mark_module()'s
body be gated on CONFIG_TASKS_RCU_TRAMPOLINE_READERS instead? The only
reader of module::ftrace_direct_tramp is rcu_tasks_trampoline_text(),
which lives inside kernel/rcu/tasks.h's CONFIG_TASKS_RCU_TRAMPOLINE_READERS
block:
kernel/rcu/tasks.h:rcu_tasks_trampoline_text() {
#ifdef CONFIG_MODULES
scoped_guard(rcu) {
struct module *mod = __module_text_address(ip);
if (mod) {
ret = kprobe_in_optimized_region(ip);
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
ret = ret || READ_ONCE(mod->ftrace_direct_tramp);
#endif
}
}
#endif
}
On every configuration that does not select HAVE_RCU_TRAMPOLINE_READERS
the per-module bool and the four ftrace_direct_mark_module() call sites
that write it have no consumer.
quoted hunk ↗ jump to hunk
diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h index 8667684628500..ec54a27e47fae 100644 --- a/kernel/rcu/tasks.h +++ b/kernel/rcu/tasks.h
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 53d5db60bfa56..efc4a518658a3 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -6090,6 +6113,17 @@ static void reset_direct(struct ftrace_ops *ops, unsigned long addr) * and save the parameters of the function being traced, and restore them * (or inject new ones if needed), before returning. * + * Nothing but Tasks RCU keeps the trampoline at @addr alive while a task is + * executing it or is preempted in something it called. On architectures that + * select HAVE_RCU_TRAMPOLINE_READERS, Tasks RCU only waits for such a task if + * it is a Tasks Trace RCU reader, so the trampoline must enter one + * (rcu_read_lock_trace() or its assembly equivalent, see + * samples/ftrace/ftrace-direct.h) before calling out and leave it before + * returning, as the ftrace and BPF trampolines do. The few instructions
This isn't a bug, but would it read better to point at the sample header only once it exists in the tree, or to word the ftrace/BPF trampoline part as what the series establishes rather than as current state? The new kernel-doc paragraph points readers at samples/ftrace/ftrace-direct.h, which does not exist at this commit (it is added later in the series by 'samples: ftrace: Make the direct-call trampolines Tasks Trace readers'), and states in the present tense that the ftrace and BPF trampolines already take the reader, which also only becomes true in later commits of the series (bpf/x86/arm64 reader patches). Anyone reading or bisecting at this commit gets a dangling reference and a claim the tree does not yet satisfy.
quoted hunk ↗ jump to hunk
+ * before and after are covered by the irq-exit check: automatically for + * trampolines outside kernel and module text (e.g. BPF images), and via + * ftrace_direct_mark_module() for trampolines in module text. + * * Returns: * 0 on success * -EINVAL - The @ops object was already registered with this call or
[ ... ] commit ef57b52d2606546f6d483b113fc2202c13863c2c Author: Josef Bacik [off-list ref]
ftrace: Mark modules hosting direct-call trampolines for Tasks RCU
An out-of-line direct trampoline registered with register_ftrace_direct()
is kept alive only by Tasks RCU while a task executes it or is preempted
in something it called; ftrace_shutdown()'s synchronize_rcu_tasks() is
what stops rmmod freeing it under such a task. Where Tasks RCU is built
on reader-marked trampolines, such a trampoline must be a Tasks Trace
reader across its call-out like the ftrace and BPF trampolines are, so
document that in register_ftrace_direct().
That still leaves the few instructions before the reader is entered and
after it is left. For BPF images those are in dynamically allocated
text that rcu_tasks_trampoline_text() already treats as unmarked
trampoline text, but the in-tree samples (and any similar user) place
their trampolines in module .text. Add a sticky
module::ftrace_direct_tramp flag, set by every register/modify path when
the direct address is module text, and have rcu_tasks_trampoline_text()
treat a task interrupted anywhere in such a module as a potential
holdout. Other modules' text is unaffected.
Assisted-by: LLM
Signed-off-by: Josef Bacik [off-list ref]
This looks like a bug fix for reader-marked trampoline support. Should
this include:
Fixes: 00bd9b746f5e ("rcu-tasks: Add a Tasks RCU implementation for reader-marked trampolines")
The bug was introduced in commit 00bd9b746f5e which added
rcu_tasks_trampoline_text() but failed to properly handle module .text
containing direct trampolines. The function returned false for module
text addresses, meaning such trampolines were not protected by Tasks RCU,
potentially allowing module text to be freed while still executing.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34975311706