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]);


Reply via email to