Re: [PATCH v7 16/21] NFS: Add basic readdir tracing

[Trond Myklebust <trondmy@hammerspace.com>]

On Thu, 2022-02-24 at 10:53 -0500, Benjamin Coddington wrote:
> On 23 Feb 2022, at 16:13, trondmy@kernel.org wrote:
> 
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > 
> > Add tracing to track how often the client goes to the server for 
> > updated
> > readdir information.
> > 
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> >  fs/nfs/dir.c      | 13 ++++++++-
> >  fs/nfs/nfstrace.h | 68 
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 80 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index 54f0d37485d5..41e2d02d8611 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -969,10 +969,14 @@ static int find_and_lock_cache_page(struct 
> > nfs_readdir_descriptor *desc)
> >                 return -ENOMEM;
> >         if (nfs_readdir_page_needs_filling(desc->page)) {
> >                 desc->page_index_max = desc->page_index;
> > +               trace_nfs_readdir_cache_fill(desc->file, nfsi-
> > >cookieverf,
> > +                                            desc->last_cookie,
> > +                                            desc->page_index,
> > desc->dtsize);
> >                 res = nfs_readdir_xdr_to_array(desc, nfsi-
> > >cookieverf, verf,
> >                                                &desc->page, 1);
> >                 if (res < 0) {
> >                         nfs_readdir_page_unlock_and_put_cached(desc
> > );
> > +                       trace_nfs_readdir_cache_fill_done(inode,
> > res);
> >                         if (res == -EBADCOOKIE || res == -ENOTSYNC)
> > {
> >                                 invalidate_inode_pages2(desc->file-
> > >f_mapping);
> >                                 desc->page_index = 0;
> > @@ -1090,7 +1094,14 @@ static int uncached_readdir(struct 
> > nfs_readdir_descriptor *desc)
> >         desc->duped = 0;
> >         desc->page_index_max = 0;
> > 
> > +       trace_nfs_readdir_uncached(desc->file, desc->verf, 
> > desc->last_cookie,
> > +                                  -1, desc->dtsize);
> > +
> >         status = nfs_readdir_xdr_to_array(desc, desc->verf, verf,
> > arrays, 
> > sz);
> > +       if (status < 0) {
> > +               trace_nfs_readdir_uncached_done(file_inode(desc-
> > >file), status);
> > +               goto out_free;
> > +       }
> > 
> >         for (i = 0; !desc->eob && i < sz && arrays[i]; i++) {
> >                 desc->page = arrays[i];
> > @@ -1109,7 +1120,7 @@ static int uncached_readdir(struct 
> > nfs_readdir_descriptor *desc)
> >                          i < (desc->page_index_max >> 1))
> >                         nfs_shrink_dtsize(desc);
> >         }
> > -
> > +out_free:
> >         for (i = 0; i < sz && arrays[i]; i++)
> >                 nfs_readdir_page_array_free(arrays[i]);
> >  out:
> > diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
> > index 3672f6703ee7..c2d0543ecb2d 100644
> > --- a/fs/nfs/nfstrace.h
> > +++ b/fs/nfs/nfstrace.h
> > @@ -160,6 +160,8 @@ DEFINE_NFS_INODE_EVENT(nfs_fsync_enter);
> >  DEFINE_NFS_INODE_EVENT_DONE(nfs_fsync_exit);
> >  DEFINE_NFS_INODE_EVENT(nfs_access_enter);
> >  DEFINE_NFS_INODE_EVENT_DONE(nfs_set_cache_invalid);
> > +DEFINE_NFS_INODE_EVENT_DONE(nfs_readdir_cache_fill_done);
> > +DEFINE_NFS_INODE_EVENT_DONE(nfs_readdir_uncached_done);
> > 
> >  TRACE_EVENT(nfs_access_exit,
> >                 TP_PROTO(
> > @@ -271,6 +273,72 @@ DEFINE_NFS_UPDATE_SIZE_EVENT(wcc);
> >  DEFINE_NFS_UPDATE_SIZE_EVENT(update);
> >  DEFINE_NFS_UPDATE_SIZE_EVENT(grow);
> > 
> > +DECLARE_EVENT_CLASS(nfs_readdir_event,
> > +               TP_PROTO(
> > +                       const struct file *file,
> > +                       const __be32 *verifier,
> > +                       u64 cookie,
> > +                       pgoff_t page_index,
> > +                       unsigned int dtsize
> > +               ),
> > +
> > +               TP_ARGS(file, verifier, cookie, page_index,
> > dtsize),
> > +
> > +               TP_STRUCT__entry(
> > +                       __field(dev_t, dev)
> > +                       __field(u32, fhandle)
> > +                       __field(u64, fileid)
> > +                       __field(u64, version)
> > +                       __array(char, verifier, NFS4_VERIFIER_SIZE)
> > +                       __field(u64, cookie)
> > +                       __field(pgoff_t, index)
> > +                       __field(unsigned int, dtsize)
> 
> 
> I'd like to be able to see the change_attr too, whether or not it's
> the
> cache_change_attribute or i_version.
> 

It is reported by the __entry->version.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com