[PATCH 0/3] Cache consistency updates

[PATCH 0/3] Cache consistency updates

[Trond Myklebust]

Add a 'deferred cache invalidation' mode that we can use when we thing
the NFS cache may have been changed on the server, but the file in
question is already open and is cached on the client. In order to avoid
performance issues due to false positive detection of server changes,
we defer invalidating the cache until the file has been closed, and
the cached data is no longer in active use.

Trond Myklebust (3):
  NFS: Fix up ftrace printout of the cache invalidation flags
  NFS: Fix up ftrace logging of nfs_inode flags
  NFS: Add deferred cache invalidation for close-to-open consistency
    violations

 fs/nfs/dir.c           |  4 ++++
 fs/nfs/inode.c         | 15 +++++++++++----
 fs/nfs/nfstrace.h      | 22 ++++++++++++++--------
 include/linux/nfs_fs.h |  2 ++
 4 files changed, 31 insertions(+), 12 deletions(-)

-- 
2.21.0

[PATCH 1/3] NFS: Fix up ftrace printout of the cache invalidation flags

[Trond Myklebust]

Update the ftrace printouts so they correctly reflect the cache
invalidation flags.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfstrace.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index a0d6910aa03a..c40aad6ef3ff 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -25,14 +25,18 @@
 
 #define nfs_show_cache_validity(v) \
 	__print_flags(v, "|", \
-			{ NFS_INO_INVALID_ATTR, "INVALID_ATTR" }, \
 			{ NFS_INO_INVALID_DATA, "INVALID_DATA" }, \
 			{ NFS_INO_INVALID_ATIME, "INVALID_ATIME" }, \
 			{ NFS_INO_INVALID_ACCESS, "INVALID_ACCESS" }, \
 			{ NFS_INO_INVALID_ACL, "INVALID_ACL" }, \
 			{ NFS_INO_REVAL_PAGECACHE, "REVAL_PAGECACHE" }, \
 			{ NFS_INO_REVAL_FORCED, "REVAL_FORCED" }, \
-			{ NFS_INO_INVALID_LABEL, "INVALID_LABEL" })
+			{ NFS_INO_INVALID_LABEL, "INVALID_LABEL" }, \
+			{ NFS_INO_INVALID_CHANGE, "INVALID_CHANGE" }, \
+			{ NFS_INO_INVALID_CTIME, "INVALID_CTIME" }, \
+			{ NFS_INO_INVALID_MTIME, "INVALID_MTIME" }, \
+			{ NFS_INO_INVALID_SIZE, "INVALID_SIZE" }, \
+			{ NFS_INO_INVALID_OTHER, "INVALID_OTHER" })
 
 #define nfs_show_nfsi_flags(v) \
 	__print_flags(v, "|", \
-- 
2.21.0

[PATCH 2/3] NFS: Fix up ftrace logging of nfs_inode flags

[Trond Myklebust]

Also print out the layoutstats and O_DIRECT flag settings.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfstrace.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index c40aad6ef3ff..864ae1c11bd2 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -40,12 +40,14 @@
 
 #define nfs_show_nfsi_flags(v) \
 	__print_flags(v, "|", \
-			{ 1 << NFS_INO_ADVISE_RDPLUS, "ADVISE_RDPLUS" }, \
-			{ 1 << NFS_INO_STALE, "STALE" }, \
-			{ 1 << NFS_INO_INVALIDATING, "INVALIDATING" }, \
-			{ 1 << NFS_INO_FSCACHE, "FSCACHE" }, \
-			{ 1 << NFS_INO_LAYOUTCOMMIT, "NEED_LAYOUTCOMMIT" }, \
-			{ 1 << NFS_INO_LAYOUTCOMMITTING, "LAYOUTCOMMIT" })
+			{ BIT(NFS_INO_ADVISE_RDPLUS), "ADVISE_RDPLUS" }, \
+			{ BIT(NFS_INO_STALE), "STALE" }, \
+			{ BIT(NFS_INO_INVALIDATING), "INVALIDATING" }, \
+			{ BIT(NFS_INO_FSCACHE), "FSCACHE" }, \
+			{ BIT(NFS_INO_LAYOUTCOMMIT), "NEED_LAYOUTCOMMIT" }, \
+			{ BIT(NFS_INO_LAYOUTCOMMITTING), "LAYOUTCOMMIT" }, \
+			{ BIT(NFS_INO_LAYOUTSTATS), "LAYOUTSTATS" }, \
+			{ BIT(NFS_INO_ODIRECT), "O_DIRECT" })
 
 DECLARE_EVENT_CLASS(nfs_inode_event,
 		TP_PROTO(
-- 
2.21.0

[PATCH 3/3] NFS: Add deferred cache invalidation for close-to-open consistency violations

[Trond Myklebust]

If the client detects that close-to-open cache consistency has been
violated, and that the file or directory has been changed on the
server, then do a cache invalidation when we're done working with
the file.
The reason we don't do an immediate cache invalidation is that we
want to avoid performance problems due to false positives. Also,
note that we cannot guarantee cache consistency in this situation
even if we do invalidate the cache.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/dir.c           |  4 ++++
 fs/nfs/inode.c         | 15 +++++++++++----
 include/linux/nfs_fs.h |  2 ++
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 57b6a45576ad..bd1f9555447b 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -80,6 +80,10 @@ static struct nfs_open_dir_context *alloc_nfs_open_dir_context(struct inode *dir
 		ctx->dup_cookie = 0;
 		ctx->cred = get_cred(cred);
 		spin_lock(&dir->i_lock);
+		if (list_empty(&nfsi->open_files) &&
+		    (nfsi->cache_validity & NFS_INO_DATA_INVAL_DEFER))
+			nfsi->cache_validity |= NFS_INO_INVALID_DATA |
+				NFS_INO_REVAL_FORCED;
 		list_add(&ctx->list, &nfsi->open_files);
 		spin_unlock(&dir->i_lock);
 		return ctx;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 0b4a1a974411..8274d021d46a 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -208,7 +208,7 @@ static void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
 	}
 
 	if (inode->i_mapping->nrpages == 0)
-		flags &= ~NFS_INO_INVALID_DATA;
+		flags &= ~(NFS_INO_INVALID_DATA|NFS_INO_DATA_INVAL_DEFER);
 	nfsi->cache_validity |= flags;
 	if (flags & NFS_INO_INVALID_DATA)
 		nfs_fscache_invalidate(inode);
@@ -652,7 +652,8 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset)
 	i_size_write(inode, offset);
 	/* Optimisation */
 	if (offset == 0)
-		NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_DATA;
+		NFS_I(inode)->cache_validity &= ~(NFS_INO_INVALID_DATA |
+				NFS_INO_DATA_INVAL_DEFER);
 	NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_SIZE;
 
 	spin_unlock(&inode->i_lock);
@@ -1032,6 +1033,10 @@ void nfs_inode_attach_open_context(struct nfs_open_context *ctx)
 	struct nfs_inode *nfsi = NFS_I(inode);
 
 	spin_lock(&inode->i_lock);
+	if (list_empty(&nfsi->open_files) &&
+	    (nfsi->cache_validity & NFS_INO_DATA_INVAL_DEFER))
+		nfsi->cache_validity |= NFS_INO_INVALID_DATA |
+			NFS_INO_REVAL_FORCED;
 	list_add_tail_rcu(&ctx->list, &nfsi->open_files);
 	spin_unlock(&inode->i_lock);
 }
@@ -1312,7 +1317,8 @@ int nfs_revalidate_mapping(struct inode *inode,
 
 	set_bit(NFS_INO_INVALIDATING, bitlock);
 	smp_wmb();
-	nfsi->cache_validity &= ~NFS_INO_INVALID_DATA;
+	nfsi->cache_validity &= ~(NFS_INO_INVALID_DATA|
+			NFS_INO_DATA_INVAL_DEFER);
 	spin_unlock(&inode->i_lock);
 	trace_nfs_invalidate_mapping_enter(inode);
 	ret = nfs_invalidate_mapping(inode, mapping);
@@ -1870,7 +1876,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 				dprintk("NFS: change_attr change on server for file %s/%ld\n",
 						inode->i_sb->s_id,
 						inode->i_ino);
-			}
+			} else if (!have_delegation)
+				nfsi->cache_validity |= NFS_INO_DATA_INVAL_DEFER;
 			inode_set_iversion_raw(inode, fattr->change_attr);
 			attr_changed = true;
 		}
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index d363d5765cdf..0a11712a80e3 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -223,6 +223,8 @@ struct nfs4_copy_state {
 #define NFS_INO_INVALID_MTIME	BIT(10)		/* cached mtime is invalid */
 #define NFS_INO_INVALID_SIZE	BIT(11)		/* cached size is invalid */
 #define NFS_INO_INVALID_OTHER	BIT(12)		/* other attrs are invalid */
+#define NFS_INO_DATA_INVAL_DEFER	\
+				BIT(13)		/* Deferred cache invalidation */
 
 #define NFS_INO_INVALID_ATTR	(NFS_INO_INVALID_CHANGE \
 		| NFS_INO_INVALID_CTIME \
-- 
2.21.0

Re: [PATCH 0/3] Cache consistency updates

[Chuck Lever]

> On Jun 11, 2019, at 2:25 PM, Trond Myklebust <trondmy@gmail.com> wrote:
> 
> Add a 'deferred cache invalidation' mode that we can use when we thing
> the NFS cache may have been changed on the server, but the file in
> question is already open and is cached on the client. In order to avoid
> performance issues due to false positive detection of server changes,
> we defer invalidating the cache until the file has been closed, and
> the cached data is no longer in active use.
> 
> Trond Myklebust (3):
>  NFS: Fix up ftrace printout of the cache invalidation flags
>  NFS: Fix up ftrace logging of nfs_inode flags

I also fixed these items in my for-5.3 patch series, but
my patches add TRACE_DEFINE_ENUM definitions.


>  NFS: Add deferred cache invalidation for close-to-open consistency
>    violations
> 
> fs/nfs/dir.c           |  4 ++++
> fs/nfs/inode.c         | 15 +++++++++++----
> fs/nfs/nfstrace.h      | 22 ++++++++++++++--------
> include/linux/nfs_fs.h |  2 ++
> 4 files changed, 31 insertions(+), 12 deletions(-)
> 
> -- 
> 2.21.0
> 

--
Chuck Lever

Re: [PATCH 0/3] Cache consistency updates

[Trond Myklebust]

On Tue, 2019-06-11 at 14:31 -0400, Chuck Lever wrote:
> > On Jun 11, 2019, at 2:25 PM, Trond Myklebust <trondmy@gmail.com>
> > wrote:
> > 
> > Add a 'deferred cache invalidation' mode that we can use when we
> > thing
> > the NFS cache may have been changed on the server, but the file in
> > question is already open and is cached on the client. In order to
> > avoid
> > performance issues due to false positive detection of server
> > changes,
> > we defer invalidating the cache until the file has been closed, and
> > the cached data is no longer in active use.
> > 
> > Trond Myklebust (3):
> >  NFS: Fix up ftrace printout of the cache invalidation flags
> >  NFS: Fix up ftrace logging of nfs_inode flags
> 
> I also fixed these items in my for-5.3 patch series, but
> my patches add TRACE_DEFINE_ENUM definitions.

Oh. I missed those because they were embedded with the RDMA changes.

Can you please fix them up to also change the (1 << NFS_INO_*) stuff to
use the BIT() macro? That causes an expansion to an unsigned long type
instead of the current signed int.

> 
> >  NFS: Add deferred cache invalidation for close-to-open consistency
> >    violations
> > 
> > fs/nfs/dir.c           |  4 ++++
> > fs/nfs/inode.c         | 15 +++++++++++----
> > fs/nfs/nfstrace.h      | 22 ++++++++++++++--------
> > include/linux/nfs_fs.h |  2 ++
> > 4 files changed, 31 insertions(+), 12 deletions(-)
> > 
> > -- 
> > 2.21.0
> > 
> 
> --
> Chuck Lever
> 
> 
> 
-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: [PATCH 0/3] Cache consistency updates

[Chuck Lever]

> On Jun 11, 2019, at 3:21 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Tue, 2019-06-11 at 14:31 -0400, Chuck Lever wrote:
>>> On Jun 11, 2019, at 2:25 PM, Trond Myklebust <trondmy@gmail.com>
>>> wrote:
>>> 
>>> Add a 'deferred cache invalidation' mode that we can use when we
>>> thing
>>> the NFS cache may have been changed on the server, but the file in
>>> question is already open and is cached on the client. In order to
>>> avoid
>>> performance issues due to false positive detection of server
>>> changes,
>>> we defer invalidating the cache until the file has been closed, and
>>> the cached data is no longer in active use.
>>> 
>>> Trond Myklebust (3):
>>> NFS: Fix up ftrace printout of the cache invalidation flags
>>> NFS: Fix up ftrace logging of nfs_inode flags
>> 
>> I also fixed these items in my for-5.3 patch series, but
>> my patches add TRACE_DEFINE_ENUM definitions.
> 
> Oh. I missed those because they were embedded with the RDMA changes.
> 
> Can you please fix them up to also change the (1 << NFS_INO_*) stuff to
> use the BIT() macro? That causes an expansion to an unsigned long type
> instead of the current signed int.

Yes, absolutely.


>>> NFS: Add deferred cache invalidation for close-to-open consistency
>>>   violations
>>> 
>>> fs/nfs/dir.c           |  4 ++++
>>> fs/nfs/inode.c         | 15 +++++++++++----
>>> fs/nfs/nfstrace.h      | 22 ++++++++++++++--------
>>> include/linux/nfs_fs.h |  2 ++
>>> 4 files changed, 31 insertions(+), 12 deletions(-)
>>> 
>>> -- 
>>> 2.21.0
>>> 
>> 
>> --
>> Chuck Lever
>> 
>> 
>> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com

--
Chuck Lever