* [PATCH 01/11] NFSv4: Don't request size+change attribute if they are delegated to us @ 2018-06-04 22:31 Trond Myklebust 2018-06-04 22:31 ` [PATCH 02/11] NFS: Pass the inode down to the getattr() callback Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@primarydata.com> When we hold a delegation, we should not need to request attributes such as the file size or the change attribute. For some servers, avoiding asking for these unneeded attributes can improve the overall system performance. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/nfs4proc.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index bb1141c48281..c44cfa6be8ff 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -71,6 +71,8 @@ #define NFSDBG_FACILITY NFSDBG_PROC +#define NFS4_BITMASK_SZ 3 + #define NFS4_POLL_RETRY_MIN (HZ/10) #define NFS4_POLL_RETRY_MAX (15*HZ) @@ -274,6 +276,33 @@ const u32 nfs4_fs_locations_bitmap[3] = { | FATTR4_WORD1_MOUNTED_ON_FILEID, }; +static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src, + struct inode *inode) +{ + unsigned long cache_validity; + + memcpy(dst, src, NFS4_BITMASK_SZ*sizeof(*dst)); + if (!inode || !nfs4_have_delegation(inode, FMODE_READ)) + return; + + cache_validity = READ_ONCE(NFS_I(inode)->cache_validity); + if (!(cache_validity & NFS_INO_REVAL_FORCED)) + cache_validity &= ~(NFS_INO_INVALID_CHANGE + | NFS_INO_INVALID_SIZE); + + if (!(cache_validity & NFS_INO_INVALID_SIZE)) + dst[0] &= ~FATTR4_WORD0_SIZE; + + if (!(cache_validity & NFS_INO_INVALID_CHANGE)) + dst[0] &= ~FATTR4_WORD0_CHANGE; +} + +static void nfs4_bitmap_copy_adjust_setattr(__u32 *dst, + const __u32 *src, struct inode *inode) +{ + nfs4_bitmap_copy_adjust(dst, src, inode); +} + static void nfs4_setup_readdir(u64 cookie, __be32 *verifier, struct dentry *dentry, struct nfs4_readdir_arg *readdir) { @@ -3074,12 +3103,13 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, struct nfs4_label *olabel) { struct nfs_server *server = NFS_SERVER(inode); + __u32 bitmask[NFS4_BITMASK_SZ]; struct nfs4_state *state = ctx ? ctx->state : NULL; struct nfs_setattrargs arg = { .fh = NFS_FH(inode), .iap = sattr, .server = server, - .bitmask = server->attr_bitmask, + .bitmask = bitmask, .label = ilabel, }; struct nfs_setattrres res = { @@ -3094,11 +3124,11 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, }; int err; - arg.bitmask = nfs4_bitmask(server, ilabel); - if (ilabel) - arg.bitmask = nfs4_bitmask(server, olabel); - do { + nfs4_bitmap_copy_adjust_setattr(bitmask, + nfs4_bitmask(server, olabel), + inode); + err = _nfs4_do_setattr(inode, &arg, &res, cred, ctx); switch (err) { case -NFS4ERR_OPENMODE: -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 02/11] NFS: Pass the inode down to the getattr() callback 2018-06-04 22:31 [PATCH 01/11] NFSv4: Don't request size+change attribute if they are delegated to us Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 03/11] NFSv4: Don't ask for delegated attributes when revalidating the inode Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@primarydata.com> Allow the getattr() callback to check things like whether or not we hold a delegation so that it can adjust the attributes that it is asking for. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/client.c | 3 ++- fs/nfs/dir.c | 3 ++- fs/nfs/export.c | 2 +- fs/nfs/inode.c | 3 ++- fs/nfs/nfs3proc.c | 3 ++- fs/nfs/nfs4proc.c | 17 ++++++++++------- fs/nfs/proc.c | 3 ++- include/linux/nfs_xdr.h | 3 ++- 8 files changed, 23 insertions(+), 14 deletions(-) diff --git a/fs/nfs/client.c b/fs/nfs/client.c index b9129e2befea..02e97c29af0c 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -969,7 +969,8 @@ struct nfs_server *nfs_create_server(struct nfs_mount_info *mount_info, } if (!(fattr->valid & NFS_ATTR_FATTR)) { - error = nfs_mod->rpc_ops->getattr(server, mount_info->mntfh, fattr, NULL); + error = nfs_mod->rpc_ops->getattr(server, mount_info->mntfh, + fattr, NULL, NULL); if (error < 0) { dprintk("nfs_create_server: getattr error = %d\n", -error); goto error; diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 978a22ea962c..7a9c14426855 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1656,7 +1656,8 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle, nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); if (!(fattr->valid & NFS_ATTR_FATTR)) { struct nfs_server *server = NFS_SB(dentry->d_sb); - error = server->nfs_client->rpc_ops->getattr(server, fhandle, fattr, NULL); + error = server->nfs_client->rpc_ops->getattr(server, fhandle, + fattr, NULL, NULL); if (error < 0) goto out_error; } diff --git a/fs/nfs/export.c b/fs/nfs/export.c index ab5de3246c5c..deecb67638aa 100644 --- a/fs/nfs/export.c +++ b/fs/nfs/export.c @@ -102,7 +102,7 @@ nfs_fh_to_dentry(struct super_block *sb, struct fid *fid, } rpc_ops = NFS_SB(sb)->nfs_client->rpc_ops; - ret = rpc_ops->getattr(NFS_SB(sb), server_fh, fattr, label); + ret = rpc_ops->getattr(NFS_SB(sb), server_fh, fattr, label, NULL); if (ret) { dprintk("%s: getattr failed %d\n", __func__, ret); dentry = ERR_PTR(ret); diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 5de724b1b90c..a720427e5aa3 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1101,7 +1101,8 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode) goto out; } - status = NFS_PROTO(inode)->getattr(server, NFS_FH(inode), fattr, label); + status = NFS_PROTO(inode)->getattr(server, NFS_FH(inode), fattr, + label, inode); if (status != 0) { dfprintk(PAGECACHE, "nfs_revalidate_inode: (%s/%Lu) getattr failed, error=%d\n", inode->i_sb->s_id, diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c index 5645ef4c5259..ec8a9efa268f 100644 --- a/fs/nfs/nfs3proc.c +++ b/fs/nfs/nfs3proc.c @@ -101,7 +101,8 @@ nfs3_proc_get_root(struct nfs_server *server, struct nfs_fh *fhandle, */ static int nfs3_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, - struct nfs_fattr *fattr, struct nfs4_label *label) + struct nfs_fattr *fattr, struct nfs4_label *label, + struct inode *inode) { struct rpc_message msg = { .rpc_proc = &nfs3_procedures[NFS3PROC_GETATTR], diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index c44cfa6be8ff..cd60e8360ef2 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -92,8 +92,8 @@ static void nfs4_layoutget_release(void *calldata); static int _nfs4_recover_proc_open(struct nfs4_opendata *data); static int nfs4_do_fsinfo(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *); static void nfs_fixup_referral_attributes(struct nfs_fattr *fattr); -static int nfs4_proc_getattr(struct nfs_server *, struct nfs_fh *, struct nfs_fattr *, struct nfs4_label *label); -static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fattr *fattr, struct nfs4_label *label); +static int nfs4_proc_getattr(struct nfs_server *, struct nfs_fh *, struct nfs_fattr *, struct nfs4_label *label, struct inode *inode); +static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fattr *fattr, struct nfs4_label *label, struct inode *inode); static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, struct nfs_fattr *fattr, struct iattr *sattr, struct nfs_open_context *ctx, struct nfs4_label *ilabel, @@ -2494,7 +2494,8 @@ static int _nfs4_proc_open(struct nfs4_opendata *data, } if (!(o_res->f_attr->valid & NFS_ATTR_FATTR)) { nfs4_sequence_free_slot(&o_res->seq_res); - nfs4_proc_getattr(server, &o_res->fh, o_res->f_attr, o_res->f_label); + nfs4_proc_getattr(server, &o_res->fh, o_res->f_attr, + o_res->f_label, NULL); } return 0; } @@ -3763,7 +3764,7 @@ static int nfs4_proc_get_root(struct nfs_server *server, struct nfs_fh *mntfh, if (IS_ERR(label)) return PTR_ERR(label); - error = nfs4_proc_getattr(server, mntfh, fattr, label); + error = nfs4_proc_getattr(server, mntfh, fattr, label, NULL); if (error < 0) { dprintk("nfs4_get_root: getattr error = %d\n", -error); goto err_free_label; @@ -3828,7 +3829,8 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir, } static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, - struct nfs_fattr *fattr, struct nfs4_label *label) + struct nfs_fattr *fattr, struct nfs4_label *label, + struct inode *inode) { struct nfs4_getattr_arg args = { .fh = fhandle, @@ -3852,12 +3854,13 @@ static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, } static int nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, - struct nfs_fattr *fattr, struct nfs4_label *label) + struct nfs_fattr *fattr, struct nfs4_label *label, + struct inode *inode) { struct nfs4_exception exception = { }; int err; do { - err = _nfs4_proc_getattr(server, fhandle, fattr, label); + err = _nfs4_proc_getattr(server, fhandle, fattr, label, inode); trace_nfs4_getattr(server, fhandle, fattr, err); err = nfs4_handle_exception(server, err, &exception); diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c index 763f77e7f1f1..e0c257bd62b9 100644 --- a/fs/nfs/proc.c +++ b/fs/nfs/proc.c @@ -99,7 +99,8 @@ nfs_proc_get_root(struct nfs_server *server, struct nfs_fh *fhandle, */ static int nfs_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, - struct nfs_fattr *fattr, struct nfs4_label *label) + struct nfs_fattr *fattr, struct nfs4_label *label, + struct inode *inode) { struct rpc_message msg = { .rpc_proc = &nfs_procedures[NFSPROC_GETATTR], diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index 09dc14ac5804..9dee3c23895d 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -1581,7 +1581,8 @@ struct nfs_rpc_ops { struct dentry *(*try_mount) (int, const char *, struct nfs_mount_info *, struct nfs_subversion *); int (*getattr) (struct nfs_server *, struct nfs_fh *, - struct nfs_fattr *, struct nfs4_label *); + struct nfs_fattr *, struct nfs4_label *, + struct inode *); int (*setattr) (struct dentry *, struct nfs_fattr *, struct iattr *); int (*lookup) (struct inode *, const struct qstr *, -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 03/11] NFSv4: Don't ask for delegated attributes when revalidating the inode 2018-06-04 22:31 ` [PATCH 02/11] NFS: Pass the inode down to the getattr() callback Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 04/11] NFSv4: Don't ask for delegated attributes when adding a hard link Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@primarydata.com> Again, when revalidating the inode, we don't need to ask for attributes for which we are authoritative. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/nfs4proc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index cd60e8360ef2..744a6367618d 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3832,9 +3832,10 @@ static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fattr *fattr, struct nfs4_label *label, struct inode *inode) { + __u32 bitmask[NFS4_BITMASK_SZ]; struct nfs4_getattr_arg args = { .fh = fhandle, - .bitmask = server->attr_bitmask, + .bitmask = bitmask, }; struct nfs4_getattr_res res = { .fattr = fattr, @@ -3847,7 +3848,7 @@ static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, .rpc_resp = &res, }; - args.bitmask = nfs4_bitmask(server, label); + nfs4_bitmap_copy_adjust(bitmask, nfs4_bitmask(server, label), inode); nfs_fattr_init(fattr); return nfs4_call_sync(server->client, server, &msg, &args.seq_args, &res.seq_res, 0); -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 04/11] NFSv4: Don't ask for delegated attributes when adding a hard link 2018-06-04 22:31 ` [PATCH 03/11] NFSv4: Don't ask for delegated attributes when revalidating the inode Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 05/11] NFSv4: Ignore NFS_INO_REVAL_FORCED in nfs4_proc_access Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@primarydata.com> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/nfs4proc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 744a6367618d..544cdcb79b4f 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -4378,11 +4378,12 @@ static int nfs4_proc_rename_done(struct rpc_task *task, struct inode *old_dir, static int _nfs4_proc_link(struct inode *inode, struct inode *dir, const struct qstr *name) { struct nfs_server *server = NFS_SERVER(inode); + __u32 bitmask[NFS4_BITMASK_SZ]; struct nfs4_link_arg arg = { .fh = NFS_FH(inode), .dir_fh = NFS_FH(dir), .name = name, - .bitmask = server->attr_bitmask, + .bitmask = bitmask, }; struct nfs4_link_res res = { .server = server, @@ -4404,9 +4405,9 @@ static int _nfs4_proc_link(struct inode *inode, struct inode *dir, const struct status = PTR_ERR(res.label); goto out; } - arg.bitmask = nfs4_bitmask(server, res.label); nfs4_inode_make_writeable(inode); + nfs4_bitmap_copy_adjust_setattr(bitmask, nfs4_bitmask(server, res.label), inode); status = nfs4_call_sync(server->client, server, &msg, &arg.seq_args, &res.seq_res, 1); if (!status) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 05/11] NFSv4: Ignore NFS_INO_REVAL_FORCED in nfs4_proc_access 2018-06-04 22:31 ` [PATCH 04/11] NFSv4: Don't ask for delegated attributes when adding a hard link Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 06/11] NFSv4: Ensure the inode is clean when we set a delegation Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs If we hold a delegation, we don't need to care about whether or not the inode attributes are up to date. We know we can cache the results of this call regardless. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/nfs4proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 544cdcb79b4f..f413d0c8c837 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -4114,7 +4114,7 @@ static int _nfs4_proc_access(struct inode *inode, struct nfs_access_entry *entry }; int status = 0; - if (!nfs_have_delegated_attributes(inode)) { + if (!nfs4_have_delegation(inode, FMODE_READ)) { res.fattr = nfs_alloc_fattr(); if (res.fattr == NULL) return -ENOMEM; -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 06/11] NFSv4: Ensure the inode is clean when we set a delegation 2018-06-04 22:31 ` [PATCH 05/11] NFSv4: Ignore NFS_INO_REVAL_FORCED in nfs4_proc_access Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 07/11] NFS: fix up nfs_setattr_update_inode Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@primarydata.com> If there are attributes that are still invalid when we set a delegation, then we need to set the NFS_INO_REVAL_FORCED flag. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 105b0f39a5b2..91c3737d69df 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -404,6 +404,10 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, trace_nfs4_set_delegation(inode, type); + spin_lock(&inode->i_lock); + if (NFS_I(inode)->cache_validity & (NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME)) + NFS_I(inode)->cache_validity |= NFS_INO_REVAL_FORCED; + spin_unlock(&inode->i_lock); out: spin_unlock(&clp->cl_lock); if (delegation != NULL) -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 07/11] NFS: fix up nfs_setattr_update_inode 2018-06-04 22:31 ` [PATCH 06/11] NFSv4: Ensure the inode is clean when we set a delegation Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 08/11] NFS: Fix attribute revalidation Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@primarydata.com> Always try to set the attributes, even if we don't have a valid struct nfs_fattr. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/inode.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index a720427e5aa3..2b7edd011d86 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -671,9 +671,13 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr, spin_lock(&inode->i_lock); NFS_I(inode)->attr_gencount = fattr->gencount; - nfs_set_cache_invalid(inode, NFS_INO_INVALID_CHANGE - | NFS_INO_INVALID_CTIME); + if ((attr->ia_valid & ATTR_SIZE) != 0) { + nfs_set_cache_invalid(inode, NFS_INO_INVALID_MTIME); + nfs_inc_stats(inode, NFSIOS_SETATTRTRUNC); + nfs_vmtruncate(inode, attr->ia_size); + } if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) != 0) { + NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_CTIME; if ((attr->ia_valid & ATTR_MODE) != 0) { int mode = attr->ia_mode & S_IALLUGO; mode |= inode->i_mode & ~S_IALLUGO; @@ -683,13 +687,45 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr, inode->i_uid = attr->ia_uid; if ((attr->ia_valid & ATTR_GID) != 0) inode->i_gid = attr->ia_gid; + if (fattr->valid & NFS_ATTR_FATTR_CTIME) + inode->i_ctime = fattr->ctime; + else + nfs_set_cache_invalid(inode, NFS_INO_INVALID_CHANGE + | NFS_INO_INVALID_CTIME); nfs_set_cache_invalid(inode, NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL); } - if ((attr->ia_valid & ATTR_SIZE) != 0) { - nfs_set_cache_invalid(inode, NFS_INO_INVALID_MTIME); - nfs_inc_stats(inode, NFSIOS_SETATTRTRUNC); - nfs_vmtruncate(inode, attr->ia_size); + if (attr->ia_valid & (ATTR_ATIME_SET|ATTR_ATIME)) { + NFS_I(inode)->cache_validity &= ~(NFS_INO_INVALID_ATIME + | NFS_INO_INVALID_CTIME); + if (fattr->valid & NFS_ATTR_FATTR_ATIME) + inode->i_atime = fattr->atime; + else if (attr->ia_valid & ATTR_ATIME_SET) + inode->i_atime = attr->ia_atime; + else + nfs_set_cache_invalid(inode, NFS_INO_INVALID_ATIME); + + if (fattr->valid & NFS_ATTR_FATTR_CTIME) + inode->i_ctime = fattr->ctime; + else + nfs_set_cache_invalid(inode, NFS_INO_INVALID_CHANGE + | NFS_INO_INVALID_CTIME); + } + if (attr->ia_valid & (ATTR_MTIME_SET|ATTR_MTIME)) { + NFS_I(inode)->cache_validity &= ~(NFS_INO_INVALID_MTIME + | NFS_INO_INVALID_CTIME); + if (fattr->valid & NFS_ATTR_FATTR_MTIME) + inode->i_mtime = fattr->mtime; + else if (attr->ia_valid & ATTR_MTIME_SET) + inode->i_mtime = attr->ia_mtime; + else + nfs_set_cache_invalid(inode, NFS_INO_INVALID_MTIME); + + if (fattr->valid & NFS_ATTR_FATTR_CTIME) + inode->i_ctime = fattr->ctime; + else + nfs_set_cache_invalid(inode, NFS_INO_INVALID_CHANGE + | NFS_INO_INVALID_CTIME); } if (fattr->valid) nfs_update_inode(inode, fattr); -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 08/11] NFS: Fix attribute revalidation 2018-06-04 22:31 ` [PATCH 07/11] NFS: fix up nfs_setattr_update_inode Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 09/11] NFS: Improve caching while holding a delegation Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs Don't mark attributes as invalid just because they have changed. Instead, for the purposes of adjusting the attribute cache timeout, keep a separate variable that tracks whether or not a change occurred. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/inode.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 2b7edd011d86..f35c5eb09cc9 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1788,6 +1788,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) unsigned long save_cache_validity; bool have_writers = nfs_file_has_buffered_writers(nfsi); bool cache_revalidated = true; + bool attr_changed = false; dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%x)\n", __func__, inode->i_sb->s_id, inode->i_ino, @@ -1848,8 +1849,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) inode->i_sb->s_id, inode->i_ino); /* Could it be a race with writeback? */ if (!have_writers) { - invalid |= NFS_INO_INVALID_CHANGE - | NFS_INO_INVALID_DATA + invalid |= NFS_INO_INVALID_DATA | NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL; /* Force revalidate of all attributes */ @@ -1861,6 +1861,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) nfs_force_lookup_revalidate(inode); } inode_set_iversion_raw(inode, fattr->change_attr); + attr_changed = true; } } else { nfsi->cache_validity |= save_cache_validity & @@ -1899,6 +1900,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) i_size_write(inode, new_isize); if (!have_writers) invalid |= NFS_INO_INVALID_DATA; + attr_changed = true; } dprintk("NFS: isize change on server for file %s/%ld " "(%Ld to %Ld)\n", @@ -1931,14 +1933,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) newmode |= fattr->mode & S_IALLUGO; inode->i_mode = newmode; invalid |= NFS_INO_INVALID_ACCESS - | NFS_INO_INVALID_ACL - | NFS_INO_INVALID_OTHER; + | NFS_INO_INVALID_ACL; + attr_changed = true; } } else if (server->caps & NFS_CAP_MODE) { nfsi->cache_validity |= save_cache_validity & - (NFS_INO_INVALID_ACCESS - | NFS_INO_INVALID_ACL - | NFS_INO_INVALID_OTHER + (NFS_INO_INVALID_OTHER | NFS_INO_REVAL_FORCED); cache_revalidated = false; } @@ -1946,15 +1946,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) if (fattr->valid & NFS_ATTR_FATTR_OWNER) { if (!uid_eq(inode->i_uid, fattr->uid)) { invalid |= NFS_INO_INVALID_ACCESS - | NFS_INO_INVALID_ACL - | NFS_INO_INVALID_OTHER; + | NFS_INO_INVALID_ACL; inode->i_uid = fattr->uid; + attr_changed = true; } } else if (server->caps & NFS_CAP_OWNER) { nfsi->cache_validity |= save_cache_validity & - (NFS_INO_INVALID_ACCESS - | NFS_INO_INVALID_ACL - | NFS_INO_INVALID_OTHER + (NFS_INO_INVALID_OTHER | NFS_INO_REVAL_FORCED); cache_revalidated = false; } @@ -1962,25 +1960,23 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) if (fattr->valid & NFS_ATTR_FATTR_GROUP) { if (!gid_eq(inode->i_gid, fattr->gid)) { invalid |= NFS_INO_INVALID_ACCESS - | NFS_INO_INVALID_ACL - | NFS_INO_INVALID_OTHER; + | NFS_INO_INVALID_ACL; inode->i_gid = fattr->gid; + attr_changed = true; } } else if (server->caps & NFS_CAP_OWNER_GROUP) { nfsi->cache_validity |= save_cache_validity & - (NFS_INO_INVALID_ACCESS - | NFS_INO_INVALID_ACL - | NFS_INO_INVALID_OTHER + (NFS_INO_INVALID_OTHER | NFS_INO_REVAL_FORCED); cache_revalidated = false; } if (fattr->valid & NFS_ATTR_FATTR_NLINK) { if (inode->i_nlink != fattr->nlink) { - invalid |= NFS_INO_INVALID_OTHER; if (S_ISDIR(inode->i_mode)) invalid |= NFS_INO_INVALID_DATA; set_nlink(inode, fattr->nlink); + attr_changed = true; } } else if (server->caps & NFS_CAP_NLINK) { nfsi->cache_validity |= save_cache_validity & @@ -2000,7 +1996,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) cache_revalidated = false; /* Update attrtimeo value if we're out of the unstable period */ - if (invalid & NFS_INO_INVALID_ATTR) { + if (attr_changed) { invalid &= ~NFS_INO_INVALID_ATTR; nfs_inc_stats(inode, NFSIOS_ATTRINVALIDATE); nfsi->attrtimeo = NFS_MINATTRTIMEO(inode); -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 09/11] NFS: Improve caching while holding a delegation 2018-06-04 22:31 ` [PATCH 08/11] NFS: Fix attribute revalidation Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 10/11] NFS: Ignore NFS_INO_REVAL_FORCED in nfs_check_inode_attributes() Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs Make sure that the client completely ignores change attribute and size changes on the server when it holds a delegation. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/inode.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index f35c5eb09cc9..90d9fbfd82db 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1789,6 +1789,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) bool have_writers = nfs_file_has_buffered_writers(nfsi); bool cache_revalidated = true; bool attr_changed = false; + bool have_delegation; dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%x)\n", __func__, inode->i_sb->s_id, inode->i_ino, @@ -1823,6 +1824,9 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) !IS_AUTOMOUNT(inode)) server->fsid = fattr->fsid; + /* Save the delegation state before clearing cache_validity */ + have_delegation = nfs_have_delegated_attributes(inode); + /* * Update the read time so we don't revalidate too often. */ @@ -1845,10 +1849,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) /* More cache consistency checks */ if (fattr->valid & NFS_ATTR_FATTR_CHANGE) { if (!inode_eq_iversion_raw(inode, fattr->change_attr)) { - dprintk("NFS: change_attr change on server for file %s/%ld\n", - inode->i_sb->s_id, inode->i_ino); /* Could it be a race with writeback? */ - if (!have_writers) { + if (!(have_writers || have_delegation)) { invalid |= NFS_INO_INVALID_DATA | NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL; @@ -1859,6 +1861,9 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) | NFS_INO_INVALID_OTHER; if (S_ISDIR(inode->i_mode)) nfs_force_lookup_revalidate(inode); + dprintk("NFS: change_attr change on server for file %s/%ld\n", + inode->i_sb->s_id, + inode->i_ino); } inode_set_iversion_raw(inode, fattr->change_attr); attr_changed = true; @@ -1893,7 +1898,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) if (fattr->valid & NFS_ATTR_FATTR_SIZE) { new_isize = nfs_size_to_loff_t(fattr->size); cur_isize = i_size_read(inode); - if (new_isize != cur_isize) { + if (new_isize != cur_isize && !have_delegation) { /* Do we perhaps have any outstanding writes, or has * the file grown beyond our last write? */ if (!nfs_have_writebacks(inode) || new_isize > cur_isize) { @@ -2022,9 +2027,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))) invalid &= ~NFS_INO_INVALID_DATA; - if (!NFS_PROTO(inode)->have_delegation(inode, FMODE_READ) || - (save_cache_validity & NFS_INO_REVAL_FORCED)) - nfs_set_cache_invalid(inode, invalid); + nfs_set_cache_invalid(inode, invalid); return 0; out_err: -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 10/11] NFS: Ignore NFS_INO_REVAL_FORCED in nfs_check_inode_attributes() 2018-06-04 22:31 ` [PATCH 09/11] NFS: Improve caching while holding a delegation Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 2018-06-04 22:31 ` [PATCH 11/11] NFS: Filter cache invalidation when holding a delegation Trond Myklebust 0 siblings, 1 reply; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs If we hold a delegation, we should not need to call nfs_check_inode_attributes() since we already know which attributes are valid, and which ones may still need revalidation. The state of the NFS_INO_REVAL_FORCED flag is therefore irrelevant. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/inode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 90d9fbfd82db..bc84ecaae886 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1390,8 +1390,9 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat unsigned long invalid = 0; - if (nfs_have_delegated_attributes(inode)) + if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) return 0; + /* Has the inode gone and changed behind our back? */ if ((fattr->valid & NFS_ATTR_FATTR_FILEID) && nfsi->fileid != fattr->fileid) return -ESTALE; @@ -1441,7 +1442,7 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat invalid |= NFS_INO_INVALID_ATIME; if (invalid != 0) - nfs_set_cache_invalid(inode, invalid | NFS_INO_REVAL_FORCED); + nfs_set_cache_invalid(inode, invalid); nfsi->read_cache_jiffies = fattr->time_start; return 0; -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 11/11] NFS: Filter cache invalidation when holding a delegation 2018-06-04 22:31 ` [PATCH 10/11] NFS: Ignore NFS_INO_REVAL_FORCED in nfs_check_inode_attributes() Trond Myklebust @ 2018-06-04 22:31 ` Trond Myklebust 0 siblings, 0 replies; 11+ messages in thread From: Trond Myklebust @ 2018-06-04 22:31 UTC (permalink / raw) To: linux-nfs If the client holds a delegation, then ensure we filter out attempts to invalidate the size, owner, group owner, or mode unless we made the change, in which case, check that NFS_INO_REVAL_FORCED is set by the caller. Always filter out attempts to invalidate the change attribute and size, since we are authoritative for those. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/inode.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index bc84ecaae886..73473d9bdfa4 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -195,10 +195,16 @@ bool nfs_check_cache_invalid(struct inode *inode, unsigned long flags) static void nfs_set_cache_invalid(struct inode *inode, unsigned long flags) { struct nfs_inode *nfsi = NFS_I(inode); - bool have_delegation = nfs_have_delegated_attributes(inode); + bool have_delegation = NFS_PROTO(inode)->have_delegation(inode, FMODE_READ); + + if (have_delegation) { + if (!(flags & NFS_INO_REVAL_FORCED)) + flags &= ~NFS_INO_INVALID_OTHER; + flags &= ~(NFS_INO_INVALID_CHANGE + | NFS_INO_INVALID_SIZE + | NFS_INO_REVAL_PAGECACHE); + } - if (have_delegation) - flags &= ~(NFS_INO_INVALID_CHANGE|NFS_INO_REVAL_PAGECACHE); if (inode->i_mapping->nrpages == 0) flags &= ~NFS_INO_INVALID_DATA; nfsi->cache_validity |= flags; -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-06-04 22:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-04 22:31 [PATCH 01/11] NFSv4: Don't request size+change attribute if they are delegated to us Trond Myklebust 2018-06-04 22:31 ` [PATCH 02/11] NFS: Pass the inode down to the getattr() callback Trond Myklebust 2018-06-04 22:31 ` [PATCH 03/11] NFSv4: Don't ask for delegated attributes when revalidating the inode Trond Myklebust 2018-06-04 22:31 ` [PATCH 04/11] NFSv4: Don't ask for delegated attributes when adding a hard link Trond Myklebust 2018-06-04 22:31 ` [PATCH 05/11] NFSv4: Ignore NFS_INO_REVAL_FORCED in nfs4_proc_access Trond Myklebust 2018-06-04 22:31 ` [PATCH 06/11] NFSv4: Ensure the inode is clean when we set a delegation Trond Myklebust 2018-06-04 22:31 ` [PATCH 07/11] NFS: fix up nfs_setattr_update_inode Trond Myklebust 2018-06-04 22:31 ` [PATCH 08/11] NFS: Fix attribute revalidation Trond Myklebust 2018-06-04 22:31 ` [PATCH 09/11] NFS: Improve caching while holding a delegation Trond Myklebust 2018-06-04 22:31 ` [PATCH 10/11] NFS: Ignore NFS_INO_REVAL_FORCED in nfs_check_inode_attributes() Trond Myklebust 2018-06-04 22:31 ` [PATCH 11/11] NFS: Filter cache invalidation when holding a delegation Trond Myklebust
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).