On Thu, Oct 03, 2024 at 03:35:00PM -0400, Mike Snitzer wrote:
> Otherwise nfsd_file_acquire, nfsd_file_insert_err, and
> nfsd_file_cons_err will hit a NULL pointer when they are enabled and
> LOCALIO used.
>
> Example trace output (note xid is 0x0 and LOCALIO flag set):
> nfsd_file_acquire: xid=0x0 inode=0000000069a1b2e7
> may_flags=WRITE|LOCALIO ref=1 nf_flags=HASHED|GC nf_may=WRITE
> nf_file=0000000070123234 status=0
>
> Fixes: c63f0e48febf ("nfsd: add nfsd_file_acquire_local()")
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> ---
> fs/nfsd/trace.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index c625966cfcf3..b8470d4cbe99 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -1113,7 +1113,7 @@ TRACE_EVENT(nfsd_file_acquire,
> ),
>
> TP_fast_assign(
> - __entry->xid = be32_to_cpu(rqstp->rq_xid);
> + __entry->xid = rqstp ? be32_to_cpu(rqstp->rq_xid) : 0;
> __entry->inode = inode;
> __entry->may_flags = may_flags;
> __entry->nf_ref = nf ? refcount_read(&nf->nf_ref) : 0;
A future auditor of this code might wonder why trace_nfsd_fh_verify()
takes this approach instead:
197 TRACE_EVENT_CONDITION(nfsd_fh_verify,
198 TP_PROTO(
199 const struct svc_rqst *rqstp,
200 const struct svc_fh *fhp,
201 umode_t type,
202 int access
203 ),
204 TP_ARGS(rqstp, fhp, type, access),
205 TP_CONDITION(rqstp != NULL),
The answer is that trace_nfsd_fh_verify() uses @rqstp in its
TP_STRUCT__entry () clause. Thus the entire nfsd_fh_verify() trace
point has to be short-circuited if @rqstp is NULL.
Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> @@ -1147,7 +1147,7 @@ TRACE_EVENT(nfsd_file_insert_err,
> __field(long, error)
> ),
> TP_fast_assign(
> - __entry->xid = be32_to_cpu(rqstp->rq_xid);
> + __entry->xid = rqstp ? be32_to_cpu(rqstp->rq_xid) : 0;
> __entry->inode = inode;
> __entry->may_flags = may_flags;
> __entry->error = error;
> @@ -1177,7 +1177,7 @@ TRACE_EVENT(nfsd_file_cons_err,
> __field(const void *, nf_file)
> ),
> TP_fast_assign(
> - __entry->xid = be32_to_cpu(rqstp->rq_xid);
> + __entry->xid = rqstp ? be32_to_cpu(rqstp->rq_xid) : 0;
> __entry->inode = inode;
> __entry->may_flags = may_flags;
> __entry->nf_ref = refcount_read(&nf->nf_ref);
> --
> 2.44.0
>
--
Chuck Lever
