* [PATCH 0/9] Fix error reporting for NFS writes @ 2020-01-06 18:40 Trond Myklebust 2020-01-06 18:40 ` [PATCH 1/9] nfsd: Allow nfsd_vfs_write() to take the nfsd_file as an argument Trond Myklebust 2020-01-22 15:27 ` [PATCH 0/9] Fix error reporting for NFS writes J. Bruce Fields 0 siblings, 2 replies; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs In cases where we have transient errors, such as ENOSPC, it is important to ensure that errors are reported on all writes that may be affected. The problem we have is that not all errors are guaranteed to be reported at write time. Some are reported only when we call fsync(). In particular, this can be a problem for stable NFS writes. Since most filesystems protect the write to the page cache with the inode lock, but do not protect the subsequent call to generic_write_sync(), this means that if we have parallel writes to the same file, we can end up assigning the error to the wrong stable write call. If the application expects to be able to fix the transient errors, it may end up replaying the wrong write. One area where we have seen this happen is in flexfiles writes, where the server is capable of freeing up space on the DS in case of ENOSPC. The other area where we have seen a similar problem is when we have unstable writes, and the client sends a backgrounded commit in order to free up memory. If there are outstanding writes while the commit gets a transient error and bumps the write verifier, then we want to ensure that those writes get the approprite write verifier depending on whether they were affected by the fsync() or not. Right now, because the NFSv3 verifier is set in the XDR encoder well after the write is done, there is fairly large window for a race with a background commit. This patch series deals with both issues by adding per-file-descriptor locking that ensures that writes, fsync error handling, and write verifier updates are appropriately serialised. Trond Myklebust (9): nfsd: Allow nfsd_vfs_write() to take the nfsd_file as an argument nfsd: Fix stable writes nfsd: Update the boot verifier on stable writes too. nfsd: Pass the nfsd_file as arguments to nfsd4_clone_file_range() nfsd: Ensure exclusion between CLONE and WRITE errors sunrpc: Fix potential leaks in sunrpc_cache_unhash() sunrpc: clean up cache entry add/remove from hashtable nfsd: Ensure sampling of the commit verifier is atomic with the commit nfsd: Ensure sampling of the write verifier is atomic with the write fs/nfsd/filecache.c | 1 + fs/nfsd/filecache.h | 1 + fs/nfsd/nfs3proc.c | 5 +-- fs/nfsd/nfs3xdr.c | 16 +++------ fs/nfsd/nfs4proc.c | 14 ++++---- fs/nfsd/nfsproc.c | 2 +- fs/nfsd/vfs.c | 79 ++++++++++++++++++++++++++++++++++----------- fs/nfsd/vfs.h | 16 +++++---- fs/nfsd/xdr3.h | 2 ++ net/sunrpc/cache.c | 48 ++++++++++++++------------- 10 files changed, 115 insertions(+), 69 deletions(-) -- 2.24.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/9] nfsd: Allow nfsd_vfs_write() to take the nfsd_file as an argument 2020-01-06 18:40 [PATCH 0/9] Fix error reporting for NFS writes Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 2/9] nfsd: Fix stable writes Trond Myklebust 2020-01-22 15:27 ` [PATCH 0/9] Fix error reporting for NFS writes J. Bruce Fields 1 sibling, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Needed in order to fix stable writes. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/nfs4proc.c | 2 +- fs/nfsd/vfs.c | 5 +++-- fs/nfsd/vfs.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 4798667af647..3d4e78118e53 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1012,7 +1012,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, &write->wr_head, write->wr_buflen); WARN_ON_ONCE(nvecs > ARRAY_SIZE(rqstp->rq_vec)); - status = nfsd_vfs_write(rqstp, &cstate->current_fh, nf->nf_file, + status = nfsd_vfs_write(rqstp, &cstate->current_fh, nf, write->wr_offset, rqstp->rq_vec, nvecs, &cnt, write->wr_how_written); nfsd_file_put(nf); diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 82cf80dde5c7..69cbdb62b262 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -947,10 +947,11 @@ static int wait_for_concurrent_writes(struct file *file) } __be32 -nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, +nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf, loff_t offset, struct kvec *vec, int vlen, unsigned long *cnt, int stable) { + struct file *file = nf->nf_file; struct svc_export *exp; struct iov_iter iter; __be32 nfserr; @@ -1057,7 +1058,7 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset, if (err) goto out; - err = nfsd_vfs_write(rqstp, fhp, nf->nf_file, offset, vec, + err = nfsd_vfs_write(rqstp, fhp, nf, offset, vec, vlen, cnt, stable); nfsd_file_put(nf); out: diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h index cc110a10bfe8..fd779c3bb35b 100644 --- a/fs/nfsd/vfs.h +++ b/fs/nfsd/vfs.h @@ -34,6 +34,8 @@ #define NFSD_MAY_CREATE (NFSD_MAY_EXEC|NFSD_MAY_WRITE) #define NFSD_MAY_REMOVE (NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC) +struct nfsd_file; + /* * Callback function for readdir */ @@ -93,7 +95,7 @@ __be32 nfsd_read(struct svc_rqst *, struct svc_fh *, __be32 nfsd_write(struct svc_rqst *, struct svc_fh *, loff_t, struct kvec *, int, unsigned long *, int); __be32 nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, - struct file *file, loff_t offset, + struct nfsd_file *nf, loff_t offset, struct kvec *vec, int vlen, unsigned long *cnt, int stable); __be32 nfsd_readlink(struct svc_rqst *, struct svc_fh *, -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/9] nfsd: Fix stable writes 2020-01-06 18:40 ` [PATCH 1/9] nfsd: Allow nfsd_vfs_write() to take the nfsd_file as an argument Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 3/9] nfsd: Update the boot verifier on stable writes too Trond Myklebust 0 siblings, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Strictly speaking, a stable write error needs to reflect the write + the commit of that write (and only that write). To ensure that we don't pick up the write errors from other writebacks, add a rw_semaphore to provide exclusion. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/filecache.c | 1 + fs/nfsd/filecache.h | 1 + fs/nfsd/vfs.c | 18 ++++++++++++++++-- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c index f275c11c4e28..2fadf080ac42 100644 --- a/fs/nfsd/filecache.c +++ b/fs/nfsd/filecache.c @@ -195,6 +195,7 @@ nfsd_file_alloc(struct inode *inode, unsigned int may, unsigned int hashval, __set_bit(NFSD_FILE_BREAK_READ, &nf->nf_flags); } nf->nf_mark = NULL; + init_rwsem(&nf->nf_rwsem); trace_nfsd_file_alloc(nf); } return nf; diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h index 79a7d6808d97..986c325a54bd 100644 --- a/fs/nfsd/filecache.h +++ b/fs/nfsd/filecache.h @@ -46,6 +46,7 @@ struct nfsd_file { atomic_t nf_ref; unsigned char nf_may; struct nfsd_file_mark *nf_mark; + struct rw_semaphore nf_rwsem; }; int nfsd_file_cache_init(void); diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 69cbdb62b262..218b8293c633 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -982,7 +982,18 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf, flags |= RWF_SYNC; iov_iter_kvec(&iter, WRITE, vec, vlen, *cnt); - host_err = vfs_iter_write(file, &iter, &pos, flags); + if (flags & RWF_SYNC) { + down_write(&nf->nf_rwsem); + host_err = vfs_iter_write(file, &iter, &pos, flags); + if (host_err < 0) + nfsd_reset_boot_verifier(net_generic(SVC_NET(rqstp), + nfsd_net_id)); + up_write(&nf->nf_rwsem); + } else { + down_read(&nf->nf_rwsem); + host_err = vfs_iter_write(file, &iter, &pos, flags); + up_read(&nf->nf_rwsem); + } if (host_err < 0) goto out_nfserr; *cnt = host_err; @@ -1097,8 +1108,10 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp, if (err) goto out; if (EX_ISSYNC(fhp->fh_export)) { - int err2 = vfs_fsync_range(nf->nf_file, offset, end, 0); + int err2; + down_write(&nf->nf_rwsem); + err2 = vfs_fsync_range(nf->nf_file, offset, end, 0); switch (err2) { case 0: break; @@ -1110,6 +1123,7 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp, nfsd_reset_boot_verifier(net_generic(nf->nf_net, nfsd_net_id)); } + up_write(&nf->nf_rwsem); } nfsd_file_put(nf); -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/9] nfsd: Update the boot verifier on stable writes too. 2020-01-06 18:40 ` [PATCH 2/9] nfsd: Fix stable writes Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 4/9] nfsd: Pass the nfsd_file as arguments to nfsd4_clone_file_range() Trond Myklebust 0 siblings, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs We don't know if the error returned by the fsync() call is exclusive to the data written by the stable write, so play it safe. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/vfs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 218b8293c633..38db4a083375 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -994,8 +994,11 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf, host_err = vfs_iter_write(file, &iter, &pos, flags); up_read(&nf->nf_rwsem); } - if (host_err < 0) + if (host_err < 0) { + nfsd_reset_boot_verifier(net_generic(SVC_NET(rqstp), + nfsd_net_id)); goto out_nfserr; + } *cnt = host_err; nfsdstats.io_write += *cnt; fsnotify_modify(file); -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/9] nfsd: Pass the nfsd_file as arguments to nfsd4_clone_file_range() 2020-01-06 18:40 ` [PATCH 3/9] nfsd: Update the boot verifier on stable writes too Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 5/9] nfsd: Ensure exclusion between CLONE and WRITE errors Trond Myklebust 0 siblings, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Needed in order to fix exclusion w.r.t. writes. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/nfs4proc.c | 4 ++-- fs/nfsd/vfs.c | 6 ++++-- fs/nfsd/vfs.h | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 3d4e78118e53..7fce319e5b85 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1076,8 +1076,8 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, if (status) goto out; - status = nfsd4_clone_file_range(src->nf_file, clone->cl_src_pos, - dst->nf_file, clone->cl_dst_pos, clone->cl_count, + status = nfsd4_clone_file_range(src, clone->cl_src_pos, + dst, clone->cl_dst_pos, clone->cl_count, EX_ISSYNC(cstate->current_fh.fh_export)); nfsd_file_put(dst); diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 38db4a083375..7e7e31dfc672 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -530,9 +530,11 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp, } #endif -__be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst, - u64 dst_pos, u64 count, bool sync) +__be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, + struct nfsd_file *nf_dst, u64 dst_pos, u64 count, bool sync) { + struct file *src = nf_src->nf_file; + struct file *dst = nf_dst->nf_file; loff_t cloned; cloned = vfs_clone_file_range(src, src_pos, dst, dst_pos, count, 0); diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h index fd779c3bb35b..69d23e9926cf 100644 --- a/fs/nfsd/vfs.h +++ b/fs/nfsd/vfs.h @@ -57,8 +57,9 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *, struct xdr_netobj *); __be32 nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *, struct file *, loff_t, loff_t, int); -__be32 nfsd4_clone_file_range(struct file *, u64, struct file *, - u64, u64, bool); +__be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, + struct nfsd_file *nf_dst, u64 dst_pos, + u64 count, bool sync); #endif /* CONFIG_NFSD_V4 */ __be32 nfsd_create_locked(struct svc_rqst *, struct svc_fh *, char *name, int len, struct iattr *attrs, -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/9] nfsd: Ensure exclusion between CLONE and WRITE errors 2020-01-06 18:40 ` [PATCH 4/9] nfsd: Pass the nfsd_file as arguments to nfsd4_clone_file_range() Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 6/9] sunrpc: Fix potential leaks in sunrpc_cache_unhash() Trond Myklebust 0 siblings, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Ensure that we can distinguish between synchronous CLONE and WRITE errors, and that we can assign them correctly. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/vfs.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 7e7e31dfc672..b2984a996ab8 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -536,22 +536,33 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, struct file *src = nf_src->nf_file; struct file *dst = nf_dst->nf_file; loff_t cloned; + __be32 ret = 0; + down_write(&nf_dst->nf_rwsem); cloned = vfs_clone_file_range(src, src_pos, dst, dst_pos, count, 0); - if (cloned < 0) - return nfserrno(cloned); - if (count && cloned != count) - return nfserrno(-EINVAL); + if (cloned < 0) { + ret = nfserrno(cloned); + goto out_err; + } + if (count && cloned != count) { + ret = nfserrno(-EINVAL); + goto out_err; + } if (sync) { loff_t dst_end = count ? dst_pos + count - 1 : LLONG_MAX; int status = vfs_fsync_range(dst, dst_pos, dst_end, 0); if (!status) status = commit_inode_metadata(file_inode(src)); - if (status < 0) - return nfserrno(status); + if (status < 0) { + nfsd_reset_boot_verifier(net_generic(nf_dst->nf_net, + nfsd_net_id)); + ret = nfserrno(status); + } } - return 0; +out_err: + up_write(&nf_dst->nf_rwsem); + return ret; } ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/9] sunrpc: Fix potential leaks in sunrpc_cache_unhash() 2020-01-06 18:40 ` [PATCH 5/9] nfsd: Ensure exclusion between CLONE and WRITE errors Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 7/9] sunrpc: clean up cache entry add/remove from hashtable Trond Myklebust 0 siblings, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs When we unhash the cache entry, we need to handle any pending upcalls by calling cache_fresh_unlocked(). Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- net/sunrpc/cache.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index f740cb51802a..7ede1e52fd81 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -1888,7 +1888,9 @@ void sunrpc_cache_unhash(struct cache_detail *cd, struct cache_head *h) if (!hlist_unhashed(&h->cache_list)){ hlist_del_init_rcu(&h->cache_list); cd->entries--; + set_bit(CACHE_CLEANED, &h->flags); spin_unlock(&cd->hash_lock); + cache_fresh_unlocked(h, cd); cache_put(h, cd); } else spin_unlock(&cd->hash_lock); -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 7/9] sunrpc: clean up cache entry add/remove from hashtable 2020-01-06 18:40 ` [PATCH 6/9] sunrpc: Fix potential leaks in sunrpc_cache_unhash() Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 8/9] nfsd: Ensure sampling of the commit verifier is atomic with the commit Trond Myklebust 0 siblings, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- net/sunrpc/cache.c | 50 ++++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 7ede1e52fd81..52d927210d32 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -77,6 +77,22 @@ static struct cache_head *sunrpc_cache_find_rcu(struct cache_detail *detail, return NULL; } +static void sunrpc_begin_cache_remove_entry(struct cache_head *ch, + struct cache_detail *cd) +{ + /* Must be called under cd->hash_lock */ + hlist_del_init_rcu(&ch->cache_list); + set_bit(CACHE_CLEANED, &ch->flags); + cd->entries --; +} + +static void sunrpc_end_cache_remove_entry(struct cache_head *ch, + struct cache_detail *cd) +{ + cache_fresh_unlocked(ch, cd); + cache_put(ch, cd); +} + static struct cache_head *sunrpc_cache_add_entry(struct cache_detail *detail, struct cache_head *key, int hash) @@ -100,8 +116,7 @@ static struct cache_head *sunrpc_cache_add_entry(struct cache_detail *detail, hlist_for_each_entry_rcu(tmp, head, cache_list) { if (detail->match(tmp, key)) { if (cache_is_expired(detail, tmp)) { - hlist_del_init_rcu(&tmp->cache_list); - detail->entries --; + sunrpc_begin_cache_remove_entry(tmp, detail); freeme = tmp; break; } @@ -117,10 +132,8 @@ static struct cache_head *sunrpc_cache_add_entry(struct cache_detail *detail, cache_get(new); spin_unlock(&detail->hash_lock); - if (freeme) { - cache_fresh_unlocked(freeme, detail); - cache_put(freeme, detail); - } + if (freeme) + sunrpc_end_cache_remove_entry(freeme, detail); return new; } @@ -454,8 +467,7 @@ static int cache_clean(void) if (!cache_is_expired(current_detail, ch)) continue; - hlist_del_init_rcu(&ch->cache_list); - current_detail->entries--; + sunrpc_begin_cache_remove_entry(ch, current_detail); rv = 1; break; } @@ -465,11 +477,8 @@ static int cache_clean(void) if (!ch) current_index ++; spin_unlock(&cache_list_lock); - if (ch) { - set_bit(CACHE_CLEANED, &ch->flags); - cache_fresh_unlocked(ch, d); - cache_put(ch, d); - } + if (ch) + sunrpc_end_cache_remove_entry(ch, d); } else spin_unlock(&cache_list_lock); @@ -525,13 +534,9 @@ void cache_purge(struct cache_detail *detail) for (i = 0; i < detail->hash_size; i++) { head = &detail->hash_table[i]; hlist_for_each_entry_safe(ch, tmp, head, cache_list) { - hlist_del_init_rcu(&ch->cache_list); - detail->entries--; - - set_bit(CACHE_CLEANED, &ch->flags); + sunrpc_begin_cache_remove_entry(ch, detail); spin_unlock(&detail->hash_lock); - cache_fresh_unlocked(ch, detail); - cache_put(ch, detail); + sunrpc_end_cache_remove_entry(ch, detail); spin_lock(&detail->hash_lock); } } @@ -1886,12 +1891,9 @@ void sunrpc_cache_unhash(struct cache_detail *cd, struct cache_head *h) { spin_lock(&cd->hash_lock); if (!hlist_unhashed(&h->cache_list)){ - hlist_del_init_rcu(&h->cache_list); - cd->entries--; - set_bit(CACHE_CLEANED, &h->flags); + sunrpc_begin_cache_remove_entry(h, cd); spin_unlock(&cd->hash_lock); - cache_fresh_unlocked(h, cd); - cache_put(h, cd); + sunrpc_end_cache_remove_entry(h, cd); } else spin_unlock(&cd->hash_lock); } -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 8/9] nfsd: Ensure sampling of the commit verifier is atomic with the commit 2020-01-06 18:40 ` [PATCH 7/9] sunrpc: clean up cache entry add/remove from hashtable Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 2020-01-06 18:40 ` [PATCH 9/9] nfsd: Ensure sampling of the write verifier is atomic with the write Trond Myklebust 0 siblings, 1 reply; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs When we have a successful commit, ensure we sample the commit verifier before releasing the lock. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/nfs3proc.c | 3 ++- fs/nfsd/nfs3xdr.c | 8 ++------ fs/nfsd/nfs4proc.c | 4 ++-- fs/nfsd/vfs.c | 8 ++++++-- fs/nfsd/vfs.h | 2 +- fs/nfsd/xdr3.h | 1 + 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index cea68d8411ac..ffdc592868a6 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -683,7 +683,8 @@ nfsd3_proc_commit(struct svc_rqst *rqstp) RETURN_STATUS(nfserr_inval); fh_copy(&resp->fh, &argp->fh); - nfserr = nfsd_commit(rqstp, &resp->fh, argp->offset, argp->count); + nfserr = nfsd_commit(rqstp, &resp->fh, argp->offset, argp->count, + resp->verf); RETURN_STATUS(nfserr); } diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c index 195ab7a0fc89..4aaa85f42da2 100644 --- a/fs/nfsd/nfs3xdr.c +++ b/fs/nfsd/nfs3xdr.c @@ -1125,16 +1125,12 @@ int nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p) { struct nfsd3_commitres *resp = rqstp->rq_resp; - struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id); - __be32 verf[2]; p = encode_wcc_data(rqstp, p, &resp->fh); /* Write verifier */ if (resp->status == 0) { - /* unique identifier, y2038 overflow can be ignored */ - nfsd_copy_boot_verifier(verf, nn); - *p++ = verf[0]; - *p++ = verf[1]; + *p++ = resp->verf[0]; + *p++ = resp->verf[1]; } return xdr_ressize_check(rqstp, p); } diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 7fce319e5b85..c24a4f96c973 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -581,9 +581,9 @@ nfsd4_commit(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, { struct nfsd4_commit *commit = &u->commit; - gen_boot_verifier(&commit->co_verf, SVC_NET(rqstp)); return nfsd_commit(rqstp, &cstate->current_fh, commit->co_offset, - commit->co_count); + commit->co_count, + (__be32 *)commit->co_verf.data); } static __be32 diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index b2984a996ab8..1d69a1e78b03 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1105,7 +1105,7 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset, */ __be32 nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp, - loff_t offset, unsigned long count) + loff_t offset, unsigned long count, __be32 *verf) { struct nfsd_file *nf; loff_t end = LLONG_MAX; @@ -1130,6 +1130,8 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp, err2 = vfs_fsync_range(nf->nf_file, offset, end, 0); switch (err2) { case 0: + nfsd_copy_boot_verifier(verf, net_generic(nf->nf_net, + nfsd_net_id)); break; case -EINVAL: err = nfserr_notsupp; @@ -1140,7 +1142,9 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp, nfsd_net_id)); } up_write(&nf->nf_rwsem); - } + } else + nfsd_copy_boot_verifier(verf, net_generic(nf->nf_net, + nfsd_net_id)); nfsd_file_put(nf); out: diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h index 69d23e9926cf..29e30688ccfb 100644 --- a/fs/nfsd/vfs.h +++ b/fs/nfsd/vfs.h @@ -74,7 +74,7 @@ __be32 do_nfsd_create(struct svc_rqst *, struct svc_fh *, struct svc_fh *res, int createmode, u32 *verifier, bool *truncp, bool *created); __be32 nfsd_commit(struct svc_rqst *, struct svc_fh *, - loff_t, unsigned long); + loff_t, unsigned long, __be32 *verf); #endif /* CONFIG_NFSD_V3 */ int nfsd_open_break_lease(struct inode *, int); __be32 nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t, diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h index 99ff9f403ff1..21fc1f14bcad 100644 --- a/fs/nfsd/xdr3.h +++ b/fs/nfsd/xdr3.h @@ -223,6 +223,7 @@ struct nfsd3_pathconfres { struct nfsd3_commitres { __be32 status; struct svc_fh fh; + __be32 verf[2]; }; struct nfsd3_getaclres { -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 9/9] nfsd: Ensure sampling of the write verifier is atomic with the write 2020-01-06 18:40 ` [PATCH 8/9] nfsd: Ensure sampling of the commit verifier is atomic with the commit Trond Myklebust @ 2020-01-06 18:40 ` Trond Myklebust 0 siblings, 0 replies; 12+ messages in thread From: Trond Myklebust @ 2020-01-06 18:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs When doing an unstable write, we need to ensure that we sample the write verifier before releasing the lock, and allowing a commit to the same file to proceed. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/nfs3proc.c | 2 +- fs/nfsd/nfs3xdr.c | 8 ++------ fs/nfsd/nfs4proc.c | 4 ++-- fs/nfsd/nfsproc.c | 2 +- fs/nfsd/vfs.c | 12 +++++++++--- fs/nfsd/vfs.h | 5 +++-- fs/nfsd/xdr3.h | 1 + 7 files changed, 19 insertions(+), 15 deletions(-) diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index ffdc592868a6..288bc76b4574 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -203,7 +203,7 @@ nfsd3_proc_write(struct svc_rqst *rqstp) RETURN_STATUS(nfserr_io); nfserr = nfsd_write(rqstp, &resp->fh, argp->offset, rqstp->rq_vec, nvecs, &cnt, - resp->committed); + resp->committed, resp->verf); resp->count = cnt; RETURN_STATUS(nfserr); } diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c index 4aaa85f42da2..e1f6c65049b3 100644 --- a/fs/nfsd/nfs3xdr.c +++ b/fs/nfsd/nfs3xdr.c @@ -751,17 +751,13 @@ int nfs3svc_encode_writeres(struct svc_rqst *rqstp, __be32 *p) { struct nfsd3_writeres *resp = rqstp->rq_resp; - struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id); - __be32 verf[2]; p = encode_wcc_data(rqstp, p, &resp->fh); if (resp->status == 0) { *p++ = htonl(resp->count); *p++ = htonl(resp->committed); - /* unique identifier, y2038 overflow can be ignored */ - nfsd_copy_boot_verifier(verf, nn); - *p++ = verf[0]; - *p++ = verf[1]; + *p++ = resp->verf[0]; + *p++ = resp->verf[1]; } return xdr_ressize_check(rqstp, p); } diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index c24a4f96c973..af2cb7dcb74c 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1006,7 +1006,6 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, } write->wr_how_written = write->wr_stable_how; - gen_boot_verifier(&write->wr_verifier, SVC_NET(rqstp)); nvecs = svc_fill_write_vector(rqstp, write->wr_pagelist, &write->wr_head, write->wr_buflen); @@ -1014,7 +1013,8 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, status = nfsd_vfs_write(rqstp, &cstate->current_fh, nf, write->wr_offset, rqstp->rq_vec, nvecs, &cnt, - write->wr_how_written); + write->wr_how_written, + (__be32 *)write->wr_verifier.data); nfsd_file_put(nf); write->wr_bytes_written = cnt; diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c index c83ddac22f38..bb1fbb67f697 100644 --- a/fs/nfsd/nfsproc.c +++ b/fs/nfsd/nfsproc.c @@ -226,7 +226,7 @@ nfsd_proc_write(struct svc_rqst *rqstp) return nfserr_io; nfserr = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh), argp->offset, rqstp->rq_vec, nvecs, - &cnt, NFS_DATA_SYNC); + &cnt, NFS_DATA_SYNC, NULL); return nfsd_return_attrs(nfserr, resp); } diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 1d69a1e78b03..8beed0fbf93a 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -962,7 +962,8 @@ static int wait_for_concurrent_writes(struct file *file) __be32 nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf, loff_t offset, struct kvec *vec, int vlen, - unsigned long *cnt, int stable) + unsigned long *cnt, int stable, + __be32 *verf) { struct file *file = nf->nf_file; struct svc_export *exp; @@ -1004,6 +1005,10 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf, up_write(&nf->nf_rwsem); } else { down_read(&nf->nf_rwsem); + if (verf) + nfsd_copy_boot_verifier(verf, + net_generic(SVC_NET(rqstp), + nfsd_net_id)); host_err = vfs_iter_write(file, &iter, &pos, flags); up_read(&nf->nf_rwsem); } @@ -1074,7 +1079,8 @@ __be32 nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp, */ __be32 nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset, - struct kvec *vec, int vlen, unsigned long *cnt, int stable) + struct kvec *vec, int vlen, unsigned long *cnt, int stable, + __be32 *verf) { struct nfsd_file *nf; __be32 err; @@ -1086,7 +1092,7 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset, goto out; err = nfsd_vfs_write(rqstp, fhp, nf, offset, vec, - vlen, cnt, stable); + vlen, cnt, stable, verf); nfsd_file_put(nf); out: trace_nfsd_write_done(rqstp, fhp, offset, *cnt); diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h index 29e30688ccfb..11a112617e91 100644 --- a/fs/nfsd/vfs.h +++ b/fs/nfsd/vfs.h @@ -94,11 +94,12 @@ __be32 nfsd_read(struct svc_rqst *, struct svc_fh *, loff_t, struct kvec *, int, unsigned long *, u32 *eof); __be32 nfsd_write(struct svc_rqst *, struct svc_fh *, loff_t, - struct kvec *, int, unsigned long *, int); + struct kvec *, int, unsigned long *, + int stable, __be32 *verf); __be32 nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf, loff_t offset, struct kvec *vec, int vlen, unsigned long *cnt, - int stable); + int stable, __be32 *verf); __be32 nfsd_readlink(struct svc_rqst *, struct svc_fh *, char *, int *); __be32 nfsd_symlink(struct svc_rqst *, struct svc_fh *, diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h index 21fc1f14bcad..fed959a809e8 100644 --- a/fs/nfsd/xdr3.h +++ b/fs/nfsd/xdr3.h @@ -159,6 +159,7 @@ struct nfsd3_writeres { struct svc_fh fh; unsigned long count; int committed; + __be32 verf[2]; }; struct nfsd3_renameres { -- 2.24.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/9] Fix error reporting for NFS writes 2020-01-06 18:40 [PATCH 0/9] Fix error reporting for NFS writes Trond Myklebust 2020-01-06 18:40 ` [PATCH 1/9] nfsd: Allow nfsd_vfs_write() to take the nfsd_file as an argument Trond Myklebust @ 2020-01-22 15:27 ` J. Bruce Fields 2020-01-22 17:04 ` J. Bruce Fields 1 sibling, 1 reply; 12+ messages in thread From: J. Bruce Fields @ 2020-01-22 15:27 UTC (permalink / raw) To: Trond Myklebust; +Cc: J. Bruce Fields, linux-nfs By the way, anyone know how to handle quoted-printable patches? For some reason, git-am seems to deal with them, but git-apply doesn't. So it's fine until there's some minor conflict. --b. On Mon, Jan 06, 2020 at 01:40:28PM -0500, Trond Myklebust wrote: > In cases where we have transient errors, such as ENOSPC, it is important > to ensure that errors are reported on all writes that may be affected. > > The problem we have is that not all errors are guaranteed to be reported > at write time. Some are reported only when we call fsync(). In > particular, this can be a problem for stable NFS writes. Since most > filesystems protect the write to the page cache with the inode lock, > but do not protect the subsequent call to generic_write_sync(), this > means that if we have parallel writes to the same file, we can end up > assigning the error to the wrong stable write call. If the application > expects to be able to fix the transient errors, it may end up replaying > the wrong write. One area where we have seen this happen is in flexfiles > writes, where the server is capable of freeing up space on the DS in > case of ENOSPC. > > The other area where we have seen a similar problem is when we have > unstable writes, and the client sends a backgrounded commit in order > to free up memory. If there are outstanding writes while the commit > gets a transient error and bumps the write verifier, then we want to > ensure that those writes get the approprite write verifier depending > on whether they were affected by the fsync() or not. Right now, > because the NFSv3 verifier is set in the XDR encoder well after the > write is done, there is fairly large window for a race with a > background commit. > > This patch series deals with both issues by adding per-file-descriptor > locking that ensures that writes, fsync error handling, and write verifier > updates are appropriately serialised. > > Trond Myklebust (9): > nfsd: Allow nfsd_vfs_write() to take the nfsd_file as an argument > nfsd: Fix stable writes > nfsd: Update the boot verifier on stable writes too. > nfsd: Pass the nfsd_file as arguments to nfsd4_clone_file_range() > nfsd: Ensure exclusion between CLONE and WRITE errors > sunrpc: Fix potential leaks in sunrpc_cache_unhash() > sunrpc: clean up cache entry add/remove from hashtable > nfsd: Ensure sampling of the commit verifier is atomic with the commit > nfsd: Ensure sampling of the write verifier is atomic with the write > > fs/nfsd/filecache.c | 1 + > fs/nfsd/filecache.h | 1 + > fs/nfsd/nfs3proc.c | 5 +-- > fs/nfsd/nfs3xdr.c | 16 +++------ > fs/nfsd/nfs4proc.c | 14 ++++---- > fs/nfsd/nfsproc.c | 2 +- > fs/nfsd/vfs.c | 79 ++++++++++++++++++++++++++++++++++----------- > fs/nfsd/vfs.h | 16 +++++---- > fs/nfsd/xdr3.h | 2 ++ > net/sunrpc/cache.c | 48 ++++++++++++++------------- > 10 files changed, 115 insertions(+), 69 deletions(-) > > -- > 2.24.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/9] Fix error reporting for NFS writes 2020-01-22 15:27 ` [PATCH 0/9] Fix error reporting for NFS writes J. Bruce Fields @ 2020-01-22 17:04 ` J. Bruce Fields 0 siblings, 0 replies; 12+ messages in thread From: J. Bruce Fields @ 2020-01-22 17:04 UTC (permalink / raw) To: Trond Myklebust; +Cc: J. Bruce Fields, linux-nfs On Wed, Jan 22, 2020 at 10:27:51AM -0500, bfields wrote: > By the way, anyone know how to handle quoted-printable patches? > > For some reason, git-am seems to deal with them, but git-apply doesn't. > So it's fine until there's some minor conflict. (Ended up running them through "tcucodec quote -d". Maybe there's some more obvious solution.) --b. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-01-22 17:04 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-06 18:40 [PATCH 0/9] Fix error reporting for NFS writes Trond Myklebust 2020-01-06 18:40 ` [PATCH 1/9] nfsd: Allow nfsd_vfs_write() to take the nfsd_file as an argument Trond Myklebust 2020-01-06 18:40 ` [PATCH 2/9] nfsd: Fix stable writes Trond Myklebust 2020-01-06 18:40 ` [PATCH 3/9] nfsd: Update the boot verifier on stable writes too Trond Myklebust 2020-01-06 18:40 ` [PATCH 4/9] nfsd: Pass the nfsd_file as arguments to nfsd4_clone_file_range() Trond Myklebust 2020-01-06 18:40 ` [PATCH 5/9] nfsd: Ensure exclusion between CLONE and WRITE errors Trond Myklebust 2020-01-06 18:40 ` [PATCH 6/9] sunrpc: Fix potential leaks in sunrpc_cache_unhash() Trond Myklebust 2020-01-06 18:40 ` [PATCH 7/9] sunrpc: clean up cache entry add/remove from hashtable Trond Myklebust 2020-01-06 18:40 ` [PATCH 8/9] nfsd: Ensure sampling of the commit verifier is atomic with the commit Trond Myklebust 2020-01-06 18:40 ` [PATCH 9/9] nfsd: Ensure sampling of the write verifier is atomic with the write Trond Myklebust 2020-01-22 15:27 ` [PATCH 0/9] Fix error reporting for NFS writes J. Bruce Fields 2020-01-22 17:04 ` J. Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).