git: 50dcff0816e5 - main - nfscl: Add setting n_localmodtime to the Write RPC code

From: Rick Macklem <rmacklem_at_FreeBSD.org>
Date: Sun, 31 Oct 2021 00:12:15 UTC
The branch main has been updated by rmacklem:

URL: https://cgit.FreeBSD.org/src/commit/?id=50dcff0816e5e4aa94f51ce27da5121e49902996

commit 50dcff0816e5e4aa94f51ce27da5121e49902996
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-10-31 00:08:28 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-10-31 00:08:28 +0000

    nfscl: Add setting n_localmodtime to the Write RPC code
    
    Similar to commit 2be417843a, I believe there could be a race between
    the NFS client VOP_LOOKUP() and file Writing that could result in stale
    file attributes being loaded into the NFS vnode by VOP_LOOKUP().
    
    I have not been able to reproduce a failure due to this race, but
    I believe that there are two possibilities:
    
    The Lookup RPC happens while VOP_WRITE() is being executed and loads
    stale file attributes after VOP_WRITE() returns when it has already
    completed the Write/Commit RPC(s).
    --> For this case, setting the local modify timestamp at the end of
      VOP_WRITE() should ensure that stale file attributes are not loaded.
    
    The Lookup RPC occurs after VOP_WRITE() has returned, while
    asynchronous Write/Commit RPCs are in progress and then is
    blocked by the vnode held by VOP_OPEN/VOP_CLOSE/VOP_FSYNC which
    will flush writes via ncl_flush() or ncl_vinvalbuf(), clearing the
    NMODIFIED flag (which indicates Writes-in-progress). The VOP_LOOKUP()
    then acquires the NFS vnode lock and fills in stale file attributes.
     --> Setting the local modify timestamp in ncl_flsuh() and ncl_vinvalbuf()
       when they clear NMODIFIED should ensure that stale file attributes
       are not loaded.
    
    This patch does the above.
    
    PR:     259071
    Reviewed by:    asomers
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D32677
---
 sys/fs/nfsclient/nfs_clbio.c   | 18 +++++++++++++++---
 sys/fs/nfsclient/nfs_clvnops.c |  7 +++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/sys/fs/nfsclient/nfs_clbio.c b/sys/fs/nfsclient/nfs_clbio.c
index 10a76f0a4b83..250d01d88948 100644
--- a/sys/fs/nfsclient/nfs_clbio.c
+++ b/sys/fs/nfsclient/nfs_clbio.c
@@ -907,6 +907,7 @@ ncl_write(struct vop_write_args *ap)
 	int bp_cached, n, on, error = 0, error1, save2, wouldcommit;
 	size_t orig_resid, local_resid;
 	off_t orig_size, tmp_off;
+	struct timespec ts;
 
 	KASSERT(uio->uio_rw == UIO_WRITE, ("ncl_write mode"));
 	KASSERT(uio->uio_segflg != UIO_USERSPACE || uio->uio_td == curthread,
@@ -1284,7 +1285,12 @@ again:
 			break;
 	} while (uio->uio_resid > 0 && n > 0);
 
-	if (error != 0) {
+	if (error == 0) {
+		nanouptime(&ts);
+		NFSLOCKNODE(np);
+		np->n_localmodtime = ts;
+		NFSUNLOCKNODE(np);
+	} else {
 		if (ioflag & IO_UNIT) {
 			VATTR_NULL(&vattr);
 			vattr.va_size = orig_size;
@@ -1356,6 +1362,7 @@ ncl_vinvalbuf(struct vnode *vp, int flags, struct thread *td, int intrflg)
 	struct nfsmount *nmp = VFSTONFS(vp->v_mount);
 	int error = 0, slpflag, slptimeo;
 	bool old_lock;
+	struct timespec ts;
 
 	ASSERT_VOP_LOCKED(vp, "ncl_vinvalbuf");
 
@@ -1400,16 +1407,21 @@ ncl_vinvalbuf(struct vnode *vp, int flags, struct thread *td, int intrflg)
 	}
 	if (NFSHASPNFS(nmp)) {
 		nfscl_layoutcommit(vp, td);
+		nanouptime(&ts);
 		/*
 		 * Invalidate the attribute cache, since writes to a DS
 		 * won't update the size attribute.
 		 */
 		NFSLOCKNODE(np);
 		np->n_attrstamp = 0;
-	} else
+	} else {
+		nanouptime(&ts);
 		NFSLOCKNODE(np);
-	if (np->n_directio_asyncwr == 0)
+	}
+	if (np->n_directio_asyncwr == 0 && (np->n_flag & NMODIFIED) != 0) {
+		np->n_localmodtime = ts;
 		np->n_flag &= ~NMODIFIED;
+	}
 	NFSUNLOCKNODE(np);
 out:
 	ncl_excl_finish(vp, old_lock);
diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c
index f63eadf26a91..1685edf5b2de 100644
--- a/sys/fs/nfsclient/nfs_clvnops.c
+++ b/sys/fs/nfsclient/nfs_clvnops.c
@@ -2914,6 +2914,7 @@ ncl_flush(struct vnode *vp, int waitfor, struct thread *td,
 #endif
 	struct buf *bvec_on_stack[NFS_COMMITBVECSIZ];
 	u_int bvecsize = 0, bveccount;
+	struct timespec ts;
 
 	if (called_from_renewthread != 0)
 		slptimeo = hz;
@@ -3234,6 +3235,12 @@ done:
 		vn_printf(vp, "ncl_flush failed");
 		error = called_from_renewthread != 0 ? EIO : EBUSY;
 	}
+	if (error == 0) {
+		nanouptime(&ts);
+		NFSLOCKNODE(np);
+		np->n_localmodtime = ts;
+		NFSUNLOCKNODE(np);
+	}
 	return (error);
 }