Re: [PATCH 1/3] NFSD: rename and update nfsd_read_vector_dio trace event to nfsd_analyze_dio

[Mike Snitzer <snitzer@kernel.org>]

On Thu, Jul 31, 2025 at 11:31:29AM -0400, Jeff Layton wrote:
> On Wed, 2025-07-30 at 19:05 -0400, Mike Snitzer wrote:
> > From: Mike Snitzer <snitzer@hammerspace.com>
> > 
> > Rename nfsd_read_vector_dio trace event to nfsd_analyze_dio and update
> > it so that it provides useful tracing for both READ and WRITE.  This
> > prepares for nfsd_vfs_write() to also make use of it when handling
> > misaligned WRITEs.
> > 
> > Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
> > ---
> >  fs/nfsd/trace.h | 37 ++++++++++++++++++++++++-------------
> >  fs/nfsd/vfs.c   | 11 ++++++-----
> >  2 files changed, 30 insertions(+), 18 deletions(-)
> > 
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index 55055482f8a8..51b47fd041a8 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -473,41 +473,52 @@ DEFINE_NFSD_IO_EVENT(write_done);
> >  DEFINE_NFSD_IO_EVENT(commit_start);
> >  DEFINE_NFSD_IO_EVENT(commit_done);
> >  
> > -TRACE_EVENT(nfsd_read_vector_dio,
> > +TRACE_EVENT(nfsd_analyze_dio,
> >  	TP_PROTO(struct svc_rqst *rqstp,
> >  		 struct svc_fh	*fhp,
> > +		 u32		rw,
> 
> I would do this a bit differently. You're hardcoding READ and WRITE
> into both tracepoints. I would turn this trace event into a class a'la
> DECLARE_EVENT_CLASS, and then just define two different tracepoints
> (maybe trace_nfsd_analyze_read/write_dio). Then you can just drop the
> above u32 field, and it'll still be evident whether it's a read or
> write in the log.

Seems overkill to me, and also forces the need for user to enable
discrete tracepoints.  Could be good, could be bad.

But if I should be taking your feedback as: "do what Jeff suggested",
that's fine too, happy to do so! ;)

Thanks,
Mike