git: 50dcff0816e5 - main - nfscl: Add setting n_localmodtime to the Write RPC code
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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); }