* [PATCH 0/5] Performance optimsations for 4.10
@ 2016-12-01 22:06 Trond Myklebust
2016-12-01 22:06 ` [PATCH 1/5] NFSv4: Don't check file access when reclaiming state Trond Myklebust
0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2016-12-01 22:06 UTC (permalink / raw)
To: linux-nfs
This patch series contains a set of minor optimisations that are mainly
useful against pNFS systems that may incur a higher performance penalty
on unnecessary attribute requests by triggering an internal RPC call from
the MDS to the DS.
- Speed up state recovery after a server reboot by avoiding unnecessary
GETATTR and ACCESS checks.
- Avoid unnecessary attribute revalidations on operations such as
OPEN_DOWNGRADE, where we don't yet need to update close-to-open
cache consistency attributes.
- Avoid unnecessary attribute revalidations when we hold a delegation.
Trond Myklebust (5):
NFSv4: Don't check file access when reclaiming state
NFSv4: Don't ask for the change attribute when reclaiming state
NFSv4: Don't request a GETATTR on open_downgrade.
NFSv4: Don't request close-to-open attribute when holding a delegation
NFSv4: Optimise away forced revalidation when we know the attributes
are OK
fs/nfs/delegation.c | 4 ----
fs/nfs/inode.c | 2 +-
fs/nfs/nfs4proc.c | 25 +++++++++++++++++++------
fs/nfs/nfs4xdr.c | 13 ++++---------
4 files changed, 24 insertions(+), 20 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/5] NFSv4: Don't check file access when reclaiming state 2016-12-01 22:06 [PATCH 0/5] Performance optimsations for 4.10 Trond Myklebust @ 2016-12-01 22:06 ` Trond Myklebust 2016-12-01 22:06 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute " Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-12-01 22:06 UTC (permalink / raw) To: linux-nfs If we're reclaiming state after a reboot, or as part of returning a delegation, we don't need to check access modes again. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 241da19b7da4..b582df89c083 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1221,6 +1221,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry, atomic_inc(&sp->so_count); p->o_arg.open_flags = flags; p->o_arg.fmode = fmode & (FMODE_READ|FMODE_WRITE); + p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim); p->o_arg.share_access = nfs4_map_atomic_open_share(server, fmode, flags); /* don't put an ACCESS op in OPEN compound if O_EXCL, because ACCESS @@ -1228,8 +1229,16 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry, if (!(flags & O_EXCL)) { /* ask server to check for all possible rights as results * are cached */ - p->o_arg.access = NFS4_ACCESS_READ | NFS4_ACCESS_MODIFY | - NFS4_ACCESS_EXTEND | NFS4_ACCESS_EXECUTE; + switch (p->o_arg.claim) { + default: + break; + case NFS4_OPEN_CLAIM_NULL: + case NFS4_OPEN_CLAIM_FH: + p->o_arg.access = NFS4_ACCESS_READ | + NFS4_ACCESS_MODIFY | + NFS4_ACCESS_EXTEND | + NFS4_ACCESS_EXECUTE; + } } p->o_arg.clientid = server->nfs_client->cl_clientid; p->o_arg.id.create_time = ktime_to_ns(sp->so_seqid.create_time); @@ -1239,7 +1248,6 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry, p->o_arg.bitmask = nfs4_bitmask(server, label); p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0]; p->o_arg.label = nfs4_label_copy(p->a_label, label); - p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim); switch (p->o_arg.claim) { case NFS4_OPEN_CLAIM_NULL: case NFS4_OPEN_CLAIM_DELEGATE_CUR: -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state 2016-12-01 22:06 ` [PATCH 1/5] NFSv4: Don't check file access when reclaiming state Trond Myklebust @ 2016-12-01 22:06 ` Trond Myklebust 2016-12-01 22:06 ` [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade Trond Myklebust 2016-12-02 17:05 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state Anna Schumaker 0 siblings, 2 replies; 10+ messages in thread From: Trond Myklebust @ 2016-12-01 22:06 UTC (permalink / raw) To: linux-nfs We don't need to ask for the change attribute when returning a delegation or recovering from a server reboot, and it could actually cause us to obtain an incorrect value if we're using a pNFS flavour that requires LAYOUTCOMMIT. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index b582df89c083..c0628f78ed98 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -226,7 +226,6 @@ static const u32 nfs4_pnfs_open_bitmap[3] = { static const u32 nfs4_open_noattr_bitmap[3] = { FATTR4_WORD0_TYPE - | FATTR4_WORD0_CHANGE | FATTR4_WORD0_FILEID, }; -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade. 2016-12-01 22:06 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute " Trond Myklebust @ 2016-12-01 22:06 ` Trond Myklebust 2016-12-01 22:06 ` [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation Trond Myklebust 2016-12-02 17:05 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state Anna Schumaker 1 sibling, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-12-01 22:06 UTC (permalink / raw) To: linux-nfs If we're not closing the file completely, there is no need to request close-to-open attributes. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4xdr.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index fc89e5ed07ee..c37473721230 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -499,14 +499,12 @@ static int nfs4_stat_to_errno(int); (compound_encode_hdr_maxsz + \ encode_sequence_maxsz + \ encode_putfh_maxsz + \ - encode_open_downgrade_maxsz + \ - encode_getattr_maxsz) + encode_open_downgrade_maxsz) #define NFS4_dec_open_downgrade_sz \ (compound_decode_hdr_maxsz + \ decode_sequence_maxsz + \ decode_putfh_maxsz + \ - decode_open_downgrade_maxsz + \ - decode_getattr_maxsz) + decode_open_downgrade_maxsz) #define NFS4_enc_close_sz (compound_encode_hdr_maxsz + \ encode_sequence_maxsz + \ encode_putfh_maxsz + \ @@ -2328,7 +2326,6 @@ static void nfs4_xdr_enc_open_downgrade(struct rpc_rqst *req, encode_sequence(xdr, &args->seq_args, &hdr); encode_putfh(xdr, args->fh, &hdr); encode_open_downgrade(xdr, args, &hdr); - encode_getfattr(xdr, args->bitmask, &hdr); encode_nops(&hdr); } @@ -6115,9 +6112,6 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, if (status) goto out; status = decode_open_downgrade(xdr, res); - if (status != 0) - goto out; - decode_getfattr(xdr, res->fattr, res->server); out: return status; } -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation 2016-12-01 22:06 ` [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade Trond Myklebust @ 2016-12-01 22:06 ` Trond Myklebust 2016-12-01 22:06 ` [PATCH 5/5] NFSv4: Optimise away forced revalidation when we know the attributes are OK Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-12-01 22:06 UTC (permalink / raw) To: linux-nfs If holding a delegation, we do not need to ask the server to return close-to-open cache consistency attributes as part of the CLOSE compound. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 10 ++++++++-- fs/nfs/nfs4xdr.c | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index c0628f78ed98..917a6db5c84f 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3156,8 +3156,15 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data) goto out_wait; } - if (calldata->arg.fmode == 0) + if (calldata->arg.fmode == 0) { task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_CLOSE]; + + /* Close-to-open cache consistency revalidation */ + if (!nfs4_have_delegation(inode, FMODE_READ)) + calldata->arg.bitmask = NFS_SERVER(inode)->cache_consistency_bitmask; + else + calldata->arg.bitmask = NULL; + } if (calldata->roc) pnfs_roc_get_barrier(inode, &calldata->roc_barrier); @@ -3240,7 +3247,6 @@ int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait) if (IS_ERR(calldata->arg.seqid)) goto out_free_calldata; calldata->arg.fmode = 0; - calldata->arg.bitmask = server->cache_consistency_bitmask; calldata->res.fattr = &calldata->fattr; calldata->res.seqid = calldata->arg.seqid; calldata->res.server = server; diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index c37473721230..b54931503872 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -2248,7 +2248,8 @@ static void nfs4_xdr_enc_close(struct rpc_rqst *req, struct xdr_stream *xdr, encode_sequence(xdr, &args->seq_args, &hdr); encode_putfh(xdr, args->fh, &hdr); encode_close(xdr, args, &hdr); - encode_getfattr(xdr, args->bitmask, &hdr); + if (args->bitmask != NULL) + encode_getfattr(xdr, args->bitmask, &hdr); encode_nops(&hdr); } -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] NFSv4: Optimise away forced revalidation when we know the attributes are OK 2016-12-01 22:06 ` [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation Trond Myklebust @ 2016-12-01 22:06 ` Trond Myklebust 0 siblings, 0 replies; 10+ messages in thread From: Trond Myklebust @ 2016-12-01 22:06 UTC (permalink / raw) To: linux-nfs The NFS_INO_REVAL_FORCED flag needs to be set if we just got a delegation, and we see that there might still be some ambiguity as to whether or not our attribute or data cache are valid. In practice, this means that a call to nfs_check_inode_attributes() will have noticed a discrepancy between cached attributes and measured ones, so let's move the setting of NFS_INO_REVAL_FORCED to there. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/delegation.c | 4 ---- fs/nfs/inode.c | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index dff600ae0d74..d7df5e67b0c1 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -391,10 +391,6 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct rcu_assign_pointer(nfsi->delegation, delegation); delegation = NULL; - /* Ensure we revalidate the attributes and page cache! */ - spin_lock(&inode->i_lock); - nfsi->cache_validity |= NFS_INO_REVAL_FORCED; - spin_unlock(&inode->i_lock); trace_nfs4_set_delegation(inode, res->delegation_type); out: diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 372bfe98f685..02453d76bfea 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1317,7 +1317,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_set_cache_invalid(inode, invalid | NFS_INO_REVAL_FORCED); nfsi->read_cache_jiffies = fattr->time_start; return 0; -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state 2016-12-01 22:06 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute " Trond Myklebust 2016-12-01 22:06 ` [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade Trond Myklebust @ 2016-12-02 17:05 ` Anna Schumaker 2016-12-02 17:28 ` Trond Myklebust 1 sibling, 1 reply; 10+ messages in thread From: Anna Schumaker @ 2016-12-02 17:05 UTC (permalink / raw) To: Trond Myklebust, linux-nfs Hi Trond, On 12/01/2016 05:06 PM, Trond Myklebust wrote: > We don't need to ask for the change attribute when returning a delegation > or recovering from a server reboot, and it could actually cause us to > obtain an incorrect value if we're using a pNFS flavour that requires > LAYOUTCOMMIT. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/nfs4proc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index b582df89c083..c0628f78ed98 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -226,7 +226,6 @@ static const u32 nfs4_pnfs_open_bitmap[3] = { > > static const u32 nfs4_open_noattr_bitmap[3] = { > FATTR4_WORD0_TYPE > - | FATTR4_WORD0_CHANGE Do these patches depend on another patch series? I'm having trouble applying this patch since my tree doesn't have FATTR4_WORD0_CHANGE yet. I'm having trouble with patch 4, too. Thanks, Anna > | FATTR4_WORD0_FILEID, > }; > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state 2016-12-02 17:05 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state Anna Schumaker @ 2016-12-02 17:28 ` Trond Myklebust 2016-12-02 17:47 ` Anna Schumaker 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-12-02 17:28 UTC (permalink / raw) To: Anna Schumaker; +Cc: Linux NFS Mailing List DQo+IE9uIERlYyAyLCAyMDE2LCBhdCAxMjowNSwgQW5uYSBTY2h1bWFrZXIgPHNjaHVtYWtlci5h bm5hQGdtYWlsLmNvbT4gd3JvdGU6DQo+IA0KPiBIaSBUcm9uZCwNCj4gDQo+IE9uIDEyLzAxLzIw MTYgMDU6MDYgUE0sIFRyb25kIE15a2xlYnVzdCB3cm90ZToNCj4+IFdlIGRvbid0IG5lZWQgdG8g YXNrIGZvciB0aGUgY2hhbmdlIGF0dHJpYnV0ZSB3aGVuIHJldHVybmluZyBhIGRlbGVnYXRpb24N Cj4+IG9yIHJlY292ZXJpbmcgZnJvbSBhIHNlcnZlciByZWJvb3QsIGFuZCBpdCBjb3VsZCBhY3R1 YWxseSBjYXVzZSB1cyB0bw0KPj4gb2J0YWluIGFuIGluY29ycmVjdCB2YWx1ZSBpZiB3ZSdyZSB1 c2luZyBhIHBORlMgZmxhdm91ciB0aGF0IHJlcXVpcmVzDQo+PiBMQVlPVVRDT01NSVQuDQo+PiAN Cj4+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVidXN0QHByaW1h cnlkYXRhLmNvbT4NCj4+IC0tLQ0KPj4gZnMvbmZzL25mczRwcm9jLmMgfCAxIC0NCj4+IDEgZmls ZSBjaGFuZ2VkLCAxIGRlbGV0aW9uKC0pDQo+PiANCj4+IGRpZmYgLS1naXQgYS9mcy9uZnMvbmZz NHByb2MuYyBiL2ZzL25mcy9uZnM0cHJvYy5jDQo+PiBpbmRleCBiNTgyZGY4OWMwODMuLmMwNjI4 Zjc4ZWQ5OCAxMDA2NDQNCj4+IC0tLSBhL2ZzL25mcy9uZnM0cHJvYy5jDQo+PiArKysgYi9mcy9u ZnMvbmZzNHByb2MuYw0KPj4gQEAgLTIyNiw3ICsyMjYsNiBAQCBzdGF0aWMgY29uc3QgdTMyIG5m czRfcG5mc19vcGVuX2JpdG1hcFszXSA9IHsNCj4+IA0KPj4gc3RhdGljIGNvbnN0IHUzMiBuZnM0 X29wZW5fbm9hdHRyX2JpdG1hcFszXSA9IHsNCj4+IAlGQVRUUjRfV09SRDBfVFlQRQ0KPj4gLQl8 IEZBVFRSNF9XT1JEMF9DSEFOR0UNCj4gDQo+IERvIHRoZXNlIHBhdGNoZXMgZGVwZW5kIG9uIGFu b3RoZXIgcGF0Y2ggc2VyaWVzPyAgSSdtIGhhdmluZyB0cm91YmxlIGFwcGx5aW5nIHRoaXMgcGF0 Y2ggc2luY2UgbXkgdHJlZSBkb2Vzbid0IGhhdmUgRkFUVFI0X1dPUkQwX0NIQU5HRSB5ZXQuICBJ J20gaGF2aW5nIHRyb3VibGUgd2l0aCBwYXRjaCA0LCB0b28uDQo+IA0KDQpJ4oCZdmUgcHVzaGVk IG91dCB0aGUgbGludXgtbmV4dCBicmFuY2ggaHR0cDovL2dpdC5saW51eC1uZnMub3JnLz9wPXRy b25kbXkvbGludXgtbmZzLmdpdDthPXNob3J0bG9nO2g9cmVmcy9oZWFkcy9saW51eC1uZXh0DQoN Cj4gVGhhbmtzLA0KPiBBbm5hDQo+IA0KPj4gCXwgRkFUVFI0X1dPUkQwX0ZJTEVJRCwNCj4+IH07 DQoNCg== ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state 2016-12-02 17:28 ` Trond Myklebust @ 2016-12-02 17:47 ` Anna Schumaker 0 siblings, 0 replies; 10+ messages in thread From: Anna Schumaker @ 2016-12-02 17:47 UTC (permalink / raw) To: Trond Myklebust; +Cc: Linux NFS Mailing List On 12/02/2016 12:28 PM, Trond Myklebust wrote: > >> On Dec 2, 2016, at 12:05, Anna Schumaker <schumaker.anna@gmail.com> wrote: >> >> Hi Trond, >> >> On 12/01/2016 05:06 PM, Trond Myklebust wrote: >>> We don't need to ask for the change attribute when returning a delegation >>> or recovering from a server reboot, and it could actually cause us to >>> obtain an incorrect value if we're using a pNFS flavour that requires >>> LAYOUTCOMMIT. >>> >>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >>> --- >>> fs/nfs/nfs4proc.c | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>> index b582df89c083..c0628f78ed98 100644 >>> --- a/fs/nfs/nfs4proc.c >>> +++ b/fs/nfs/nfs4proc.c >>> @@ -226,7 +226,6 @@ static const u32 nfs4_pnfs_open_bitmap[3] = { >>> >>> static const u32 nfs4_open_noattr_bitmap[3] = { >>> FATTR4_WORD0_TYPE >>> - | FATTR4_WORD0_CHANGE >> >> Do these patches depend on another patch series? I'm having trouble applying this patch since my tree doesn't have FATTR4_WORD0_CHANGE yet. I'm having trouble with patch 4, too. >> > > I’ve pushed out the linux-next branch http://git.linux-nfs.org/?p=trondmy/linux-nfs.git;a=shortlog;h=refs/heads/linux-next I'll look there, then! Thanks! > >> Thanks, >> Anna >> >>> | FATTR4_WORD0_FILEID, >>> }; > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/5] Optimisations for state management
@ 2016-11-10 21:41 Trond Myklebust
2016-11-10 21:41 ` [PATCH 1/5] NFSv4: Don't check file access when reclaiming state Trond Myklebust
0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2016-11-10 21:41 UTC (permalink / raw)
To: linux-nfs
The following patches constitute a grab bag of minor optimisations when
the NFS client is managing its state.
Trond Myklebust (5):
NFSv4: Don't check file access when reclaiming state
NFSv4: Don't ask for the change attribute when reclaiming state
NFSv4: Don't request a GETATTR on open_downgrade.
NFSv4: Don't request close-to-open attribute when holding a delegation
NFSv4: Optimise away forced revalidation when we know the attributes
are OK
fs/nfs/delegation.c | 4 ----
fs/nfs/inode.c | 2 +-
fs/nfs/nfs4proc.c | 23 +++++++++++++++++------
fs/nfs/nfs4xdr.c | 7 ++-----
4 files changed, 20 insertions(+), 16 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/5] NFSv4: Don't check file access when reclaiming state 2016-11-10 21:41 [PATCH 0/5] Optimisations for state management Trond Myklebust @ 2016-11-10 21:41 ` Trond Myklebust 2016-11-10 21:41 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute " Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-11-10 21:41 UTC (permalink / raw) To: linux-nfs If we're reclaiming state after a reboot, or as part of returning a delegation, we don't need to check access modes again. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 6a1d650e0419..4eead738da8e 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1221,6 +1221,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry, atomic_inc(&sp->so_count); p->o_arg.open_flags = flags; p->o_arg.fmode = fmode & (FMODE_READ|FMODE_WRITE); + p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim); p->o_arg.share_access = nfs4_map_atomic_open_share(server, fmode, flags); /* don't put an ACCESS op in OPEN compound if O_EXCL, because ACCESS @@ -1228,8 +1229,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry, if (!(flags & O_EXCL)) { /* ask server to check for all possible rights as results * are cached */ - p->o_arg.access = NFS4_ACCESS_READ | NFS4_ACCESS_MODIFY | - NFS4_ACCESS_EXTEND | NFS4_ACCESS_EXECUTE; + switch (p->o_arg.claim) { + case NFS4_OPEN_CLAIM_NULL: + case NFS4_OPEN_CLAIM_FH: + p->o_arg.access = NFS4_ACCESS_READ | + NFS4_ACCESS_MODIFY | + NFS4_ACCESS_EXTEND | + NFS4_ACCESS_EXECUTE; + } } p->o_arg.clientid = server->nfs_client->cl_clientid; p->o_arg.id.create_time = ktime_to_ns(sp->so_seqid.create_time); @@ -1239,7 +1246,6 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry, p->o_arg.bitmask = nfs4_bitmask(server, label); p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0]; p->o_arg.label = nfs4_label_copy(p->a_label, label); - p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim); switch (p->o_arg.claim) { case NFS4_OPEN_CLAIM_NULL: case NFS4_OPEN_CLAIM_DELEGATE_CUR: -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state 2016-11-10 21:41 ` [PATCH 1/5] NFSv4: Don't check file access when reclaiming state Trond Myklebust @ 2016-11-10 21:41 ` Trond Myklebust 2016-11-10 21:41 ` [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-11-10 21:41 UTC (permalink / raw) To: linux-nfs We don't need to ask for the change attribute when returning a delegation or recovering from a server reboot, and it could actually cause us to obtain an incorrect value if we're using a pNFS flavour that requires LAYOUTCOMMIT. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 4eead738da8e..1e7e0754a96d 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -226,7 +226,6 @@ static const u32 nfs4_pnfs_open_bitmap[3] = { static const u32 nfs4_open_noattr_bitmap[3] = { FATTR4_WORD0_TYPE - | FATTR4_WORD0_CHANGE | FATTR4_WORD0_FILEID, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade. 2016-11-10 21:41 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute " Trond Myklebust @ 2016-11-10 21:41 ` Trond Myklebust 2016-11-10 21:41 ` [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-11-10 21:41 UTC (permalink / raw) To: linux-nfs If we're not closing the file completely, there is no need to request close-to-open attributes. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4xdr.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index fc89e5ed07ee..43cc989b5c06 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -2328,7 +2328,6 @@ static void nfs4_xdr_enc_open_downgrade(struct rpc_rqst *req, encode_sequence(xdr, &args->seq_args, &hdr); encode_putfh(xdr, args->fh, &hdr); encode_open_downgrade(xdr, args, &hdr); - encode_getfattr(xdr, args->bitmask, &hdr); encode_nops(&hdr); } @@ -6115,9 +6114,6 @@ static int nfs4_xdr_dec_open_downgrade(struct rpc_rqst *rqstp, if (status) goto out; status = decode_open_downgrade(xdr, res); - if (status != 0) - goto out; - decode_getfattr(xdr, res->fattr, res->server); out: return status; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation 2016-11-10 21:41 ` [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade Trond Myklebust @ 2016-11-10 21:41 ` Trond Myklebust 2016-11-10 21:41 ` [PATCH 5/5] NFSv4: Optimise away forced revalidation when we know the attributes are OK Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2016-11-10 21:41 UTC (permalink / raw) To: linux-nfs If holding a delegation, we do not need to ask the server to return close-to-open cache consistency attributes as part of the CLOSE compound. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 10 ++++++++-- fs/nfs/nfs4xdr.c | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 1e7e0754a96d..ded8854bc32f 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3141,8 +3141,15 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data) goto out_wait; } - if (calldata->arg.fmode == 0) + if (calldata->arg.fmode == 0) { task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_CLOSE]; + + /* Close-to-open cache consistency revalidation */ + if (!nfs4_have_delegation(inode, FMODE_READ)) + calldata->arg.bitmask = NFS_SERVER(inode)->cache_consistency_bitmask; + else + calldata->arg.bitmask = NULL; + } if (calldata->roc) pnfs_roc_get_barrier(inode, &calldata->roc_barrier); @@ -3225,7 +3232,6 @@ int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait) if (IS_ERR(calldata->arg.seqid)) goto out_free_calldata; calldata->arg.fmode = 0; - calldata->arg.bitmask = server->cache_consistency_bitmask; calldata->res.fattr = &calldata->fattr; calldata->res.seqid = calldata->arg.seqid; calldata->res.server = server; diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 43cc989b5c06..a8bebb52df78 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -2250,7 +2250,8 @@ static void nfs4_xdr_enc_close(struct rpc_rqst *req, struct xdr_stream *xdr, encode_sequence(xdr, &args->seq_args, &hdr); encode_putfh(xdr, args->fh, &hdr); encode_close(xdr, args, &hdr); - encode_getfattr(xdr, args->bitmask, &hdr); + if (args->bitmask != NULL) + encode_getfattr(xdr, args->bitmask, &hdr); encode_nops(&hdr); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] NFSv4: Optimise away forced revalidation when we know the attributes are OK 2016-11-10 21:41 ` [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation Trond Myklebust @ 2016-11-10 21:41 ` Trond Myklebust 0 siblings, 0 replies; 10+ messages in thread From: Trond Myklebust @ 2016-11-10 21:41 UTC (permalink / raw) To: linux-nfs The NFS_INO_REVAL_FORCED flag needs to be set if we just got a delegation, and we see that there might still be some ambiguity as to whether or not our attribute or data cache are valid. In practice, this means that a call to nfs_check_inode_attributes() will have noticed a discrepancy between cached attributes and measured ones, so let's move the setting of NFS_INO_REVAL_FORCED to there. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/delegation.c | 4 ---- fs/nfs/inode.c | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index dff600ae0d74..d7df5e67b0c1 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -391,10 +391,6 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct rcu_assign_pointer(nfsi->delegation, delegation); delegation = NULL; - /* Ensure we revalidate the attributes and page cache! */ - spin_lock(&inode->i_lock); - nfsi->cache_validity |= NFS_INO_REVAL_FORCED; - spin_unlock(&inode->i_lock); trace_nfs4_set_delegation(inode, res->delegation_type); out: diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index bf4ec5ecc97e..3575e3408bd7 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1317,7 +1317,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_set_cache_invalid(inode, invalid | NFS_INO_REVAL_FORCED); nfsi->read_cache_jiffies = fattr->time_start; return 0; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-02 17:48 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-12-01 22:06 [PATCH 0/5] Performance optimsations for 4.10 Trond Myklebust 2016-12-01 22:06 ` [PATCH 1/5] NFSv4: Don't check file access when reclaiming state Trond Myklebust 2016-12-01 22:06 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute " Trond Myklebust 2016-12-01 22:06 ` [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade Trond Myklebust 2016-12-01 22:06 ` [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation Trond Myklebust 2016-12-01 22:06 ` [PATCH 5/5] NFSv4: Optimise away forced revalidation when we know the attributes are OK Trond Myklebust 2016-12-02 17:05 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute when reclaiming state Anna Schumaker 2016-12-02 17:28 ` Trond Myklebust 2016-12-02 17:47 ` Anna Schumaker -- strict thread matches above, loose matches on Subject: below -- 2016-11-10 21:41 [PATCH 0/5] Optimisations for state management Trond Myklebust 2016-11-10 21:41 ` [PATCH 1/5] NFSv4: Don't check file access when reclaiming state Trond Myklebust 2016-11-10 21:41 ` [PATCH 2/5] NFSv4: Don't ask for the change attribute " Trond Myklebust 2016-11-10 21:41 ` [PATCH 3/5] NFSv4: Don't request a GETATTR on open_downgrade Trond Myklebust 2016-11-10 21:41 ` [PATCH 4/5] NFSv4: Don't request close-to-open attribute when holding a delegation Trond Myklebust 2016-11-10 21:41 ` [PATCH 5/5] NFSv4: Optimise away forced revalidation when we know the attributes are OK Trond Myklebust
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.