Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 3 Jul 2022 08:48:08 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Dominique Martinet <asmadeus@codewreck.org>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Christian Schoenebeck <linux_oss@crudebyte.com>,
        v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] 9p fid refcount: add a 9p_fid_ref tracepoint
Message-ID: <20220703084808.1d6a9989@rorschach.local.home>
In-Reply-To: <20220702102913.2164800-1-asmadeus@codewreck.org>
References: <20220702102913.2164800-1-asmadeus@codewreck.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sat,  2 Jul 2022 19:29:14 +0900
Dominique Martinet <asmadeus@codewreck.org> wrote:

> This adds a tracepoint event for 9p fid lifecycle tracing: when a fid
> is created, its reference count increased/decreased, and freed.
> The new 9p_fid_ref tracepoint should help anyone wishing to debug any
> fid problem such as missing clunk (destroy) or use-after-free.
> 
> Link: https://lkml.kernel.org/r/20220612085330.1451496-6-asmadeus@codewreck.org
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
> 
> Just resending this single patch of the series as it's the only one
> without a review tag.
> 
> Steven, is it ok to carry it in my tree as is or do I need blessings
> from you or Ingo?

The addition of trace events do belong to the maintainers of where the
trace events go.

> (it depends on the previous patch so I'd carry it in my tree anyway,
> but would be more comfortable with a reviewed-by tag)

Yes, I prefer people Cc me on trace events just so that I can catch
mistakes or find better ways to accomplish what is trying to be done.

Especially for something that does changes like this patch, which are
not just the trivial TRACE_EVENT() trace_*() procedure. Thanks for
Cc'ing me.

> 
> 
> v2 -> v3:
>  - added EXPORT_TRACEPOINT_SYMBOL(9p_fid_ref) to have this work when
>    built as module
> 
> v1 -> v2:
>  - added rationale to commit message
>  - adjusted to use DECLARE_TRACEPOINT + tracepoint_enable() in header
> 
>  include/net/9p/client.h   | 13 +++++++++++
>  include/trace/events/9p.h | 48 +++++++++++++++++++++++++++++++++++++++
>  net/9p/client.c           | 20 +++++++++++++++-
>  3 files changed, 80 insertions(+), 1 deletion(-)
> 

The rest looks fine.

For the tracing point of view:

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve