All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk
@ 2026-10-21 17:44 Vineet Gupta
  2026-10-21 17:44 ` [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT Vineet Gupta
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Vineet Gupta @ 2026-10-21 17:44 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: mark.rutland, mathieu.desnoyers, andrii, peterz,
	linux-trace-kernel, linux-kernel, bpf, kernel-team, Vineet Gupta

alloc_retstack_tasklist() assigns a shadow stack to every thread when
fgraph is enabled. It did so FTRACE_RETSTACK_ALLOC_SIZE (32) tasks at a
time, and since for_each_process_thread() has no cursor, every sweep
restarted from init_task and re-walked the tasks already served, making
the ftrace_graph_active 0 -> 1 transition quadratic in the thread count.
On a 60-core Sapphire Rapids machine with 400000 idle threads it took
227 s, inside a single bpf() syscall for the kprobe_multi case.

 - 1/2 allocates inline with GFP_NOWAIT, which removes the batching:
   a single sweep serves every task, 227 s -> 0.092 s. It keeps the
   open-coded rcu_read_lock() for ease of backport.

 - 2/2 is the cleanup: convert that same walk to scoped_guard(rcu).
   No functional change, and not needed by stable.

Both were tested on bare metal and in a VM, including forcing the
GFP_NOWAIT failure path with failslab injection.

v1: https://lore.kernel.org/all/20260922225526.1554758-1-vineet.gupta@linux.dev/
v2: https://lore.kernel.org/all/20260929005411.4105448-1-vineet.gupta@linux.dev/
v3: https://lore.kernel.org/all/20261001102909.1152572-1-vineet.gupta@linux.dev/

Changes since v3:
 - split into two patches: the fix keeps goto-based unwinding so it
   backports cleanly, and scoped_guard(rcu) moves to a separate
   cleanup, per Masami and Steve:
   https://lore.kernel.org/all/20261013194542.7e036b03@226.227.38.69.naturalwireless.com/
 - in the cleanup, -EAGAIN now exits via the existing free label
   rather than returning from inside the guard, so there is no longer
   a path that skips the free loop

Changes since v2:
 - update the stale alloc_retstack_tasklist() comment, which still
   described the old batch-of-32 behaviour (bot+bpf-ci)

Changes since v1:
 - replace both v1 patches with Peter's inline GFP_NOWAIT approach,
   which removes the quadratic behaviour rather than dividing it by a
   constant. With a single sweep there is no retry loop left, so the
   v1 cond_resched() patch has nothing to attach to and is dropped.

Vineet Gupta (2):
  tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
  tracing: fgraph: use scoped_guard() for the task list walk

 kernel/trace/fgraph.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)


base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
  2026-10-21 17:44 [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk Vineet Gupta
@ 2026-10-21 17:44 ` Vineet Gupta
  2026-10-23 18:13   ` Andrii Nakryiko
  2026-10-21 17:44 ` [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk Vineet Gupta
  2026-10-21 22:01 ` [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk Alexei Starovoitov
  2 siblings, 1 reply; 11+ messages in thread
From: Vineet Gupta @ 2026-10-21 17:44 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: mark.rutland, mathieu.desnoyers, andrii, peterz,
	linux-trace-kernel, linux-kernel, bpf, kernel-team, Vineet Gupta,
	stable

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. And while the loop end calls rcu unlock,
and re-enables preemption briefly, there is no explicit yield.

The total number of iterations turn out to be

	N_total * N_null / (2 * FTRACE_RETSTACK_ALLOC_SIZE)

where N_total is the number of threads on the system and N_null the number
of those whose ret_stack is still NULL. After boot with no fgraph user that
is every thread.

This is not a tracing-only path. Since commit 4346ba160409 ("fprobe:
Rewrite fprobe on function-graph tracer") fprobe is built on fgraph,
so an ordinary kprobe_multi BPF attach that flips ftrace_graph_active
from 0 to 1 pays the whole cost inside one bpf() syscall. Commit
2c67dc457bc6 ("tracing: fprobe: optimization for entry only case")
narrows that to return and session probes but does not remove it.

A host running hundreds of thousands of threads, with the current batch
size of 32, turns the ftrace_graph_active 0 -> 1 transition into a
multi-second, and eventually multi-minute, operation which showed up as
RCU stall and softlockup_panic on heavily loaded Meta Fleet machines
with preemption disabled.

    rcu: INFO: rcu_sched self-detected stall on CPU
    rcu: 0-....: (20718 ticks this GP) (t=21000 jiffies g=913 q=8 ncpus=8)
    CPU: 0 UID: 0 PID: 160141 Comm: bpftrace
     ___slab_alloc+0x549/0xa20
     kmem_cache_alloc_noprof+0x16c/0x340
     register_ftrace_graph+0x3d7/0x6c0
     register_fprobe_ips+0x2d5/0x310
     bpf_kprobe_multi_link_attach+0x218/0x7e0
     __sys_bpf+0x267a/0x27a0

Fix this by allocating inline with GFP_NOWAIT instead of pre-allocating
a fixed batch. GFP_NOWAIT does not sleep, so it is legal inside the RCU
read-side critical section.

A single sweep now serves every task, so the walk becomes O(N) rather
than O(N_total * N_null), and the retry loop is no longer the normal
path.

The pre-allocated array remains as the fallback when GFP_NOWAIT fails.
The runs below never needed it, so it was exercised with failslab
injection: at a 100% failure rate every task falls back and -EAGAIN
retries ~1560 times for 50000 threads, with no stall or splat.

The open-coded rcu_read_lock() and the goto unwind are kept as-is so
this stays easy to backport; converting them to scope-based cleanup is
left to a follow-up.

smp_store_release() is a drop-in replacement for the smp_wmb() and
plain store it replaces.

Measured on a 60-core Sapphire Rapids machine running a PREEMPT_LAZY
kernel (CONFIG_PREEMPT_DYNAMIC=y, mode lazy), timing the write that
drives the ftrace_graph_active 0 -> 1 transition. Threads are spawned
while fgraph is inactive so every one of them has ret_stack == NULL,
and the loop was instrumented to count sweeps:

	threads   before              after
	          sweeps   time       sweeps   time
	100000     3126     15.3 s      1       0.019 s
	200000     6253     59.1 s      1       0.047 s
	400000    12503    227.8 s      1       0.092 s

The sweep count is 1 throughout. Time now scales linearly with the
thread count at roughly 230 ns per task, which is the allocation and
initialisation of one shadow stack.

Fixes: f201ae2356c7 ("tracing/function-return-tracer: store return stack into task_struct and allocate it dynamically")
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
Tested-by: Vineet Gupta <vineet.gupta@linux.dev>
Cc: stable@vger.kernel.org
---
 kernel/trace/fgraph.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index ed455b53513b..4c74f1a15ad4 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -1033,13 +1033,16 @@ void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
 trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
 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. */
+/*
+ * Loop thru all the tasks in the system and assign ret_stack.
+ * The @ret_stack_list is a small pre-allocated fallback, in case
+ * the GFP_NOWAIT allocations here fail.
+ */
 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;
@@ -1056,20 +1059,26 @@ 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;
-		}
+		unsigned long *rs;
 
-		if (t->ret_stack == NULL) {
-			atomic_set(&t->trace_overrun, 0);
-			ret_stack_init_task_vars(ret_stack_list[start]);
-			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++];
+		if (t->ret_stack)
+			continue;
+
+		rs = kmem_cache_alloc(fgraph_stack_cachep, GFP_NOWAIT);
+		if (!rs) {
+			if (start == end) {
+				ret = -EAGAIN;
+				goto unlock;
+			}
+			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);
 	}
 
 unlock:

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk
  2026-10-21 17:44 [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk Vineet Gupta
  2026-10-21 17:44 ` [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT Vineet Gupta
@ 2026-10-21 17:44 ` Vineet Gupta
  2026-10-21 17:53   ` sashiko-bot
  2026-10-21 22:01 ` [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk Alexei Starovoitov
  2 siblings, 1 reply; 11+ messages in thread
From: Vineet Gupta @ 2026-10-21 17:44 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: mark.rutland, mathieu.desnoyers, andrii, peterz,
	linux-trace-kernel, linux-kernel, bpf, kernel-team, Vineet Gupta

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@infradead.org>
Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
Tested-by: Vineet Gupta <vineet.gupta@linux.dev>
---
 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk
  2026-10-21 17:44 ` [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk Vineet Gupta
@ 2026-10-21 17:53   ` sashiko-bot
  2026-10-21 17:56     ` Vineet Gupta
  0 siblings, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-10-21 17:53 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: linux-trace-kernel, bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] tracing: fgraph: mix of scoped_guard() and goto in alloc_retstack_tasklist()
--

commit 7d59298fba17927bc12b63b0da1c66b0d138332a
Author: Vineet Gupta <vineet.gupta@linux.dev>

tracing: fgraph: use scoped_guard() for the task list walk

This commit replaces the explicit rcu_read_lock() and rcu_read_unlock() pair
in alloc_retstack_tasklist() with scoped_guard(rcu) and removes the unlock
label by jumping to the free label on the -EAGAIN path.

> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 4c74f1a15ad48..bbf6e4e2da2d4 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]);

[Severity: Low]
In kernel/trace/fgraph.c, alloc_retstack_tasklist() now uses
scoped_guard(rcu) while still jumping out of the guarded scope via goto free
when kmem_cache_alloc(fgraph_stack_cachep, GFP_NOWAIT) fails and start == end,
as well as jumping to free from the earlier pre-allocation loop:

kernel/trace/fgraph.c:alloc_retstack_tasklist() {
	for (i = 0; i < FTRACE_RETSTACK_ALLOC_SIZE; i++) {
		ret_stack_list[i] = kmem_cache_alloc(fgraph_stack_cachep, GFP_KERNEL);
		if (!ret_stack_list[i]) {
			start = 0;
			end = i;
			ret = -ENOMEM;
			goto free;
		}
	}
}

The documentation in include/linux/cleanup.h notes that goto-based cleanup
and scope-based cleanup helpers should not be mixed within the same function:

 * Lastly, given that the benefit of cleanup helpers is removal of
 * "goto", and that the "goto" statement can jump between scopes, the
 * expectation is that usage of "goto" and cleanup helpers is never
 * mixed in the same function. I.e. for a given routine, convert all
 * resources that need a "goto" cleanup to scope-based cleanup, or
 * convert none of them.

Also, on the -EAGAIN path inside scoped_guard(rcu), start == end is already
true because all pre-allocated entries in ret_stack_list[] have been consumed,
so the cleanup loop at the free label runs zero iterations on that path.

Should alloc_retstack_tasklist() avoid mixing scoped_guard(rcu) with goto
free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021174406.2891598-1-vineet.gupta@linux.dev?part=2

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk
  2026-10-21 17:53   ` sashiko-bot
@ 2026-10-21 17:56     ` Vineet Gupta
  0 siblings, 0 replies; 11+ messages in thread
From: Vineet Gupta @ 2026-10-21 17:56 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-trace-kernel, bpf

On 10/21/26 10:53 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] tracing: fgraph: mix of scoped_guard() and goto in alloc_retstack_tasklist()
> --
>
> commit 7d59298fba17927bc12b63b0da1c66b0d138332a
> Author: Vineet Gupta <vineet.gupta@linux.dev>
>
> tracing: fgraph: use scoped_guard() for the task list walk
>
> This commit replaces the explicit rcu_read_lock() and rcu_read_unlock() pair
> in alloc_retstack_tasklist() with scoped_guard(rcu) and removes the unlock
> label by jumping to the free label on the -EAGAIN path.
>
>> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
>> index 4c74f1a15ad48..bbf6e4e2da2d4 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]);
> [Severity: Low]
> In kernel/trace/fgraph.c, alloc_retstack_tasklist() now uses
> scoped_guard(rcu) while still jumping out of the guarded scope via goto free
> when kmem_cache_alloc(fgraph_stack_cachep, GFP_NOWAIT) fails and start == end,
> as well as jumping to free from the earlier pre-allocation loop:
>
> kernel/trace/fgraph.c:alloc_retstack_tasklist() {
> 	for (i = 0; i < FTRACE_RETSTACK_ALLOC_SIZE; i++) {
> 		ret_stack_list[i] = kmem_cache_alloc(fgraph_stack_cachep, GFP_KERNEL);
> 		if (!ret_stack_list[i]) {
> 			start = 0;
> 			end = i;
> 			ret = -ENOMEM;
> 			goto free;
> 		}
> 	}
> }
>
> The documentation in include/linux/cleanup.h notes that goto-based cleanup
> and scope-based cleanup helpers should not be mixed within the same function:
>
>   * Lastly, given that the benefit of cleanup helpers is removal of
>   * "goto", and that the "goto" statement can jump between scopes, the
>   * expectation is that usage of "goto" and cleanup helpers is never
>   * mixed in the same function. I.e. for a given routine, convert all
>   * resources that need a "goto" cleanup to scope-based cleanup, or
>   * convert none of them.
>
> Also, on the -EAGAIN path inside scoped_guard(rcu), start == end is already
> true because all pre-allocated entries in ret_stack_list[] have been consumed,
> so the cleanup loop at the free label runs zero iterations on that path.
>
> Should alloc_retstack_tasklist() avoid mixing scoped_guard(rcu) with goto
> free?

This has been discussed before and it was agreed this is fine.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk
  2026-10-21 17:44 [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk Vineet Gupta
  2026-10-21 17:44 ` [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT Vineet Gupta
  2026-10-21 17:44 ` [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk Vineet Gupta
@ 2026-10-21 22:01 ` Alexei Starovoitov
  2 siblings, 0 replies; 11+ messages in thread
From: Alexei Starovoitov @ 2026-10-21 22:01 UTC (permalink / raw)
  To: Vineet Gupta, rostedt, mhiramat
  Cc: mark.rutland, mathieu.desnoyers, andrii, peterz,
	linux-trace-kernel, linux-kernel, bpf, kernel-team

On Wed, Oct 21, 2026 at 10:44 AM Vineet Gupta <vineet.gupta@linux.dev> wrote:
> On a 60-core Sapphire Rapids machine with 400000 idle threads it took
> 227 s, inside a single bpf() syscall for the kprobe_multi case.

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
  2026-10-21 17:44 ` [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT Vineet Gupta
@ 2026-10-23 18:13   ` Andrii Nakryiko
  2026-10-23 18:38     ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2026-10-23 18:13 UTC (permalink / raw)
  To: Vineet Gupta, mhiramat, rostedt
  Cc: mark.rutland, mathieu.desnoyers, andrii, peterz,
	linux-trace-kernel, linux-kernel, bpf, kernel-team, stable

On Wed, Oct 21, 2026 at 10:44 AM Vineet Gupta <vineet.gupta@linux.dev> 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. And while the loop end calls rcu unlock,
> and re-enables preemption briefly, there is no explicit yield.
>
> The total number of iterations turn out to be
>
>         N_total * N_null / (2 * FTRACE_RETSTACK_ALLOC_SIZE)
>
> where N_total is the number of threads on the system and N_null the number
> of those whose ret_stack is still NULL. After boot with no fgraph user that
> is every thread.
>
> This is not a tracing-only path. Since commit 4346ba160409 ("fprobe:
> Rewrite fprobe on function-graph tracer") fprobe is built on fgraph,
> so an ordinary kprobe_multi BPF attach that flips ftrace_graph_active
> from 0 to 1 pays the whole cost inside one bpf() syscall. Commit
> 2c67dc457bc6 ("tracing: fprobe: optimization for entry only case")
> narrows that to return and session probes but does not remove it.
>
> A host running hundreds of thousands of threads, with the current batch
> size of 32, turns the ftrace_graph_active 0 -> 1 transition into a
> multi-second, and eventually multi-minute, operation which showed up as
> RCU stall and softlockup_panic on heavily loaded Meta Fleet machines
> with preemption disabled.
>
>     rcu: INFO: rcu_sched self-detected stall on CPU
>     rcu: 0-....: (20718 ticks this GP) (t=21000 jiffies g=913 q=8 ncpus=8)
>     CPU: 0 UID: 0 PID: 160141 Comm: bpftrace
>      ___slab_alloc+0x549/0xa20
>      kmem_cache_alloc_noprof+0x16c/0x340
>      register_ftrace_graph+0x3d7/0x6c0
>      register_fprobe_ips+0x2d5/0x310
>      bpf_kprobe_multi_link_attach+0x218/0x7e0
>      __sys_bpf+0x267a/0x27a0
>
> Fix this by allocating inline with GFP_NOWAIT instead of pre-allocating
> a fixed batch. GFP_NOWAIT does not sleep, so it is legal inside the RCU
> read-side critical section.
>
> A single sweep now serves every task, so the walk becomes O(N) rather
> than O(N_total * N_null), and the retry loop is no longer the normal
> path.
>
> The pre-allocated array remains as the fallback when GFP_NOWAIT fails.
> The runs below never needed it, so it was exercised with failslab
> injection: at a 100% failure rate every task falls back and -EAGAIN
> retries ~1560 times for 50000 threads, with no stall or splat.
>
> The open-coded rcu_read_lock() and the goto unwind are kept as-is so
> this stays easy to backport; converting them to scope-based cleanup is
> left to a follow-up.
>
> smp_store_release() is a drop-in replacement for the smp_wmb() and
> plain store it replaces.
>
> Measured on a 60-core Sapphire Rapids machine running a PREEMPT_LAZY
> kernel (CONFIG_PREEMPT_DYNAMIC=y, mode lazy), timing the write that
> drives the ftrace_graph_active 0 -> 1 transition. Threads are spawned
> while fgraph is inactive so every one of them has ret_stack == NULL,
> and the loop was instrumented to count sweeps:
>
>         threads   before              after
>                   sweeps   time       sweeps   time
>         100000     3126     15.3 s      1       0.019 s
>         200000     6253     59.1 s      1       0.047 s
>         400000    12503    227.8 s      1       0.092 s
>
> The sweep count is 1 throughout. Time now scales linearly with the
> thread count at roughly 230 ns per task, which is the allocation and
> initialisation of one shadow stack.
>
> Fixes: f201ae2356c7 ("tracing/function-return-tracer: store return stack into task_struct and allocate it dynamically")
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
> Tested-by: Vineet Gupta <vineet.gupta@linux.dev>
> Cc: stable@vger.kernel.org
> ---
>  kernel/trace/fgraph.c | 39 ++++++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
>

Acked-by: Andrii Nakryiko <andrii@kernel.org>

I presume this will go through Masami and Steven's tree, is that right?

> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index ed455b53513b..4c74f1a15ad4 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -1033,13 +1033,16 @@ void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
>  trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
>  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. */
> +/*
> + * Loop thru all the tasks in the system and assign ret_stack.
> + * The @ret_stack_list is a small pre-allocated fallback, in case
> + * the GFP_NOWAIT allocations here fail.
> + */
>  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;
> @@ -1056,20 +1059,26 @@ 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;
> -               }
> +               unsigned long *rs;
>
> -               if (t->ret_stack == NULL) {
> -                       atomic_set(&t->trace_overrun, 0);
> -                       ret_stack_init_task_vars(ret_stack_list[start]);
> -                       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++];
> +               if (t->ret_stack)
> +                       continue;
> +
> +               rs = kmem_cache_alloc(fgraph_stack_cachep, GFP_NOWAIT);
> +               if (!rs) {
> +                       if (start == end) {
> +                               ret = -EAGAIN;
> +                               goto unlock;
> +                       }
> +                       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);
>         }
>
>  unlock:
>
> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
> --
> 2.53.0-Meta
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
  2026-10-23 18:13   ` Andrii Nakryiko
@ 2026-10-23 18:38     ` Steven Rostedt
  2026-10-23 20:13       ` Vineet Gupta
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2026-10-23 18:38 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Vineet Gupta, mhiramat, mark.rutland, mathieu.desnoyers, andrii,
	peterz, linux-trace-kernel, linux-kernel, bpf, kernel-team,
	stable

On Fri, 23 Oct 2026 11:13:44 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> I presume this will go through Masami and Steven's tree, is that right?

Yes, but it came in during the merge window, and since it's just a speed up
and not fixing something that's broken, I was going to let this go in the
next merge window. I rather have a change like this sit through a full -rc
release.

-- Steve

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
  2026-10-23 18:38     ` Steven Rostedt
@ 2026-10-23 20:13       ` Vineet Gupta
  2026-10-23 22:09         ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Vineet Gupta @ 2026-10-23 20:13 UTC (permalink / raw)
  To: Steven Rostedt, Andrii Nakryiko
  Cc: mhiramat, mark.rutland, mathieu.desnoyers, andrii, peterz,
	linux-trace-kernel, linux-kernel, bpf, kernel-team, stable

On 10/23/26 11:38 AM, Steven Rostedt wrote:
> On Fri, 23 Oct 2026 11:13:44 -0700
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
>> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>>
>> I presume this will go through Masami and Steven's tree, is that right?
> Yes, but it came in during the merge window, and since it's just a speed up
> and not fixing something that's broken, I was going to let this go in the
> next merge window. I rather have a change like this sit through a full -rc
> release.

It is a speed up but not for optimization sake but to cure rcu stalls in 
production at Meta. IMHO it should go in sooner than wait a full release 
cycle.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
  2026-10-23 20:13       ` Vineet Gupta
@ 2026-10-23 22:09         ` Steven Rostedt
  2026-10-23 22:25           ` Vineet Gupta
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2026-10-23 22:09 UTC (permalink / raw)
  To: Vineet Gupta
  Cc: Andrii Nakryiko, mhiramat, mark.rutland, mathieu.desnoyers,
	andrii, peterz, linux-trace-kernel, linux-kernel, bpf,
	kernel-team, stable

On Fri, 23 Oct 2026 13:13:41 -0700
Vineet Gupta <vineet.gupta@linux.dev> wrote:

> On 10/23/26 11:38 AM, Steven Rostedt wrote:
> > On Fri, 23 Oct 2026 11:13:44 -0700
> > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >  
> >> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> >>
> >> I presume this will go through Masami and Steven's tree, is that right?  
> > Yes, but it came in during the merge window, and since it's just a speed up
> > and not fixing something that's broken, I was going to let this go in the
> > next merge window. I rather have a change like this sit through a full -rc
> > release.  
> 
> It is a speed up but not for optimization sake but to cure rcu stalls in 
> production at Meta. IMHO it should go in sooner than wait a full release 
> cycle.
> 

Either way, I'd like to give it a bit more testing first. That is, the
earliest it will go is after -rc1.

-- Steve

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
  2026-10-23 22:09         ` Steven Rostedt
@ 2026-10-23 22:25           ` Vineet Gupta
  0 siblings, 0 replies; 11+ messages in thread
From: Vineet Gupta @ 2026-10-23 22:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrii Nakryiko, mhiramat, mark.rutland, mathieu.desnoyers,
	andrii, peterz, linux-trace-kernel, linux-kernel, bpf,
	kernel-team, stable

On 10/23/26 3:09 PM, Steven Rostedt wrote:
> On Fri, 23 Oct 2026 13:13:41 -0700
> Vineet Gupta <vineet.gupta@linux.dev> wrote:
>
>> On 10/23/26 11:38 AM, Steven Rostedt wrote:
>>> On Fri, 23 Oct 2026 11:13:44 -0700
>>> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>   
>>>> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>>>>
>>>> I presume this will go through Masami and Steven's tree, is that right?
>>> Yes, but it came in during the merge window, and since it's just a speed up
>>> and not fixing something that's broken, I was going to let this go in the
>>> next merge window. I rather have a change like this sit through a full -rc
>>> release.
>> It is a speed up but not for optimization sake but to cure rcu stalls in
>> production at Meta. IMHO it should go in sooner than wait a full release
>> cycle.
>>
> Either way, I'd like to give it a bit more testing first. That is, the
> earliest it will go is after -rc1.

That's fair.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-10-23 22:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 17:44 [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk Vineet Gupta
2026-10-21 17:44 ` [PATCH v4 1/2] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT Vineet Gupta
2026-10-23 18:13   ` Andrii Nakryiko
2026-10-23 18:38     ` Steven Rostedt
2026-10-23 20:13       ` Vineet Gupta
2026-10-23 22:09         ` Steven Rostedt
2026-10-23 22:25           ` Vineet Gupta
2026-10-21 17:44 ` [PATCH v4 2/2] tracing: fgraph: use scoped_guard() for the task list walk Vineet Gupta
2026-10-21 17:53   ` sashiko-bot
2026-10-21 17:56     ` Vineet Gupta
2026-10-21 22:01 ` [PATCH v4 0/2] tracing: fgraph: fix the quadratic shadow stack walk Alexei Starovoitov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.