Re: [PATCH v2 1/1] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT

[Vineet Gupta]

On 9/29/26 4:46 PM, bot+bpf-ci@xxxxxxxxxx wrote:
> > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> > index ed455b53513b..336f40dc6618 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)
> >   {
> Does this comment still match the function's behavior? Before the patch,
> alloc_retstack_tasklist() assigned at most FTRACE_RETSTACK_ALLOC_SIZE
> stacks per call and returned -EAGAIN to trigger another batch. After the
> patch, it walks every thread in one pass, allocating inline with
> GFP_NOWAIT. The FTRACE_RETSTACK_ALLOC_SIZE pre-allocated entries are now
> only a fallback when GFP_NOWAIT fails, and in the normal case they're all
> freed at the free: label.

Posted v3 [1] with comment fix.

Thx,
-Vineet

[1] 
https://lore.kernel.org/bpf/20261001102909.1152572-1-vineet.gupta@xxxxxxxxx/T/#t

> > -	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,37 @@ 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) {
> > +				/*
> > +				 * Returning from inside scoped_guard() drops
> > +				 * the RCU read lock, but skips the free loop
> > +				 * below. That is only safe because start ==
> > +				 * end here, which leaves that loop nothing to
> > +				 * free. Keep the two in step if this
> > +				 * exhaustion check ever changes.
> > +				 */
> > +				if (start == end)
> > +					return -EAGAIN;
> > +				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]);
>
> ---
> 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/36644771324