diff mbox

nfs4: client: do not send empty SETATTR after OPEN_CREATE

Message ID 1463004843-3733-1-git-send-email-tigran.mkrtchyan@desy.de (mailing list archive)
State New, archived
Headers show

Commit Message

Mkrtchyan, Tigran May 11, 2016, 10:14 p.m. UTC
OPEN_CREATE with EXCLUSIVE4_1 sends initial file permission.
Ignoring  fact, that server have indicated that file mod is set, client
will send yet another SETATTR request, but, as mode is already set,
new SETATTR will be empty. This is not a problem, nevertheless
an extra roundtrip and slow open on high latency networks.

This change is aims to by-pass setattr if there are no attributes defined.

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/nfs4proc.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Trond Myklebust May 11, 2016, 10:25 p.m. UTC | #1
On 5/11/16, 18:14, "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de> wrote:

>OPEN_CREATE with EXCLUSIVE4_1 sends initial file permission.

>Ignoring  fact, that server have indicated that file mod is set, client

>will send yet another SETATTR request, but, as mode is already set,

>new SETATTR will be empty. This is not a problem, nevertheless

>an extra roundtrip and slow open on high latency networks.

>

>This change is aims to by-pass setattr if there are no attributes defined.

>

>Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>

>---

> fs/nfs/nfs4proc.c | 6 ++++++

> 1 file changed, 6 insertions(+)

>

>diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c

>index 327b8c3..f6b278f 100644

>--- a/fs/nfs/nfs4proc.c

>+++ b/fs/nfs/nfs4proc.c

>@@ -2724,6 +2724,12 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,

> 		.inode = inode,

> 	};

> 	int err;

>+	/*

>+	 * a shortcut: it there are no attributes to be updated, do not send setattr at all

>+	 */

>+	if (sattr->ia_valid == ATTR_OPEN)

>+		return 0;

>+


There is already a similar optimization in nfs_setattr(), so I don’t think it makes sense to put this in nfs4_do_setattr(). We should rather just fix _nfs4_do_open().

> 	do {

> 		err = _nfs4_do_setattr(inode, cred, fattr, sattr, state, ilabel, olabel);

> 		switch (err) {

>-- 

>2.5.5

>
Mkrtchyan, Tigran May 12, 2016, 9:03 a.m. UTC | #2
----- Original Message -----
> From: "Trond Myklebust" <trondmy@primarydata.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: linux-nfs@vger.kernel.org
> Sent: Thursday, May 12, 2016 12:25:18 AM
> Subject: Re: [PATCH] nfs4: client: do not send empty SETATTR after OPEN_CREATE

> On 5/11/16, 18:14, "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de> wrote:
> 
>>OPEN_CREATE with EXCLUSIVE4_1 sends initial file permission.
>>Ignoring  fact, that server have indicated that file mod is set, client
>>will send yet another SETATTR request, but, as mode is already set,
>>new SETATTR will be empty. This is not a problem, nevertheless
>>an extra roundtrip and slow open on high latency networks.
>>
>>This change is aims to by-pass setattr if there are no attributes defined.
>>
>>Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
>>---
>> fs/nfs/nfs4proc.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>>diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>index 327b8c3..f6b278f 100644
>>--- a/fs/nfs/nfs4proc.c
>>+++ b/fs/nfs/nfs4proc.c
>>@@ -2724,6 +2724,12 @@ static int nfs4_do_setattr(struct inode *inode, struct
>>rpc_cred *cred,
>> 		.inode = inode,
>> 	};
>> 	int err;
>>+	/*
>>+	 * a shortcut: it there are no attributes to be updated, do not send setattr
>>at all
>>+	 */
>>+	if (sattr->ia_valid == ATTR_OPEN)
>>+		return 0;
>>+
> 
> There is already a similar optimization in nfs_setattr(), so I don’t think it
Obviously not mature enough.


> makes sense to put this in nfs4_do_setattr(). We should rather just fix
> _nfs4_do_open().

Agree. See v2.

Tigran.

> 
>> 	do {
>> 		err = _nfs4_do_setattr(inode, cred, fattr, sattr, state, ilabel, olabel);
>> 		switch (err) {
>>--
>>2.5.5
>>
> 
> N?????r??y???b?X???v?^?)?{.n?+????{???"??^n?r???z???h????&???G???h?(????j"???m?????z?????f???h???~?m?
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 327b8c3..f6b278f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2724,6 +2724,12 @@  static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
 		.inode = inode,
 	};
 	int err;
+	/*
+	 * a shortcut: it there are no attributes to be updated, do not send setattr at all
+	 */
+	if (sattr->ia_valid == ATTR_OPEN)
+		return 0;
+
 	do {
 		err = _nfs4_do_setattr(inode, cred, fattr, sattr, state, ilabel, olabel);
 		switch (err) {