[PATCH v2 3/3] NFS: stamp after the server's ctime following a CLONE

[Jeff Layton <jlayton@kernel.org>]

nfs42_copy_dest_done() stamps the client's own c/mtime when it holds
delegated timestamps. A server that stamps c/mtime on CLONE (as nfsd did
before "nfsd: mark the delegation written on COPY/CLONE instead of
stamping c/mtime", and as btrfs did before "btrfs: honor FMODE_NOCMTIME
when cloning") can end up with a ctime newer than the client's coarse
stamp, which it won't move back when the holder returns its times.

The stamp didn't survive anyway: nfs_post_op_update_inode() then marks
ctime invalid, so nfs_fattr_fixup_delegated() keeps the reply's ctime
and nfs_update_inode() writes it over the stamp. Against a server that
no longer stamps on CLONE, the client's ctime goes back to its old value
until DELEGRETURN.

- Pass the CLONE reply's attributes to nfs42_copy_dest_done() and raise
  the cached ctime to the server's before stamping. COPY returns no
  attributes, so it passes NULL.
- Stamp before dropping i_lock, so a concurrent refresh can't take the
  server's times while ctime and mtime are marked invalid.
- Under delegated timestamps, apply the reply with nfs_refresh_inode(),
  which doesn't mark ctime invalid.

Fixes: 6e7daa3dad29 ("NFSv4.2: fix CLONE/COPY attrs in presence of delegated attributes")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfs/delegation.h |  2 ++
 fs/nfs/inode.c      |  6 ++++--
 fs/nfs/nfs42proc.c  | 25 ++++++++++++++++++++-----
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index fba4699952b8..cd6142939d34 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -95,6 +95,8 @@ void nfs4_inode_make_writeable(struct inode *inode);
 void nfs_update_delegated_atime(struct inode *inode);
 void nfs_update_delegated_mtime(struct inode *inode);
 void nfs_update_delegated_mtime_locked(struct inode *inode);
+void nfs_delegated_ctime_raise(struct inode *inode,
+			       const struct nfs_fattr *fattr);
 
 static inline int nfs_have_read_or_write_delegation(struct inode *inode)
 {
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 270d8bb46110..5a682f74d7d1 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -652,8 +652,8 @@ static struct timespec64 nfs_delegated_stamp(struct inode *inode)
  * cached ctime to the server's so that the next delegated stamp lands
  * after it.
  */
-static void nfs_delegated_ctime_raise(struct inode *inode,
-				      const struct nfs_fattr *fattr)
+void nfs_delegated_ctime_raise(struct inode *inode,
+			       const struct nfs_fattr *fattr)
 {
 	struct timespec64 ctime = inode_get_ctime(inode);
 
@@ -661,6 +661,7 @@ static void nfs_delegated_ctime_raise(struct inode *inode,
 	    timespec64_compare(&fattr->ctime, &ctime) > 0)
 		inode_set_ctime_to_ts(inode, fattr->ctime);
 }
+EXPORT_SYMBOL_GPL(nfs_delegated_ctime_raise);
 
 static void nfs_set_timestamps_to_ts(struct inode *inode, struct iattr *attr)
 {
@@ -711,6 +712,7 @@ void nfs_update_delegated_mtime_locked(struct inode *inode)
 	    nfs_have_directory_delegation(inode))
 		nfs_update_mtime(inode);
 }
+EXPORT_SYMBOL_GPL(nfs_update_delegated_mtime_locked);
 
 void nfs_update_delegated_mtime(struct inode *inode)
 {
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index ab86246fc364..09b4a6ecad88 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -380,6 +380,7 @@ static int process_copy_commit(struct file *dst, loff_t pos_dst,
  * @pos: destination offset
  * @len: copy length
  * @oldsize: length of the file prior to clone/copy
+ * @fattr: destination attributes from the reply, or NULL
  *
  * Punch a hole in the inode page cache, so that the NFS client will
  * know to retrieve new data.
@@ -387,7 +388,7 @@ static int process_copy_commit(struct file *dst, loff_t pos_dst,
  * invalid cached values for change attribute, ctime, mtime and space used.
  */
 static void nfs42_copy_dest_done(struct file *file, loff_t pos, loff_t len,
-				 loff_t oldsize)
+				 loff_t oldsize, const struct nfs_fattr *fattr)
 {
 	struct inode *inode = file_inode(file);
 	struct address_space *mapping = file->f_mapping;
@@ -405,8 +406,12 @@ static void nfs42_copy_dest_done(struct file *file, loff_t pos, loff_t len,
 					     NFS_INO_INVALID_CTIME |
 					     NFS_INO_INVALID_MTIME |
 					     NFS_INO_INVALID_BLOCKS);
+	/* The server may have stamped c/mtime; stamp after it */
+	if (fattr && nfs_have_delegated_mtime(inode))
+		nfs_delegated_ctime_raise(inode, fattr);
+	/* Before a refresh can see the times invalid and take the server's */
+	nfs_update_delegated_mtime_locked(inode);
 	spin_unlock(&inode->i_lock);
-	nfs_update_delegated_mtime(inode);
 }
 
 static ssize_t _nfs42_proc_copy(struct file *src,
@@ -506,7 +511,8 @@ static ssize_t _nfs42_proc_copy(struct file *src,
 			goto out;
 	}
 
-	nfs42_copy_dest_done(dst, pos_dst, res->write_res.count, oldsize_dst);
+	nfs42_copy_dest_done(dst, pos_dst, res->write_res.count, oldsize_dst,
+			     NULL);
 	nfs_invalidate_atime(src_inode);
 	status = res->write_res.count;
 out:
@@ -1311,8 +1317,17 @@ static int _nfs42_proc_clone(struct rpc_message *msg, struct file *src_f,
 		/* a zero-length count means clone to EOF in src */
 		if (count == 0 && res.dst_fattr->valid & NFS_ATTR_FATTR_SIZE)
 			count = nfs_size_to_loff_t(res.dst_fattr->size) - dst_offset;
-		nfs42_copy_dest_done(dst_f, dst_offset, count, oldsize_dst);
-		status = nfs_post_op_update_inode(dst_inode, res.dst_fattr);
+		nfs42_copy_dest_done(dst_f, dst_offset, count, oldsize_dst,
+				     res.dst_fattr);
+		/*
+		 * nfs_post_op_update_inode() would mark ctime invalid, so the
+		 * reply's ctime would overwrite the delegated stamp.
+		 */
+		if (nfs_have_delegated_mtime(dst_inode))
+			status = nfs_refresh_inode(dst_inode, res.dst_fattr);
+		else
+			status = nfs_post_op_update_inode(dst_inode,
+							  res.dst_fattr);
 	}
 
 	kfree(res.dst_fattr);

-- 
2.56.0