From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta0.migadu.com (out-144.mta0.migadu.com [91.218.175.144]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9D878311C2D for ; Tue, 29 Sep 2026 01:22:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.144 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790644933; cv=none; b=pvqRZ8PhProRWyMBCPcsvFKNaSdaRGclsTBMLpMQe+mvBXKqYgETFt4McdVL/8kURx57VkHO00cdZMOUY4BDY7maJ5WU+qavRInOTEesJbznhV0feH/I6Suh+e6lCgfQoJbNr45zpOeYQfyQNT5wUtCyeGYjy33Nm2P3Ycyc/a8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790644933; c=relaxed/simple; bh=y6zwP3b2fxxBONzUl72JLW4nTSFbQsYTTWD04VWcprk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=GJN2pOJH00hJ2rByItxr6uj01u+Fs3NaoymhCepylttyLLyTkYkJ2FDtdraf4QFoUhvzSyT9PMIvQ6LyCh0quADl2HuuqQ/Nb9BfEMMCZtTKVUySN2v40sfCoDkI+yzO/kJK0U7gWmHOye+1uRKbKPbU2usESuQhOof+m8Hh/5k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=csQ2F424; arc=none smtp.client-ip=91.218.175.144 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="csQ2F424" X-Envelope-To: linux-kernel@vger.kernel.org DKIM-Signature: a=rsa-sha256; bh=y6zwP3b2fxxBONzUl72JLW4nTSFbQsYTTWD04VWcprk=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1790644929; v=1; x=1791249729; b=csQ2F424N0nEzbZgEWW2TqgN3RVvIITuTUrMqNtOvn9uLfe3ccp96NpXcsXQszLquOPo/Hb8 a6UVHOL4++prHudRC/S2PjeUSPUTO5lg/9z4IhM8yR9yB2V5VT0ib2MwZOCI026eqDYbe+KuFwa EP1Gr5T5YAkJEzBpOEzVZPpQ= X-Envelope-To: linux-kernel@vger.kernel.org Received: by smtp.migadu.com with ESMTPS id 64465a751bbcb560; Tue, 29 Sep 2026 01:22:09 +0000 X-Mizu-Trace-ID: 64465a751bbcb560 X-Migadu-Flow: FLOW_OUT Message-ID: <0499fb85-d323-4141-b76e-bc9812b1d142@linux.dev> Date: Mon, 28 Sep 2026 18:22:02 -0700 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/1] tracing: fgraph: allocate shadow stacks inline with GFP_NOWAIT To: sashiko-reviews@lists.linux.dev, Peter Zijlstra , rostedt@goodmis.org, Andrii Nakryiko Cc: bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org, LKML , kernel-team@meta.com References: <20260929005411.4105448-1-vineet.gupta@linux.dev> <20260929005411.4105448-2-vineet.gupta@linux.dev> <20260929010327.024281F000FF@smtp.kernel.org> From: Vineet Gupta Content-Language: en-US In-Reply-To: <20260929010327.024281F000FF@smtp.kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 9/28/26 6:03 PM, sashiko-bot@kernel.org wrote: > 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 > 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? And a third option is to drop the fallback allocation completely and return -EAGAIN and come back. Granted I don't have much experience of typical handling of GFP_NOWAIT fails and retrying immediately: would that recover at all or does that take us back to where we started ? Peter Thx, -Vineet