On Wed, 2025-07-30 at 19:05 -0400, Mike Snitzer wrote:
> From: Mike Snitzer <snitzer@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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.
> u64 offset,
> u32 len,
> - loff_t start,
> - loff_t start_extra,
> - loff_t end,
> - loff_t end_extra),
> - TP_ARGS(rqstp, fhp, offset, len, start, start_extra, end, end_extra),
> + loff_t start,
> + ssize_t start_len,
> + loff_t middle,
> + ssize_t middle_len,
> + loff_t end,
> + ssize_t end_len),
> + TP_ARGS(rqstp, fhp, rw, offset, len, start, start_len, middle, middle_len, end, end_len),
> TP_STRUCT__entry(
> __field(u32, xid)
> __field(u32, fh_hash)
> + __field(u32, rw)
> __field(u64, offset)
> __field(u32, len)
> __field(loff_t, start)
> - __field(loff_t, start_extra)
> + __field(ssize_t, start_len)
> + __field(loff_t, middle)
> + __field(ssize_t, middle_len)
> __field(loff_t, end)
> - __field(loff_t, end_extra)
> + __field(ssize_t, end_len)
> ),
> TP_fast_assign(
> __entry->xid = be32_to_cpu(rqstp->rq_xid);
> __entry->fh_hash = knfsd_fh_hash(&fhp->fh_handle);
> + __entry->rw = rw;
> __entry->offset = offset;
> __entry->len = len;
> __entry->start = start;
> - __entry->start_extra = start_extra;
> + __entry->start_len = start_len;
> + __entry->middle = middle;
> + __entry->middle_len = middle_len;
> __entry->end = end;
> - __entry->end_extra = end_extra;
> + __entry->end_len = end_len;
> ),
> - TP_printk("xid=0x%08x fh_hash=0x%08x offset=%llu len=%u start=%llu+%llu end=%llu-%llu",
> + TP_printk("xid=0x%08x fh_hash=0x%08x %s offset=%llu len=%u start=%llu+%lu middle=%llu+%lu end=%llu+%lu",
> __entry->xid, __entry->fh_hash,
> + __entry->rw ? "WRITE" : "READ",
> __entry->offset, __entry->len,
> - __entry->start, __entry->start_extra,
> - __entry->end, __entry->end_extra)
> + __entry->start, __entry->start_len,
> + __entry->middle, __entry->middle_len,
> + __entry->end, __entry->end_len)
> );
>
> DECLARE_EVENT_CLASS(nfsd_err_class,
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 46189020172f..0863350c4259 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1094,7 +1094,7 @@ static bool nfsd_analyze_read_dio(struct svc_rqst *rqstp, struct svc_fh *fhp,
> struct nfsd_read_dio *read_dio)
> {
> const u32 dio_blocksize = nf->nf_dio_read_offset_align;
> - loff_t orig_end = offset + len;
> + loff_t middle_end, orig_end = offset + len;
>
> if (WARN_ONCE(!nf->nf_dio_mem_align || !nf->nf_dio_read_offset_align,
> "%s: underlying filesystem has not provided DIO alignment info\n",
> @@ -1133,10 +1133,11 @@ static bool nfsd_analyze_read_dio(struct svc_rqst *rqstp, struct svc_fh *fhp,
> }
>
> /* Show original offset and count, and how it was expanded for DIO */
> - trace_nfsd_read_vector_dio(rqstp, fhp, offset, len,
> - read_dio->start, read_dio->start_extra,
> - read_dio->end, read_dio->end_extra);
> -
> + middle_end = read_dio->end - read_dio->end_extra;
> + trace_nfsd_analyze_dio(rqstp, fhp, READ, offset, len,
> + read_dio->start, read_dio->start_extra,
> + offset, (middle_end - offset),
> + middle_end, read_dio->end_extra);
> return true;
> }
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
