Re: [PATCH 1/2] tracing: fgraph: Raise FTRACE_RETSTACK_ALLOC_SIZE to 1024

[Steven Rostedt]

On Thu, 24 Sep 2026 12:00:40 +0200
Peter Zijlstra <pet...@infradead.org> wrote:

> 
> Does this work?
> 
> 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);
> +                     if (!rs) {
> +                             if (start == end)
> +                                     return -EAGAIN;
> +                             rs = ret_stack_list[start++];
> +                     }

Yeah this is fine. This iteration code goes back to when function graph
was first added back in 2008. Back then, looping all tasks required
taking the task_list lock and I remember we didn't want to call
kmalloc() while holding that. This code could definitely use some
reassessment.

-- Steve


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