Re: [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk

[Masami Hiramatsu (Google)]

On Wed, 21 Oct 2026 10:44:06 -0700, Vineet Gupta <vineet.gupta@xxxxxxxxx> wrote:
> alloc_retstack_tasklist() open-codes rcu_read_lock() and unwinds it
> through an unlock label. Use scoped_guard(rcu) instead and let the
> compiler emit the unlock on every exit path.
> 
> The -EAGAIN case now jumps straight to the free label rather than to
> unlock; leaving the guard's scope releases the RCU read lock on the
> way, so the unlock label has no remaining users and is removed.
> 
> No functional change: objdump of fgraph.o shows identical call targets
> and counts, including the three __rcu_read_unlock() sites the compiler
> emits for the exits from the read-side section. A PROVE_RCU +
> DEBUG_ATOMIC_SLEEP build with the -EAGAIN path forced several thousand
> times is clean.
> 
> The preceding patch deliberately left this alone so that it stays
> straightforward to backport. This is the follow-up.
> 
> Suggested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Signed-off-by: Vineet Gupta <vineet.gupta@xxxxxxxxx>
> Tested-by: Vineet Gupta <vineet.gupta@xxxxxxxxx>

This looks good to me. 

Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Thanks.

> ---
>  kernel/trace/fgraph.c | 39 +++++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 4c74f1a15ad4..bbf6e4e2da2d 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -1057,32 +1057,31 @@ static int alloc_retstack_tasklist(unsigned long **ret_stack_list)
>  		}
>  	}
>  
> -	rcu_read_lock();
> -	for_each_process_thread(g, t) {
> -		unsigned long *rs;
> +	scoped_guard (rcu) {
> +		for_each_process_thread(g, t) {
> +			unsigned long *rs;
>  
> -		if (t->ret_stack)
> -			continue;
> +			if (t->ret_stack)
> +				continue;
>  
> -		rs = kmem_cache_alloc(fgraph_stack_cachep, GFP_NOWAIT);
> -		if (!rs) {
> -			if (start == end) {
> -				ret = -EAGAIN;
> -				goto unlock;
> +			rs = kmem_cache_alloc(fgraph_stack_cachep, GFP_NOWAIT);
> +			if (!rs) {
> +				if (start == end) {
> +					ret = -EAGAIN;
> +					goto free;
> +				}
> +				rs = ret_stack_list[start++];
>  			}
> -			rs = ret_stack_list[start++];
> -		}
>  
> -		atomic_set(&t->trace_overrun, 0);
> -		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_store_release(&t->ret_stack, rs);
> +			atomic_set(&t->trace_overrun, 0);
> +			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_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]);
> -- 
> 2.53.0-Meta
> 
>

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>