From: sashiko-bot@kernel.org
To: "Vineet Gupta" <vineet.gupta@linux.dev>
Cc: bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/1] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
Date: Tue, 29 Sep 2026 01:03:26 +0000 [thread overview]
Message-ID: <20260929010327.024281F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260929005411.4105448-2-vineet.gupta@linux.dev>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cleanup: mixing goto-based error handling and scope-based cleanup
--
commit a6de528fef00ef67da15965a7a8b3944fa92b053
Author: Vineet Gupta <vineet.gupta@linux.dev>
Subject: tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT
This commit aims to resolve performance regressions with ftrace graph tracing
when there are a large number of tasks. It replaces the pre-allocated batch
approach with inline allocation using GFP_NOWAIT, avoiding repeated sweeps of
the task list. Additionally, it transitions the RCU read-side section to use
scoped_guard(rcu) instead of manual rcu_read_lock and rcu_read_unlock calls.
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 40d373d65f9b9..1c5ccc3f80d53 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
[ ... ]
> @@ -1057,26 +1056,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) {
[Severity: Low]
Does this mix goto-based error handling and scope-based cleanup within the same
function?
Looking at alloc_retstack_tasklist(), the function retains the legacy goto free
path in the initial pre-allocation loop (just before this block), while
introducing scoped_guard(rcu) here.
The cleanup subsystem guidelines recommend that for a given routine, we should
convert all resources that need a goto cleanup to scope-based cleanup, or
convert none of them. Mixing these paradigms creates confusing ownership
semantics and is highly prone to double-free or resource leak regressions upon
future modifications.
Could we either convert the ret_stack_list array cleanup to use scope-based
helpers, or stick to the manual rcu_read_lock() and rcu_read_unlock() here to
keep the error handling style consistent?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260929005411.4105448-2-vineet.gupta@linux.dev?part=1
next prev parent reply other threads:[~2026-09-29 1:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-29 0:54 [PATCH v2] tracing: fgraph: fix the quadratic shadow stack walk Vineet Gupta
2026-09-29 0:54 ` [PATCH v2 1/1] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT Vineet Gupta
2026-09-29 1:03 ` sashiko-bot [this message]
2026-09-29 1:22 ` Vineet Gupta
2026-09-29 2:25 ` Andrii Nakryiko
2026-09-29 8:03 ` Peter Zijlstra
2026-09-29 23:46 ` bot+bpf-ci
2026-10-01 10:32 ` Vineet Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260929010327.024281F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=vineet.gupta@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox