2022-02-24 00:42:59

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v7 15/21] NFS: Don't request readdirplus when revalidation was forced

From: Trond Myklebust <[email protected]>

If the revalidation was forced, due to the presence of a LOOKUP_EXCL or
a LOOKUP_REVAL flag, then readdirplus won't help. It also can't help
when we're doing a path component lookup.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index a9098d5a9fc8..54f0d37485d5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -682,10 +682,13 @@ void nfs_readdir_record_entry_cache_miss(struct inode *dir)
}
}

-static void nfs_lookup_advise_force_readdirplus(struct inode *dir)
+static void nfs_lookup_advise_force_readdirplus(struct inode *dir,
+ unsigned int flags)
{
if (nfs_server_capable(dir, NFS_CAP_CASE_INSENSITIVE))
return;
+ if (flags & (LOOKUP_EXCL | LOOKUP_PARENT | LOOKUP_REVAL))
+ return;
nfs_readdir_record_entry_cache_miss(dir);
}

@@ -1583,15 +1586,17 @@ nfs_lookup_revalidate_delegated(struct inode *dir, struct dentry *dentry,
return nfs_lookup_revalidate_done(dir, dentry, inode, 1);
}

-static int
-nfs_lookup_revalidate_dentry(struct inode *dir, struct dentry *dentry,
- struct inode *inode)
+static int nfs_lookup_revalidate_dentry(struct inode *dir,
+ struct dentry *dentry,
+ struct inode *inode, unsigned int flags)
{
struct nfs_fh *fhandle;
struct nfs_fattr *fattr;
unsigned long dir_verifier;
int ret;

+ trace_nfs_lookup_revalidate_enter(dir, dentry, flags);
+
ret = -ENOMEM;
fhandle = nfs_alloc_fhandle();
fattr = nfs_alloc_fattr_with_label(NFS_SERVER(inode));
@@ -1612,6 +1617,10 @@ nfs_lookup_revalidate_dentry(struct inode *dir, struct dentry *dentry,
}
goto out;
}
+
+ /* Request help from readdirplus */
+ nfs_lookup_advise_force_readdirplus(dir, flags);
+
ret = 0;
if (nfs_compare_fh(NFS_FH(inode), fhandle))
goto out;
@@ -1621,8 +1630,6 @@ nfs_lookup_revalidate_dentry(struct inode *dir, struct dentry *dentry,
nfs_setsecurity(inode, fattr);
nfs_set_verifier(dentry, dir_verifier);

- /* set a readdirplus hint that we had a cache miss */
- nfs_lookup_advise_force_readdirplus(dir);
ret = 1;
out:
nfs_free_fattr(fattr);
@@ -1688,8 +1695,7 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
if (NFS_STALE(inode))
goto out_bad;

- trace_nfs_lookup_revalidate_enter(dir, dentry, flags);
- return nfs_lookup_revalidate_dentry(dir, dentry, inode);
+ return nfs_lookup_revalidate_dentry(dir, dentry, inode, flags);
out_valid:
return nfs_lookup_revalidate_done(dir, dentry, inode, 1);
out_bad:
@@ -1883,7 +1889,7 @@ struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, unsigned in
goto out;

/* Notify readdir to use READDIRPLUS */
- nfs_lookup_advise_force_readdirplus(dir);
+ nfs_lookup_advise_force_readdirplus(dir, flags);

no_entry:
res = d_splice_alias(inode, dentry);
@@ -2146,7 +2152,7 @@ nfs4_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
reval_dentry:
if (flags & LOOKUP_RCU)
return -ECHILD;
- return nfs_lookup_revalidate_dentry(dir, dentry, inode);
+ return nfs_lookup_revalidate_dentry(dir, dentry, inode, flags);

full_reval:
return nfs_do_lookup_revalidate(dir, dentry, flags);
--
2.35.1


2022-02-24 00:54:42

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v7 16/21] NFS: Add basic readdir tracing

From: Trond Myklebust <[email protected]>

Add tracing to track how often the client goes to the server for updated
readdir information.

Signed-off-by: Trond Myklebust <[email protected]>
---
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)
+ ),
+
+ TP_fast_assign(
+ const struct inode *dir = file_inode(file);
+ const struct nfs_inode *nfsi = NFS_I(dir);
+
+ __entry->dev = dir->i_sb->s_dev;
+ __entry->fileid = nfsi->fileid;
+ __entry->fhandle = nfs_fhandle_hash(&nfsi->fh);
+ __entry->version = inode_peek_iversion_raw(dir);
+ if (cookie != 0)
+ memcpy(__entry->verifier, verifier,
+ NFS4_VERIFIER_SIZE);
+ else
+ memset(__entry->verifier, 0,
+ NFS4_VERIFIER_SIZE);
+ __entry->cookie = cookie;
+ __entry->index = page_index;
+ __entry->dtsize = dtsize;
+ ),
+
+ TP_printk(
+ "fileid=%02x:%02x:%llu fhandle=0x%08x version=%llu "
+ "cookie=%s:0x%llx cache_index=%lu dtsize=%u",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ (unsigned long long)__entry->fileid, __entry->fhandle,
+ __entry->version, show_nfs4_verifier(__entry->verifier),
+ (unsigned long long)__entry->cookie, __entry->index,
+ __entry->dtsize
+ )
+);
+
+#define DEFINE_NFS_READDIR_EVENT(name) \
+ DEFINE_EVENT(nfs_readdir_event, name, \
+ 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))
+
+DEFINE_NFS_READDIR_EVENT(nfs_readdir_cache_fill);
+DEFINE_NFS_READDIR_EVENT(nfs_readdir_uncached);
+
DECLARE_EVENT_CLASS(nfs_lookup_event,
TP_PROTO(
const struct inode *dir,
--
2.35.1

2022-02-24 01:49:41

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v7 17/21] NFS: Trace effects of readdirplus on the dcache

From: Trond Myklebust <[email protected]>

Trace the effects of readdirplus on attribute and dentry revalidation.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 5 +++++
fs/nfs/nfstrace.h | 3 +++
2 files changed, 8 insertions(+)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 41e2d02d8611..95b18d1ad0cf 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -742,8 +742,12 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry,
status = nfs_refresh_inode(d_inode(dentry), entry->fattr);
if (!status)
nfs_setsecurity(d_inode(dentry), entry->fattr);
+ trace_nfs_readdir_lookup_revalidate(d_inode(parent),
+ dentry, 0, status);
goto out;
} else {
+ trace_nfs_readdir_lookup_revalidate_failed(
+ d_inode(parent), dentry, 0);
d_invalidate(dentry);
dput(dentry);
dentry = NULL;
@@ -765,6 +769,7 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry,
dentry = alias;
}
nfs_set_verifier(dentry, dir_verifier);
+ trace_nfs_readdir_lookup(d_inode(parent), dentry, 0);
out:
dput(dentry);
}
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index c2d0543ecb2d..7c1102b991d0 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -432,6 +432,9 @@ DEFINE_NFS_LOOKUP_EVENT(nfs_lookup_enter);
DEFINE_NFS_LOOKUP_EVENT_DONE(nfs_lookup_exit);
DEFINE_NFS_LOOKUP_EVENT(nfs_lookup_revalidate_enter);
DEFINE_NFS_LOOKUP_EVENT_DONE(nfs_lookup_revalidate_exit);
+DEFINE_NFS_LOOKUP_EVENT(nfs_readdir_lookup);
+DEFINE_NFS_LOOKUP_EVENT(nfs_readdir_lookup_revalidate_failed);
+DEFINE_NFS_LOOKUP_EVENT_DONE(nfs_readdir_lookup_revalidate);

TRACE_EVENT(nfs_atomic_open_enter,
TP_PROTO(
--
2.35.1

2022-02-24 16:39:51

by Benjamin Coddington

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

On 23 Feb 2022, at 16:13, [email protected] wrote:

> From: Trond Myklebust <[email protected]>
>
> Add tracing to track how often the client goes to the server for
> updated
> readdir information.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> 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.

Ben

2022-02-25 03:57:03

by Trond Myklebust

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

On Thu, 2022-02-24 at 10:53 -0500, Benjamin Coddington wrote:
> On 23 Feb 2022, at 16:13, [email protected] wrote:
>
> > From: Trond Myklebust <[email protected]>
> >
> > Add tracing to track how often the client goes to the server for
> > updated
> > readdir information.
> >
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> >  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
[email protected]