| Message ID | 20241003193504.34640-4-snitzer@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | NFS LOCALIO: fixes and various cleanups | expand |
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@kernel.org> > --- > 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@oracle.com> > @@ -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 >
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; @@ -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);
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@kernel.org> --- fs/nfsd/trace.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)