On 9/24/26 3:00 AM, Peter Zijlstra wrote:
On Tue, Sep 22, 2026 at 03:55:25PM -0700, Vineet Gupta wrote:
When ftrace graphing is turned on, all tasks in the system missing
return stack page are assigned one. This is done in a simplistic
multi-sweep loop of FTRACE_RETSTACK_ALLOC_SIZE (currently 32) tasks
at a time as follows:
start_graph_tracing()
do {
alloc_retstack_tasklist
} while (-EAGAIN);
alloc_retstack_tasklist()
alloc x32 # GFP_KERNEL, may sleep
rcu_read_lock() # preempt off
for_each_process_thread walk N_total, no cond_resched
t->ret_stack = new_page
rcu_read_unlock() # preempt enable but no explicit yield
Each successive iteration of loop invokes for_each_process_thread()
which doesn't support cursor based resume and always restarts from the
init_task. Thus each successive loop needs to skip the tasks assigned
ret_stack in prior sweeps and thus take longer and longer to find the
candidate 32 tasks.
Does this work?
Yes it does and much better at that: test run for 400k threads went down
from 227 s (stock) to 7.2 s (my patch) to 92 ms. Woo hoo !
I didn't know about this cool trick.
A couple of things worth pointing out:
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index ed455b53513b..155dafad474d 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -1036,10 +1036,9 @@ trace_func_graph_ent_t ftrace_graph_entry =
ftrace_graph_entry_stub;
/* Try to assign a return stack array on FTRACE_RETSTACK_ALLOC_SIZE tasks. */
static int alloc_retstack_tasklist(unsigned long **ret_stack_list)
{
- int i;
- int ret = 0;
int start = 0, end = FTRACE_RETSTACK_ALLOC_SIZE;
struct task_struct *g, *t;
+ int i, ret = 0;
if (WARN_ON_ONCE(!fgraph_stack_cachep))
return -ENOMEM;
@@ -1054,26 +1053,29 @@ static int alloc_retstack_tasklist(unsigned long
**ret_stack_list)
}
}
- rcu_read_lock();
- for_each_process_thread(g, t) {
- if (start == end) {
- ret = -EAGAIN;
- goto unlock;
- }
+ scoped_guard (rcu) {
+ for_each_process_thread(g, t) {
+ unsigned long *rs;
+
+ if (t->ret_stack)
+ continue;
+
+ rs = kmem_cache_alloc(fgraph_stack_cachep, GFP_NOWAIT);
It still pre-allocates FTRACE_RETSTACK_ALLOC_SIZE reserves in the fast
path - my (limited) testing never hit the reserves, but I agree better
to keep it and 32 seems like a reasonably low number?
+ if (!rs) {
+ if (start == end)
+ return -EAGAIN;
This indicates the reserve pool is exhausted so it safe to elide the
free: loop at the end: warrants a comment IMO.
For submission, would you take this up as reported-by me or do you
prefer I send a v2 (your authorship and SoB etc) ? No big deal either
ways just not sure what the std operating procedure is and nuances of
all the tags etc.
Thx,
-Vineet
+ rs = ret_stack_list[start++];
+ }
- if (t->ret_stack == NULL) {
atomic_set(&t->trace_overrun, 0);
- ret_stack_init_task_vars(ret_stack_list[start]);
+ ret_stack_init_task_vars(rs);
t->curr_ret_stack = 0;
t->curr_ret_depth = -1;
/* Make sure the tasks see the 0 first: */
- smp_wmb();
- t->ret_stack = ret_stack_list[start++];
+ smp_store_release(&t->ret_stack, rs);
}
}
-unlock:
- rcu_read_unlock();
free:
for (i = start; i < end; i++)
kmem_cache_free(fgraph_stack_cachep, ret_stack_list[i]);