* [PATCH 0/9] NFS client attribute cache performance improvements @ 2016-12-17 18:27 Trond Myklebust 2016-12-17 18:27 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs The following patchset makes the largest difference w.r.t. NFSv4, where we don't always ask for the full set of attributes from the server, however there are also some tidbits for NFSv3. With this patchset, running xfstests over NFSv4.1 shows the following improvement in execution times: Generic/074 now 264s, was 344s Generic/075 now 15s was 26s Generic/089 now 178s was 236s Generic/112 now 14s was 25s Generic/113 now 14s was 20s Generic/127 now 206s was 223s Generic/133 now 85s was 118s Generic/247 now 27s was 43s I saw no statistically significant performance regressions (and no functional regressions). For nfsstats, the client shows a significant decrease in % GETATTR and % LOOKUP calls on the same workloads. For 2 runs of xfstests, the numbers are: GETATTR: now 1456511, was 2168740 LOOKUP: now 301073, was 545571 ACCESS: now 137211 was 139447 Trond Myklebust (9): NFSv4: Don't invalidate the directory twice NFSv4: Update the attribute cache info in update_changeattr NFSv4: Don't discard the attributes returned by asynchronous DELEGRETURN NFS: Don't revalidate the file on close if we hold a delegation NFS: Clean up cache validity checking NFS: Only look at the change attribute cache state in nfs_weak_revalidate() NFS: Fix and clean up the access cache validity checking NFS: Remove unused function nfs_revalidate_inode_rcu() NFS: Clean up nfs_attribute_timeout() fs/nfs/dir.c | 23 +++++++++-------- fs/nfs/file.c | 12 +-------- fs/nfs/inode.c | 68 ++++++++++++++++++++++++++++++-------------------- fs/nfs/internal.h | 1 + fs/nfs/nfs4proc.c | 17 +++++++++---- include/linux/nfs_fs.h | 2 -- 6 files changed, 67 insertions(+), 56 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/9] NFSv4: Don't invalidate the directory twice 2016-12-17 18:27 [PATCH 0/9] NFS client attribute cache performance improvements Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 2/9] NFSv4: Update the attribute cache info in update_changeattr Trond Myklebust 2016-12-19 16:54 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Anna Schumaker 0 siblings, 2 replies; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs If an operation that modified the directory raced with a GETATTR, then we don't need to invalidate the directory cache more than once. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index d33242c8d95d..713932440e07 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1088,12 +1088,15 @@ static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo) struct nfs_inode *nfsi = NFS_I(dir); spin_lock(&dir->i_lock); + if (dir->i_version == cinfo->after) + goto out; nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; if (!cinfo->atomic || cinfo->before != dir->i_version) nfs_force_lookup_revalidate(dir); dir->i_version = cinfo->after; nfsi->attr_gencount = nfs_inc_attr_generation_counter(); nfs_fscache_invalidate(dir); +out: spin_unlock(&dir->i_lock); } -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/9] NFSv4: Update the attribute cache info in update_changeattr 2016-12-17 18:27 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 3/9] NFSv4: Don't discard the attributes returned by asynchronous DELEGRETURN Trond Myklebust 2016-12-19 16:54 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Anna Schumaker 1 sibling, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs If we successfully updated the change attribute, we should timestamp the cache. While we do know that the other attributes are not completely up to date, we have the NFS_INO_INVALID_ATTR flag that let us know that, so it is valid to say that the cache has not timed out. We can also clear NFS_INO_REVAL_PAGECACHE, since our change attribute is now known to be valid. Conversely, if the change attribute did not match, we should make sure to also revalidate the access and ACL caches. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 713932440e07..2e0816cb7136 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1091,8 +1091,15 @@ static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo) if (dir->i_version == cinfo->after) goto out; nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; - if (!cinfo->atomic || cinfo->before != dir->i_version) + if (cinfo->atomic && cinfo->before == dir->i_version) { + nfsi->cache_validity &= ~NFS_INO_REVAL_PAGECACHE; + nfsi->attrtimeo_timestamp = jiffies; + } else { nfs_force_lookup_revalidate(dir); + if (cinfo->before != dir->i_version) + nfsi->cache_validity |= NFS_INO_INVALID_ACCESS | + NFS_INO_INVALID_ACL; + } dir->i_version = cinfo->after; nfsi->attr_gencount = nfs_inc_attr_generation_counter(); nfs_fscache_invalidate(dir); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/9] NFSv4: Don't discard the attributes returned by asynchronous DELEGRETURN 2016-12-17 18:27 ` [PATCH 2/9] NFSv4: Update the attribute cache info in update_changeattr Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 4/9] NFS: Don't revalidate the file on close if we hold a delegation Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs DELEGRETURN will always carry a reference to the inode except when the latter is being freed, so let's ensure that we always use that inode information to ensure close-to-open cache consistency, even when the DELEGRETURN call is asynchronous. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 2e0816cb7136..58e3d5f627f2 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -5702,6 +5702,7 @@ static void nfs4_delegreturn_release(void *calldata) if (data->lr.roc) pnfs_roc_release(&data->lr.arg, &data->lr.res, data->res.lr_ret); + nfs_post_op_update_inode_force_wcc(inode, &data->fattr); nfs_iput_and_deactive(inode); } kfree(calldata); @@ -5790,10 +5791,6 @@ static int _nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, co if (status != 0) goto out; status = data->rpc_status; - if (status == 0) - nfs_post_op_update_inode_force_wcc(inode, &data->fattr); - else - nfs_refresh_inode(inode, &data->fattr); out: rpc_put_task(task); return status; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/9] NFS: Don't revalidate the file on close if we hold a delegation 2016-12-17 18:27 ` [PATCH 3/9] NFSv4: Don't discard the attributes returned by asynchronous DELEGRETURN Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 5/9] NFS: Clean up cache validity checking Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs If we're holding a delegation, we can skip sending the close-to-open GETATTR until we're returning that delegation. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/inode.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 7de345fd8e1e..2fc237cd338e 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -795,6 +795,8 @@ void nfs_close_context(struct nfs_open_context *ctx, int is_sync) if (!is_sync) return; inode = d_inode(ctx->dentry); + if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) + return; nfsi = NFS_I(inode); if (inode->i_mapping->nrpages == 0) return; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/9] NFS: Clean up cache validity checking 2016-12-17 18:27 ` [PATCH 4/9] NFS: Don't revalidate the file on close if we hold a delegation Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 6/9] NFS: Only look at the change attribute cache state in nfs_weak_revalidate() Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs Consolidate the open-coded checking of NFS_I(inode)->cache_validity into a couple of helper functions. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/file.c | 12 +----------- fs/nfs/inode.c | 43 ++++++++++++++++++++++++++++++++----------- fs/nfs/internal.h | 1 + 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/fs/nfs/file.c b/fs/nfs/file.c index 64c11f399b3d..599a602cb2fd 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -101,21 +101,11 @@ EXPORT_SYMBOL_GPL(nfs_file_release); static int nfs_revalidate_file_size(struct inode *inode, struct file *filp) { struct nfs_server *server = NFS_SERVER(inode); - struct nfs_inode *nfsi = NFS_I(inode); - const unsigned long force_reval = NFS_INO_REVAL_PAGECACHE|NFS_INO_REVAL_FORCED; - unsigned long cache_validity = nfsi->cache_validity; - - if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ) && - (cache_validity & force_reval) != force_reval) - goto out_noreval; if (filp->f_flags & O_DIRECT) goto force_reval; - if (nfsi->cache_validity & NFS_INO_REVAL_PAGECACHE) - goto force_reval; - if (nfs_attribute_timeout(inode)) + if (nfs_check_cache_invalid(inode, NFS_INO_REVAL_PAGECACHE)) goto force_reval; -out_noreval: return 0; force_reval: return __nfs_revalidate_inode(server, inode); diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 2fc237cd338e..e3194aa797f7 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -160,6 +160,36 @@ int nfs_sync_mapping(struct address_space *mapping) return ret; } +static bool nfs_check_cache_invalid_delegated(struct inode *inode, unsigned long flags) +{ + unsigned long cache_validity = READ_ONCE(NFS_I(inode)->cache_validity); + + /* Special case for the pagecache or access cache */ + if (flags == NFS_INO_REVAL_PAGECACHE && + !(cache_validity & NFS_INO_REVAL_FORCED)) + return false; + return (cache_validity & flags) != 0; +} + +static bool nfs_check_cache_invalid_not_delegated(struct inode *inode, unsigned long flags) +{ + unsigned long cache_validity = READ_ONCE(NFS_I(inode)->cache_validity); + + if ((cache_validity & flags) != 0) + return true; + if (nfs_attribute_timeout(inode)) + return true; + return false; +} + +bool nfs_check_cache_invalid(struct inode *inode, unsigned long flags) +{ + if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) + return nfs_check_cache_invalid_delegated(inode, flags); + + return nfs_check_cache_invalid_not_delegated(inode, flags); +} + static void nfs_set_cache_invalid(struct inode *inode, unsigned long flags) { struct nfs_inode *nfsi = NFS_I(inode); @@ -1116,17 +1146,8 @@ static int nfs_invalidate_mapping(struct inode *inode, struct address_space *map bool nfs_mapping_need_revalidate_inode(struct inode *inode) { - unsigned long cache_validity = NFS_I(inode)->cache_validity; - - if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) { - const unsigned long force_reval = - NFS_INO_REVAL_PAGECACHE|NFS_INO_REVAL_FORCED; - return (cache_validity & force_reval) == force_reval; - } - - return (cache_validity & NFS_INO_REVAL_PAGECACHE) - || nfs_attribute_timeout(inode) - || NFS_STALE(inode); + return nfs_check_cache_invalid(inode, NFS_INO_REVAL_PAGECACHE) || + NFS_STALE(inode); } int nfs_revalidate_mapping_rcu(struct inode *inode) diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h index 6b79c2ca9b9a..09ca5095c04e 100644 --- a/fs/nfs/internal.h +++ b/fs/nfs/internal.h @@ -381,6 +381,7 @@ extern int nfs_drop_inode(struct inode *); extern void nfs_clear_inode(struct inode *); extern void nfs_evict_inode(struct inode *); void nfs_zap_acl_cache(struct inode *inode); +extern bool nfs_check_cache_invalid(struct inode *, unsigned long); extern int nfs_wait_bit_killable(struct wait_bit_key *key, int mode); extern int nfs_wait_atomic_killable(atomic_t *p); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/9] NFS: Only look at the change attribute cache state in nfs_weak_revalidate() 2016-12-17 18:27 ` [PATCH 5/9] NFS: Clean up cache validity checking Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 7/9] NFS: Fix and clean up the access cache validity checking Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs Just like in nfs_check_verifier(), we want to use nfs_mapping_need_revalidate_inode() to check our knowledge of the change attribute is up to date. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/dir.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index cb22a9f9ae7e..8f706f3e5c05 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1273,8 +1273,8 @@ static int nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags) */ static int nfs_weak_revalidate(struct dentry *dentry, unsigned int flags) { - int error; struct inode *inode = d_inode(dentry); + int error = 0; /* * I believe we can only get a negative dentry here in the case of a @@ -1293,7 +1293,8 @@ static int nfs_weak_revalidate(struct dentry *dentry, unsigned int flags) return 0; } - error = nfs_revalidate_inode(NFS_SERVER(inode), inode); + if (nfs_mapping_need_revalidate_inode(inode)) + error = __nfs_revalidate_inode(NFS_SERVER(inode), inode); dfprintk(LOOKUPCACHE, "NFS: %s: inode %lu is %s\n", __func__, inode->i_ino, error ? "invalid" : "valid"); return !error; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 7/9] NFS: Fix and clean up the access cache validity checking 2016-12-17 18:27 ` [PATCH 6/9] NFS: Only look at the change attribute cache state in nfs_weak_revalidate() Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 8/9] NFS: Remove unused function nfs_revalidate_inode_rcu() Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs The access cache needs to check whether or not the mode bits, ownership, or ACL has changed or the cache has timed out. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/dir.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 8f706f3e5c05..fad81041f5ab 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -2286,8 +2286,7 @@ static int nfs_access_get_cached(struct inode *inode, struct rpc_cred *cred, str if (cache == NULL) goto out; /* Found an entry, is our attribute cache valid? */ - if (!nfs_attribute_cache_expired(inode) && - !(nfsi->cache_validity & NFS_INO_INVALID_ATTR)) + if (!nfs_check_cache_invalid(inode, NFS_INO_INVALID_ACCESS)) break; err = -ECHILD; if (!may_block) @@ -2335,12 +2334,12 @@ static int nfs_access_get_cached_rcu(struct inode *inode, struct rpc_cred *cred, cache = NULL; if (cache == NULL) goto out; - err = nfs_revalidate_inode_rcu(NFS_SERVER(inode), inode); - if (err) + if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_ACCESS)) goto out; res->jiffies = cache->jiffies; res->cred = cache->cred; res->mask = cache->mask; + err = 0; out: rcu_read_unlock(); return err; @@ -2492,12 +2491,13 @@ EXPORT_SYMBOL_GPL(nfs_may_open); static int nfs_execute_ok(struct inode *inode, int mask) { struct nfs_server *server = NFS_SERVER(inode); - int ret; + int ret = 0; - if (mask & MAY_NOT_BLOCK) - ret = nfs_revalidate_inode_rcu(server, inode); - else - ret = nfs_revalidate_inode(server, inode); + if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_ACCESS)) { + if (mask & MAY_NOT_BLOCK) + return -ECHILD; + ret = __nfs_revalidate_inode(server, inode); + } if (ret == 0 && !execute_ok(inode)) ret = -EACCES; return ret; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 8/9] NFS: Remove unused function nfs_revalidate_inode_rcu() 2016-12-17 18:27 ` [PATCH 7/9] NFS: Fix and clean up the access cache validity checking Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 2016-12-17 18:27 ` [PATCH 9/9] NFS: Clean up nfs_attribute_timeout() Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/inode.c | 9 --------- include/linux/nfs_fs.h | 1 - 2 files changed, 10 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index e3194aa797f7..8224e96b5808 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1105,15 +1105,6 @@ int nfs_revalidate_inode(struct nfs_server *server, struct inode *inode) } EXPORT_SYMBOL_GPL(nfs_revalidate_inode); -int nfs_revalidate_inode_rcu(struct nfs_server *server, struct inode *inode) -{ - if (!(NFS_I(inode)->cache_validity & - (NFS_INO_INVALID_ATTR|NFS_INO_INVALID_LABEL)) - && !nfs_attribute_cache_expired(inode)) - return NFS_STALE(inode) ? -ESTALE : 0; - return -ECHILD; -} - static int nfs_invalidate_mapping(struct inode *inode, struct address_space *mapping) { struct nfs_inode *nfsi = NFS_I(inode); diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index cb631973839a..fe2e7810ae80 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -343,7 +343,6 @@ extern int nfs_open(struct inode *, struct file *); extern int nfs_attribute_timeout(struct inode *inode); extern int nfs_attribute_cache_expired(struct inode *inode); extern int nfs_revalidate_inode(struct nfs_server *server, struct inode *inode); -extern int nfs_revalidate_inode_rcu(struct nfs_server *server, struct inode *inode); extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *); extern bool nfs_mapping_need_revalidate_inode(struct inode *inode); extern int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 9/9] NFS: Clean up nfs_attribute_timeout() 2016-12-17 18:27 ` [PATCH 8/9] NFS: Remove unused function nfs_revalidate_inode_rcu() Trond Myklebust @ 2016-12-17 18:27 ` Trond Myklebust 0 siblings, 0 replies; 16+ messages in thread From: Trond Myklebust @ 2016-12-17 18:27 UTC (permalink / raw) To: linux-nfs It can be made static. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/inode.c | 14 +++++++------- include/linux/nfs_fs.h | 1 - 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 8224e96b5808..a0cd79646c1b 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -160,6 +160,13 @@ int nfs_sync_mapping(struct address_space *mapping) return ret; } +static int nfs_attribute_timeout(struct inode *inode) +{ + struct nfs_inode *nfsi = NFS_I(inode); + + return !time_in_range_open(jiffies, nfsi->read_cache_jiffies, nfsi->read_cache_jiffies + nfsi->attrtimeo); +} + static bool nfs_check_cache_invalid_delegated(struct inode *inode, unsigned long flags) { unsigned long cache_validity = READ_ONCE(NFS_I(inode)->cache_validity); @@ -1076,13 +1083,6 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode) return status; } -int nfs_attribute_timeout(struct inode *inode) -{ - struct nfs_inode *nfsi = NFS_I(inode); - - return !time_in_range_open(jiffies, nfsi->read_cache_jiffies, nfsi->read_cache_jiffies + nfsi->attrtimeo); -} - int nfs_attribute_cache_expired(struct inode *inode) { if (nfs_have_delegated_attributes(inode)) diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index fe2e7810ae80..f1da8c8dd473 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -340,7 +340,6 @@ extern void nfs_access_add_cache(struct inode *, struct nfs_access_entry *); extern void nfs_access_set_mask(struct nfs_access_entry *, u32); extern int nfs_permission(struct inode *, int); extern int nfs_open(struct inode *, struct file *); -extern int nfs_attribute_timeout(struct inode *inode); extern int nfs_attribute_cache_expired(struct inode *inode); extern int nfs_revalidate_inode(struct nfs_server *server, struct inode *inode); extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] NFSv4: Don't invalidate the directory twice 2016-12-17 18:27 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Trond Myklebust 2016-12-17 18:27 ` [PATCH 2/9] NFSv4: Update the attribute cache info in update_changeattr Trond Myklebust @ 2016-12-19 16:54 ` Anna Schumaker 2016-12-19 16:56 ` Trond Myklebust 1 sibling, 1 reply; 16+ messages in thread From: Anna Schumaker @ 2016-12-19 16:54 UTC (permalink / raw) To: Trond Myklebust, linux-nfs Hi Trond, On 12/17/2016 01:27 PM, Trond Myklebust wrote: > If an operation that modified the directory raced with a GETATTR, then we > don't need to invalidate the directory cache more than once. This patch causes cthon basic tests to fail with: ./test6: readdir ./test6: (/nfs/all) unlinked 'file.0' dir entry read pass 1 ./test6: (/nfs/all) Test failed with 1 errors basic tests failed Thanks, Anna > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/nfs4proc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index d33242c8d95d..713932440e07 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -1088,12 +1088,15 @@ static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo) > struct nfs_inode *nfsi = NFS_I(dir); > > spin_lock(&dir->i_lock); > + if (dir->i_version == cinfo->after) > + goto out; > nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; > if (!cinfo->atomic || cinfo->before != dir->i_version) > nfs_force_lookup_revalidate(dir); > dir->i_version = cinfo->after; > nfsi->attr_gencount = nfs_inc_attr_generation_counter(); > nfs_fscache_invalidate(dir); > +out: > spin_unlock(&dir->i_lock); > } > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] NFSv4: Don't invalidate the directory twice 2016-12-19 16:54 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Anna Schumaker @ 2016-12-19 16:56 ` Trond Myklebust 2016-12-19 17:00 ` Trond Myklebust 2016-12-19 17:07 ` Anna Schumaker 0 siblings, 2 replies; 16+ messages in thread From: Trond Myklebust @ 2016-12-19 16:56 UTC (permalink / raw) To: Anna Schumaker; +Cc: Linux NFS Mailing List V2hhdOKAmXMgdGhlIGZpbGVzeXN0ZW0geW914oCZcmUgdGVzdGluZyBvbj8NCg0KPiBPbiBEZWMg MTksIDIwMTYsIGF0IDExOjU0LCBBbm5hIFNjaHVtYWtlciA8c2NodW1ha2VyLmFubmFAZ21haWwu Y29tPiB3cm90ZToNCj4gDQo+IEhpIFRyb25kLA0KPiANCj4gT24gMTIvMTcvMjAxNiAwMToyNyBQ TSwgVHJvbmQgTXlrbGVidXN0IHdyb3RlOg0KPj4gSWYgYW4gb3BlcmF0aW9uIHRoYXQgbW9kaWZp ZWQgdGhlIGRpcmVjdG9yeSByYWNlZCB3aXRoIGEgR0VUQVRUUiwgdGhlbiB3ZQ0KPj4gZG9uJ3Qg bmVlZCB0byBpbnZhbGlkYXRlIHRoZSBkaXJlY3RvcnkgY2FjaGUgbW9yZSB0aGFuIG9uY2UuDQo+ IA0KPiBUaGlzIHBhdGNoIGNhdXNlcyBjdGhvbiBiYXNpYyB0ZXN0cyB0byBmYWlsIHdpdGg6DQo+ IA0KPiAuL3Rlc3Q2OiByZWFkZGlyDQo+ICAgICAgICAuL3Rlc3Q2OiAoL25mcy9hbGwpIHVubGlu a2VkICdmaWxlLjAnIGRpciBlbnRyeSByZWFkIHBhc3MgMQ0KPiAgICAgICAgLi90ZXN0NjogKC9u ZnMvYWxsKSBUZXN0IGZhaWxlZCB3aXRoIDEgZXJyb3JzDQo+IGJhc2ljIHRlc3RzIGZhaWxlZA0K PiANCj4gVGhhbmtzLA0KPiBBbm5hDQo+PiANCj4+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xl YnVzdCA8dHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbT4NCj4+IC0tLQ0KPj4gZnMvbmZz L25mczRwcm9jLmMgfCAzICsrKw0KPj4gMSBmaWxlIGNoYW5nZWQsIDMgaW5zZXJ0aW9ucygrKQ0K Pj4gDQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25mczRwcm9jLmMgYi9mcy9uZnMvbmZzNHByb2Mu Yw0KPj4gaW5kZXggZDMzMjQyYzhkOTVkLi43MTM5MzI0NDBlMDcgMTAwNjQ0DQo+PiAtLS0gYS9m cy9uZnMvbmZzNHByb2MuYw0KPj4gKysrIGIvZnMvbmZzL25mczRwcm9jLmMNCj4+IEBAIC0xMDg4 LDEyICsxMDg4LDE1IEBAIHN0YXRpYyB2b2lkIHVwZGF0ZV9jaGFuZ2VhdHRyKHN0cnVjdCBpbm9k ZSAqZGlyLCBzdHJ1Y3QgbmZzNF9jaGFuZ2VfaW5mbyAqY2luZm8pDQo+PiAJc3RydWN0IG5mc19p bm9kZSAqbmZzaSA9IE5GU19JKGRpcik7DQo+PiANCj4+IAlzcGluX2xvY2soJmRpci0+aV9sb2Nr KTsNCj4+ICsJaWYgKGRpci0+aV92ZXJzaW9uID09IGNpbmZvLT5hZnRlcikNCj4+ICsJCWdvdG8g b3V0Ow0KPj4gCW5mc2ktPmNhY2hlX3ZhbGlkaXR5IHw9IE5GU19JTk9fSU5WQUxJRF9BVFRSfE5G U19JTk9fSU5WQUxJRF9EQVRBOw0KPj4gCWlmICghY2luZm8tPmF0b21pYyB8fCBjaW5mby0+YmVm b3JlICE9IGRpci0+aV92ZXJzaW9uKQ0KPj4gCQluZnNfZm9yY2VfbG9va3VwX3JldmFsaWRhdGUo ZGlyKTsNCj4+IAlkaXItPmlfdmVyc2lvbiA9IGNpbmZvLT5hZnRlcjsNCj4+IAluZnNpLT5hdHRy X2dlbmNvdW50ID0gbmZzX2luY19hdHRyX2dlbmVyYXRpb25fY291bnRlcigpOw0KPj4gCW5mc19m c2NhY2hlX2ludmFsaWRhdGUoZGlyKTsNCj4+ICtvdXQ6DQo+PiAJc3Bpbl91bmxvY2soJmRpci0+ aV9sb2NrKTsNCj4+IH0NCj4+IA0KPj4gDQo+IA0KDQo= ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] NFSv4: Don't invalidate the directory twice 2016-12-19 16:56 ` Trond Myklebust @ 2016-12-19 17:00 ` Trond Myklebust 2016-12-19 17:07 ` Anna Schumaker 1 sibling, 0 replies; 16+ messages in thread From: Trond Myklebust @ 2016-12-19 17:00 UTC (permalink / raw) To: Anna Schumaker; +Cc: Linux NFS Mailing List DQo+IE9uIERlYyAxOSwgMjAxNiwgYXQgMTE6NTYsIFRyb25kIE15a2xlYnVzdCA8dHJvbmRteUBw cmltYXJ5ZGF0YS5jb20+IHdyb3RlOg0KPiANCj4gV2hhdOKAmXMgdGhlIGZpbGVzeXN0ZW0geW91 4oCZcmUgdGVzdGluZyBvbj8NCg0K4oCmYW5kIHdoYXQgaXMgdGhlIHRpbWVzdGFtcCByZXNvbHV0 aW9uIHlvdeKAmXJlIHNlZWluZyBvbiBpdD8NCg0KPiANCj4+IE9uIERlYyAxOSwgMjAxNiwgYXQg MTE6NTQsIEFubmEgU2NodW1ha2VyIDxzY2h1bWFrZXIuYW5uYUBnbWFpbC5jb20+IHdyb3RlOg0K Pj4gDQo+PiBIaSBUcm9uZCwNCj4+IA0KPj4gT24gMTIvMTcvMjAxNiAwMToyNyBQTSwgVHJvbmQg TXlrbGVidXN0IHdyb3RlOg0KPj4+IElmIGFuIG9wZXJhdGlvbiB0aGF0IG1vZGlmaWVkIHRoZSBk aXJlY3RvcnkgcmFjZWQgd2l0aCBhIEdFVEFUVFIsIHRoZW4gd2UNCj4+PiBkb24ndCBuZWVkIHRv IGludmFsaWRhdGUgdGhlIGRpcmVjdG9yeSBjYWNoZSBtb3JlIHRoYW4gb25jZS4NCj4+IA0KPj4g VGhpcyBwYXRjaCBjYXVzZXMgY3Rob24gYmFzaWMgdGVzdHMgdG8gZmFpbCB3aXRoOg0KPj4gDQo+ PiAuL3Rlc3Q2OiByZWFkZGlyDQo+PiAgICAgICAuL3Rlc3Q2OiAoL25mcy9hbGwpIHVubGlua2Vk ICdmaWxlLjAnIGRpciBlbnRyeSByZWFkIHBhc3MgMQ0KPj4gICAgICAgLi90ZXN0NjogKC9uZnMv YWxsKSBUZXN0IGZhaWxlZCB3aXRoIDEgZXJyb3JzDQo+PiBiYXNpYyB0ZXN0cyBmYWlsZWQNCj4+ IA0KPj4gVGhhbmtzLA0KPj4gQW5uYQ0KPj4+IA0KPj4+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15 a2xlYnVzdCA8dHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbT4NCj4+PiAtLS0NCj4+PiBm cy9uZnMvbmZzNHByb2MuYyB8IDMgKysrDQo+Pj4gMSBmaWxlIGNoYW5nZWQsIDMgaW5zZXJ0aW9u cygrKQ0KPj4+IA0KPj4+IGRpZmYgLS1naXQgYS9mcy9uZnMvbmZzNHByb2MuYyBiL2ZzL25mcy9u ZnM0cHJvYy5jDQo+Pj4gaW5kZXggZDMzMjQyYzhkOTVkLi43MTM5MzI0NDBlMDcgMTAwNjQ0DQo+ Pj4gLS0tIGEvZnMvbmZzL25mczRwcm9jLmMNCj4+PiArKysgYi9mcy9uZnMvbmZzNHByb2MuYw0K Pj4+IEBAIC0xMDg4LDEyICsxMDg4LDE1IEBAIHN0YXRpYyB2b2lkIHVwZGF0ZV9jaGFuZ2VhdHRy KHN0cnVjdCBpbm9kZSAqZGlyLCBzdHJ1Y3QgbmZzNF9jaGFuZ2VfaW5mbyAqY2luZm8pDQo+Pj4g CXN0cnVjdCBuZnNfaW5vZGUgKm5mc2kgPSBORlNfSShkaXIpOw0KPj4+IA0KPj4+IAlzcGluX2xv Y2soJmRpci0+aV9sb2NrKTsNCj4+PiArCWlmIChkaXItPmlfdmVyc2lvbiA9PSBjaW5mby0+YWZ0 ZXIpDQo+Pj4gKwkJZ290byBvdXQ7DQo+Pj4gCW5mc2ktPmNhY2hlX3ZhbGlkaXR5IHw9IE5GU19J Tk9fSU5WQUxJRF9BVFRSfE5GU19JTk9fSU5WQUxJRF9EQVRBOw0KPj4+IAlpZiAoIWNpbmZvLT5h dG9taWMgfHwgY2luZm8tPmJlZm9yZSAhPSBkaXItPmlfdmVyc2lvbikNCj4+PiAJCW5mc19mb3Jj ZV9sb29rdXBfcmV2YWxpZGF0ZShkaXIpOw0KPj4+IAlkaXItPmlfdmVyc2lvbiA9IGNpbmZvLT5h ZnRlcjsNCj4+PiAJbmZzaS0+YXR0cl9nZW5jb3VudCA9IG5mc19pbmNfYXR0cl9nZW5lcmF0aW9u X2NvdW50ZXIoKTsNCj4+PiAJbmZzX2ZzY2FjaGVfaW52YWxpZGF0ZShkaXIpOw0KPj4+ICtvdXQ6 DQo+Pj4gCXNwaW5fdW5sb2NrKCZkaXItPmlfbG9jayk7DQo+Pj4gfQ0KPj4+IA0KPj4+IA0KPj4g DQo+IA0KPiBO77+977+977+977+977+9cu+/ve+/vXnvv73vv73vv71i77+9WO+/ve+/vcendu+/ vV7vv70p3rp7Lm7vv70r77+977+977+977+9e++/ve+/ve+/vSLvv73vv71ebu+/vXLvv73vv73v v71677+9Gu+/ve+/vWjvv73vv73vv73vv70m77+977+9Hu+/vUfvv73vv73vv71o77+9Ayjvv73p mo7vv73domoi77+977+9Gu+/vRtt77+977+977+977+977+9eu+/vd6W77+977+977+9Zu+/ve+/ ve+/vWjvv73vv73vv71+77+9be+/vQ0KDQo= ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] NFSv4: Don't invalidate the directory twice 2016-12-19 16:56 ` Trond Myklebust 2016-12-19 17:00 ` Trond Myklebust @ 2016-12-19 17:07 ` Anna Schumaker 2016-12-19 17:30 ` Trond Myklebust 1 sibling, 1 reply; 16+ messages in thread From: Anna Schumaker @ 2016-12-19 17:07 UTC (permalink / raw) To: Trond Myklebust; +Cc: Linux NFS Mailing List On 12/19/2016 11:56 AM, Trond Myklebust wrote: > What’s the filesystem you’re testing on? I see this on xfs and ext4 but btrfs seems to work. What do you mean by timestamp resolution? I can't find the file in question in `ls`, but for another file `stat` tells me: File: file.10 Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: fe31h/65073d Inode: 1319795 Links: 1 Access: (0666/-rw-rw-rw-) Uid: ( 1000/ anna) Gid: ( 100/ users) Access: 2016-12-19 12:04:47.680033877 -0500 Modify: 2016-12-19 12:04:47.680033877 -0500 Change: 2016-12-19 12:04:47.680033877 -0500 I hope this helps! Anna > >> On Dec 19, 2016, at 11:54, Anna Schumaker <schumaker.anna@gmail.com> wrote: >> >> Hi Trond, >> >> On 12/17/2016 01:27 PM, Trond Myklebust wrote: >>> If an operation that modified the directory raced with a GETATTR, then we >>> don't need to invalidate the directory cache more than once. >> >> This patch causes cthon basic tests to fail with: >> >> ./test6: readdir >> ./test6: (/nfs/all) unlinked 'file.0' dir entry read pass 1 >> ./test6: (/nfs/all) Test failed with 1 errors >> basic tests failed >> >> Thanks, >> Anna >>> >>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >>> --- >>> fs/nfs/nfs4proc.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>> index d33242c8d95d..713932440e07 100644 >>> --- a/fs/nfs/nfs4proc.c >>> +++ b/fs/nfs/nfs4proc.c >>> @@ -1088,12 +1088,15 @@ static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo) >>> struct nfs_inode *nfsi = NFS_I(dir); >>> >>> spin_lock(&dir->i_lock); >>> + if (dir->i_version == cinfo->after) >>> + goto out; >>> nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; >>> if (!cinfo->atomic || cinfo->before != dir->i_version) >>> nfs_force_lookup_revalidate(dir); >>> dir->i_version = cinfo->after; >>> nfsi->attr_gencount = nfs_inc_attr_generation_counter(); >>> nfs_fscache_invalidate(dir); >>> +out: >>> spin_unlock(&dir->i_lock); >>> } >>> >>> >> > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] NFSv4: Don't invalidate the directory twice 2016-12-19 17:07 ` Anna Schumaker @ 2016-12-19 17:30 ` Trond Myklebust 2016-12-19 19:05 ` Anna Schumaker 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-12-19 17:30 UTC (permalink / raw) To: Anna Schumaker; +Cc: Linux NFS Mailing List DQo+IE9uIERlYyAxOSwgMjAxNiwgYXQgMTI6MDcsIEFubmEgU2NodW1ha2VyIDxzY2h1bWFrZXIu YW5uYUBnbWFpbC5jb20+IHdyb3RlOg0KPiANCj4gDQo+IA0KPiBPbiAxMi8xOS8yMDE2IDExOjU2 IEFNLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6DQo+PiBXaGF04oCZcyB0aGUgZmlsZXN5c3RlbSB5 b3XigJlyZSB0ZXN0aW5nIG9uPw0KPiANCj4gSSBzZWUgdGhpcyBvbiB4ZnMgYW5kIGV4dDQgYnV0 IGJ0cmZzIHNlZW1zIHRvIHdvcmsuICBXaGF0IGRvIHlvdSBtZWFuIGJ5IHRpbWVzdGFtcCByZXNv bHV0aW9uPyAgSSBjYW4ndCBmaW5kIHRoZSBmaWxlIGluIHF1ZXN0aW9uIGluIGBsc2AsIGJ1dCBm b3IgYW5vdGhlciBmaWxlIGBzdGF0YCB0ZWxscyBtZToNCj4gDQo+ICBGaWxlOiBmaWxlLjEwDQo+ ICBTaXplOiAwICAgICAgICAgICAgICAgQmxvY2tzOiAwICAgICAgICAgIElPIEJsb2NrOiA0MDk2 ICAgcmVndWxhciBlbXB0eSBmaWxlDQo+IERldmljZTogZmUzMWgvNjUwNzNkICAgIElub2RlOiAx MzE5Nzk1ICAgICBMaW5rczogMQ0KPiBBY2Nlc3M6ICgwNjY2Ly1ydy1ydy1ydy0pICBVaWQ6ICgg MTAwMC8gICAgYW5uYSkgICBHaWQ6ICggIDEwMC8gICB1c2VycykNCj4gQWNjZXNzOiAyMDE2LTEy LTE5IDEyOjA0OjQ3LjY4MDAzMzg3NyAtMDUwMA0KPiBNb2RpZnk6IDIwMTYtMTItMTkgMTI6MDQ6 NDcuNjgwMDMzODc3IC0wNTAwDQo+IENoYW5nZTogMjAxNi0xMi0xOSAxMjowNDo0Ny42ODAwMzM4 NzcgLTA1MDANCg0KSSBtZWFuIHRoYXQgdGhlIHRpbWVzdGFtcHMgYXBwZWFyIHRvIGJlIHVuYWJs ZSB0byBrZWVwIHVwIHdpdGggdGhlIHZvbHVtZSBvZiBjaGFuZ2VzIG9uIHRoZSBmaWxlc3lzdGVt LiBJZiB0aGUgY3Rob24gdGVzdCBpcyBmYWlsaW5nIGhlcmUsIHRoZW4gaXTigJlzIHByb2JhYmx5 IGJlY2F1c2UgdGhlIGJlZm9yZS9hZnRlciBjaGFuZ2UgYXR0cmlidXRlcyBhcmUgdGhlIHNhbWXi gKYgRG9lcyB3aXJlc2hhcmsgc2hvdyB0aGF0IHRoZSBiZWZvcmUvYWZ0ZXIgY2hhbmdlIGF0dHJp YnV0ZXMgb24gdGhpcyBkaXJlY3RvcnkgYXJlIGRpZmZlcmVudD8NCg0KPiANCj4gSSBob3BlIHRo aXMgaGVscHMhDQo+IEFubmENCj4+IA0KPj4+IE9uIERlYyAxOSwgMjAxNiwgYXQgMTE6NTQsIEFu bmEgU2NodW1ha2VyIDxzY2h1bWFrZXIuYW5uYUBnbWFpbC5jb20+IHdyb3RlOg0KPj4+IA0KPj4+ IEhpIFRyb25kLA0KPj4+IA0KPj4+IE9uIDEyLzE3LzIwMTYgMDE6MjcgUE0sIFRyb25kIE15a2xl YnVzdCB3cm90ZToNCj4+Pj4gSWYgYW4gb3BlcmF0aW9uIHRoYXQgbW9kaWZpZWQgdGhlIGRpcmVj dG9yeSByYWNlZCB3aXRoIGEgR0VUQVRUUiwgdGhlbiB3ZQ0KPj4+PiBkb24ndCBuZWVkIHRvIGlu dmFsaWRhdGUgdGhlIGRpcmVjdG9yeSBjYWNoZSBtb3JlIHRoYW4gb25jZS4NCj4+PiANCj4+PiBU aGlzIHBhdGNoIGNhdXNlcyBjdGhvbiBiYXNpYyB0ZXN0cyB0byBmYWlsIHdpdGg6DQo+Pj4gDQo+ Pj4gLi90ZXN0NjogcmVhZGRpcg0KPj4+ICAgICAgIC4vdGVzdDY6ICgvbmZzL2FsbCkgdW5saW5r ZWQgJ2ZpbGUuMCcgZGlyIGVudHJ5IHJlYWQgcGFzcyAxDQo+Pj4gICAgICAgLi90ZXN0NjogKC9u ZnMvYWxsKSBUZXN0IGZhaWxlZCB3aXRoIDEgZXJyb3JzDQo+Pj4gYmFzaWMgdGVzdHMgZmFpbGVk DQo+Pj4gDQo+Pj4gVGhhbmtzLA0KPj4+IEFubmENCj4+Pj4gDQo+Pj4+IFNpZ25lZC1vZmYtYnk6 IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbT4NCj4+Pj4g LS0tDQo+Pj4+IGZzL25mcy9uZnM0cHJvYy5jIHwgMyArKysNCj4+Pj4gMSBmaWxlIGNoYW5nZWQs IDMgaW5zZXJ0aW9ucygrKQ0KPj4+PiANCj4+Pj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0cHJv Yy5jIGIvZnMvbmZzL25mczRwcm9jLmMNCj4+Pj4gaW5kZXggZDMzMjQyYzhkOTVkLi43MTM5MzI0 NDBlMDcgMTAwNjQ0DQo+Pj4+IC0tLSBhL2ZzL25mcy9uZnM0cHJvYy5jDQo+Pj4+ICsrKyBiL2Zz L25mcy9uZnM0cHJvYy5jDQo+Pj4+IEBAIC0xMDg4LDEyICsxMDg4LDE1IEBAIHN0YXRpYyB2b2lk IHVwZGF0ZV9jaGFuZ2VhdHRyKHN0cnVjdCBpbm9kZSAqZGlyLCBzdHJ1Y3QgbmZzNF9jaGFuZ2Vf aW5mbyAqY2luZm8pDQo+Pj4+IAlzdHJ1Y3QgbmZzX2lub2RlICpuZnNpID0gTkZTX0koZGlyKTsN Cj4+Pj4gDQo+Pj4+IAlzcGluX2xvY2soJmRpci0+aV9sb2NrKTsNCj4+Pj4gKwlpZiAoZGlyLT5p X3ZlcnNpb24gPT0gY2luZm8tPmFmdGVyKQ0KPj4+PiArCQlnb3RvIG91dDsNCj4+Pj4gCW5mc2kt PmNhY2hlX3ZhbGlkaXR5IHw9IE5GU19JTk9fSU5WQUxJRF9BVFRSfE5GU19JTk9fSU5WQUxJRF9E QVRBOw0KPj4+PiAJaWYgKCFjaW5mby0+YXRvbWljIHx8IGNpbmZvLT5iZWZvcmUgIT0gZGlyLT5p X3ZlcnNpb24pDQo+Pj4+IAkJbmZzX2ZvcmNlX2xvb2t1cF9yZXZhbGlkYXRlKGRpcik7DQo+Pj4+ IAlkaXItPmlfdmVyc2lvbiA9IGNpbmZvLT5hZnRlcjsNCj4+Pj4gCW5mc2ktPmF0dHJfZ2VuY291 bnQgPSBuZnNfaW5jX2F0dHJfZ2VuZXJhdGlvbl9jb3VudGVyKCk7DQo+Pj4+IAluZnNfZnNjYWNo ZV9pbnZhbGlkYXRlKGRpcik7DQo+Pj4+ICtvdXQ6DQo+Pj4+IAlzcGluX3VubG9jaygmZGlyLT5p X2xvY2spOw0KPj4+PiB9DQo+Pj4+IA0KPj4+PiANCj4+PiANCj4+IA0KPiANCg0K ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] NFSv4: Don't invalidate the directory twice 2016-12-19 17:30 ` Trond Myklebust @ 2016-12-19 19:05 ` Anna Schumaker 0 siblings, 0 replies; 16+ messages in thread From: Anna Schumaker @ 2016-12-19 19:05 UTC (permalink / raw) To: Trond Myklebust; +Cc: Linux NFS Mailing List On 12/19/2016 12:30 PM, Trond Myklebust wrote: > >> On Dec 19, 2016, at 12:07, Anna Schumaker <schumaker.anna@gmail.com> wrote: >> >> >> >> On 12/19/2016 11:56 AM, Trond Myklebust wrote: >>> What’s the filesystem you’re testing on? >> >> I see this on xfs and ext4 but btrfs seems to work. What do you mean by timestamp resolution? I can't find the file in question in `ls`, but for another file `stat` tells me: >> >> File: file.10 >> Size: 0 Blocks: 0 IO Block: 4096 regular empty file >> Device: fe31h/65073d Inode: 1319795 Links: 1 >> Access: (0666/-rw-rw-rw-) Uid: ( 1000/ anna) Gid: ( 100/ users) >> Access: 2016-12-19 12:04:47.680033877 -0500 >> Modify: 2016-12-19 12:04:47.680033877 -0500 >> Change: 2016-12-19 12:04:47.680033877 -0500 > > I mean that the timestamps appear to be unable to keep up with the volume of changes on the filesystem. If the cthon test is failing here, then it’s probably because the before/after change attributes are the same… Does wireshark show that the before/after change attributes on this directory are different? Ah, that's what you mean. Yes, Wireshark does show the same values for before and after changeids. Anna > >> >> I hope this helps! >> Anna >>> >>>> On Dec 19, 2016, at 11:54, Anna Schumaker <schumaker.anna@gmail.com> wrote: >>>> >>>> Hi Trond, >>>> >>>> On 12/17/2016 01:27 PM, Trond Myklebust wrote: >>>>> If an operation that modified the directory raced with a GETATTR, then we >>>>> don't need to invalidate the directory cache more than once. >>>> >>>> This patch causes cthon basic tests to fail with: >>>> >>>> ./test6: readdir >>>> ./test6: (/nfs/all) unlinked 'file.0' dir entry read pass 1 >>>> ./test6: (/nfs/all) Test failed with 1 errors >>>> basic tests failed >>>> >>>> Thanks, >>>> Anna >>>>> >>>>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >>>>> --- >>>>> fs/nfs/nfs4proc.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>>>> index d33242c8d95d..713932440e07 100644 >>>>> --- a/fs/nfs/nfs4proc.c >>>>> +++ b/fs/nfs/nfs4proc.c >>>>> @@ -1088,12 +1088,15 @@ static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo) >>>>> struct nfs_inode *nfsi = NFS_I(dir); >>>>> >>>>> spin_lock(&dir->i_lock); >>>>> + if (dir->i_version == cinfo->after) >>>>> + goto out; >>>>> nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; >>>>> if (!cinfo->atomic || cinfo->before != dir->i_version) >>>>> nfs_force_lookup_revalidate(dir); >>>>> dir->i_version = cinfo->after; >>>>> nfsi->attr_gencount = nfs_inc_attr_generation_counter(); >>>>> nfs_fscache_invalidate(dir); >>>>> +out: >>>>> spin_unlock(&dir->i_lock); >>>>> } >>>>> >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-12-19 19:06 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-17 18:27 [PATCH 0/9] NFS client attribute cache performance improvements Trond Myklebust 2016-12-17 18:27 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Trond Myklebust 2016-12-17 18:27 ` [PATCH 2/9] NFSv4: Update the attribute cache info in update_changeattr Trond Myklebust 2016-12-17 18:27 ` [PATCH 3/9] NFSv4: Don't discard the attributes returned by asynchronous DELEGRETURN Trond Myklebust 2016-12-17 18:27 ` [PATCH 4/9] NFS: Don't revalidate the file on close if we hold a delegation Trond Myklebust 2016-12-17 18:27 ` [PATCH 5/9] NFS: Clean up cache validity checking Trond Myklebust 2016-12-17 18:27 ` [PATCH 6/9] NFS: Only look at the change attribute cache state in nfs_weak_revalidate() Trond Myklebust 2016-12-17 18:27 ` [PATCH 7/9] NFS: Fix and clean up the access cache validity checking Trond Myklebust 2016-12-17 18:27 ` [PATCH 8/9] NFS: Remove unused function nfs_revalidate_inode_rcu() Trond Myklebust 2016-12-17 18:27 ` [PATCH 9/9] NFS: Clean up nfs_attribute_timeout() Trond Myklebust 2016-12-19 16:54 ` [PATCH 1/9] NFSv4: Don't invalidate the directory twice Anna Schumaker 2016-12-19 16:56 ` Trond Myklebust 2016-12-19 17:00 ` Trond Myklebust 2016-12-19 17:07 ` Anna Schumaker 2016-12-19 17:30 ` Trond Myklebust 2016-12-19 19:05 ` Anna Schumaker
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.