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 <[email protected]> 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/[email protected]?part=1
