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);
}