> On Mar 3, 2020, at 12:59 AM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
>
> 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.
Fair enough.
>> - 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
>
>
