On Mon, 2020-03-02 at 19:22 -0500, Chuck Lever wrote:
> > On Mar 1, 2020, at 6:21 PM, Trond Myklebust <trondmy@xxxxxxxxx>
> > wrote:
> >
> > Add tracing to allow us to figure out where any stale filehandle
> > issues
> > may be originating from.
> >
> > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > ---
> > fs/nfsd/nfsfh.c | 13 ++++++++++---
> > fs/nfsd/trace.h | 30 ++++++++++++++++++++++++++++++
> > 2 files changed, 40 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > index b319080288c3..37bc8f5f4514 100644
> > --- a/fs/nfsd/nfsfh.c
> > +++ b/fs/nfsd/nfsfh.c
> > @@ -14,6 +14,7 @@
> > #include "nfsd.h"
> > #include "vfs.h"
> > #include "auth.h"
> > +#include "trace.h"
> >
> > #define NFSDDBG_FACILITY NFSDDBG_FH
> >
> > @@ -209,11 +210,14 @@ static __be32 nfsd_set_fh_dentry(struct
> > svc_rqst *rqstp, struct svc_fh *fhp)
> > }
> >
> > error = nfserr_stale;
> > - if (PTR_ERR(exp) == -ENOENT)
> > - return error;
> > + if (IS_ERR(exp)) {
> > + trace_nfsd_set_fh_dentry_badexport(rqstp, fhp,
> > PTR_ERR(exp));
> > +
> > + if (PTR_ERR(exp) == -ENOENT)
> > + return error;
> >
> > - if (IS_ERR(exp))
> > return nfserrno(PTR_ERR(exp));
> > + }
> >
> > if (exp->ex_flags & NFSEXP_NOSUBTREECHECK) {
> > /* Elevate privileges so that the lack of 'r' or 'x'
> > @@ -267,6 +271,9 @@ static __be32 nfsd_set_fh_dentry(struct
> > svc_rqst *rqstp, struct svc_fh *fhp)
> > dentry = exportfs_decode_fh(exp->ex_path.mnt, fid,
> > data_left, fileid_type,
> > nfsd_acceptable, exp);
> > + if (IS_ERR_OR_NULL(dentry))
> > + trace_nfsd_set_fh_dentry_badhandle(rqstp, fhp,
> > + dentry ? PTR_ERR(dentry) :
> > -ESTALE);
>
> If you'll be respinning this series, a handful of nits:
>
I see no need to respin the entire series. Just the last patch.
> - the line above has a double space
> - the trace point names here are a little long, will result in
> hard-to-read formatting in the trace log
> - checkpatch.pl complains about a couple of the later patches,
> where one arm of an "if" statement has braces but the other
> does not
>
> > }
> > if (dentry == NULL)
> > goto out;
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index 06dd0d337049..9abd1591a841 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -50,6 +50,36 @@ TRACE_EVENT(nfsd_compound_status,
> > __get_str(name), __entry->status)
> > )
> >
> > +DECLARE_EVENT_CLASS(nfsd_fh_err_class,
> > + TP_PROTO(struct svc_rqst *rqstp,
> > + struct svc_fh *fhp,
> > + int status),
> > + TP_ARGS(rqstp, fhp, status),
> > + TP_STRUCT__entry(
> > + __field(u32, xid)
> > + __field(u32, fh_hash)
> > + __field(int, status)
> > + ),
> > + TP_fast_assign(
> > + __entry->xid = be32_to_cpu(rqstp->rq_xid);
> > + __entry->fh_hash = knfsd_fh_hash(&fhp->fh_handle);
> > + __entry->status = status;
> > + ),
> > + TP_printk("xid=0x%08x fh_hash=0x%08x status=%d",
> > + __entry->xid, __entry->fh_hash,
> > + __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_NFSD_FH_ERR_EVENT(set_fh_dentry_badexport);
> > +DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badhandle);
> > +
> > DECLARE_EVENT_CLASS(nfsd_io_class,
> > TP_PROTO(struct svc_rqst *rqstp,
> > struct svc_fh *fhp,
> > --
> > 2.24.1
> >
>
> --
> Chuck Lever
> chucklever@xxxxxxxxx
>
>
>
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
