On Thu, Aug 29, 2024 at 10:33:18AM -0400, Jeff Layton wrote:
> On Wed, 2024-08-28 at 21:04 -0400, Mike Snitzer wrote:
> > From: Chuck Lever <chuck.lever@xxxxxxxxxx>
> >
> > LOCALIO will be able to call fh_verify() with a NULL rqstp. In this
> > case, the existing trace points need to be skipped because they
> > want to dereference the address fields in the passed-in rqstp.
> >
> > Temporarily make these trace points conditional to avoid a seg
> > fault in this case. Putting the "rqstp != NULL" check in the trace
> > points themselves makes the check more efficient.
> >
> > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> > ---
> > fs/nfsd/trace.h | 18 ++++++++++--------
> > 1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index 77bbd23aa150..d22027e23761 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -193,7 +193,7 @@ TRACE_EVENT(nfsd_compound_encode_err,
> > { S_IFIFO, "FIFO" }, \
> > { S_IFSOCK, "SOCK" })
> >
> > -TRACE_EVENT(nfsd_fh_verify,
> > +TRACE_EVENT_CONDITION(nfsd_fh_verify,
> > TP_PROTO(
> > const struct svc_rqst *rqstp,
> > const struct svc_fh *fhp,
> > @@ -201,6 +201,7 @@ TRACE_EVENT(nfsd_fh_verify,
> > int access
> > ),
> > TP_ARGS(rqstp, fhp, type, access),
> > + TP_CONDITION(rqstp != NULL),
> > TP_STRUCT__entry(
> > __field(unsigned int, netns_ino)
> > __sockaddr(server, rqstp->rq_xprt->xpt_remotelen)
> > @@ -239,7 +240,7 @@ TRACE_EVENT_CONDITION(nfsd_fh_verify_err,
> > __be32 error
> > ),
> > TP_ARGS(rqstp, fhp, type, access, error),
> > - TP_CONDITION(error),
> > + TP_CONDITION(rqstp != NULL && error),
> > TP_STRUCT__entry(
> > __field(unsigned int, netns_ino)
> > __sockaddr(server, rqstp->rq_xprt->xpt_remotelen)
> > @@ -295,12 +296,13 @@ DECLARE_EVENT_CLASS(nfsd_fh_err_class,
> > __entry->status)
> > )
> >
> > -#define DEFINE_NFSD_FH_ERR_EVENT(name) \
> > -DEFINE_EVENT(nfsd_fh_err_class, nfsd_##name, \
> > - TP_PROTO(struct svc_rqst *rqstp, \
> > - struct svc_fh *fhp, \
> > - int status), \
> > - TP_ARGS(rqstp, fhp, status))
> > +#define DEFINE_NFSD_FH_ERR_EVENT(name) \
> > +DEFINE_EVENT_CONDITION(nfsd_fh_err_class, nfsd_##name, \
> > + TP_PROTO(struct svc_rqst *rqstp, \
> > + struct svc_fh *fhp, \
> > + int status), \
> > + TP_ARGS(rqstp, fhp, status), \
> > + TP_CONDITION(rqstp != NULL))
> >
> > DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badexport);
> > DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badhandle);
>
> A bit ugly. We really only want the rqstp here to get at the socket
> structures. I'm still looking at the rest of the set, so I'll assume
> that this gets cleaned up later.
No, it doesn't. We don't have a solution for how to trace
LOCALIO activity here yet.
--
Chuck Lever
