Poor NFSv4 first impressions

Poor NFSv4 first impressions

[Jeff Garzik]

Being a big user of NFS at home, and a big fan of NFSv4, it was high 
time that I converted my home network from NFSv3 to NFSv4.

Unfortunately applications started breaking left and right.  vim 
noticeably malfunctioned, trying repeatedly to create a swapfile (sorta 
like a lockfile).  Mozilla Thunderbird would crash reproducibly whenever 
it tried anything remotely major with a mailbox, such as compressing 
folders (removing deleted messages).

Both NFSv4 server and NFSv4 client were x86-64 Linux boxes, running 
hyper-recent kernel 2.6.19-rc4-g10b1fbdb 
(10b1fbdb0a0ca91847a534ad26d0bc250c25b74f).  FC5 userland on the server, 
FC6 userland on the client.  There were no other clients connected to 
the NFS server, much less using my NFS homedir.  This data was not being 
accessed on the server directly, either.

/etc/exports contains:
/g 
10.10.10.0/255.255.255.0(rw,fsid=0,insecure,no_subtree_check,no_root_squash)

NFSv4 /etc/fstab line:
pretzel:/       /g      nfs4    defaults,proto=tcp,hard,intr    0 0

NFSv3 (previous) /etc/fstab line for same file server:
pretzel:/g              /g              nfs     defaults,tcp    0 0


I hope this is just a temporary problem, but as it looks right now, 
NFSv4 isn't ready for prime time, with all these apps breaking :/

	Jeff

Re: Poor NFSv4 first impressions

[Daniel J Blueman]

Jeff Garzik wrote:
> Being a big user of NFS at home, and a big fan of NFSv4, it was high
> time that I converted my home network from NFSv3 to NFSv4.
>
> Unfortunately applications started breaking left and right.  vim
> noticeably malfunctioned, trying repeatedly to create a swapfile (sorta
> like a lockfile).  Mozilla Thunderbird would crash reproducibly whenever
> it tried anything remotely major with a mailbox, such as compressing
> folders (removing deleted messages).
[snip]

This has all the symptoms to an open EACCES NFSv4 bug in 2.6.18/19.
This is fixed in:

http://www.citi.umich.edu/projects/nfsv4/linux/kernel-patches/2.6.19-rc3-2/linux-2.6.19-rc3-CITI_NFS4_ALL-2.diff
(see http://www.citi.umich.edu/projects/nfsv4/linux/).

With this patch, I can run just great with NFSv4 home dir (etc)
mounts; without, I get the symptom of many 0-byte temporary/lock files
being created and often the inability to create files (!). Be sure to
allow callback delegation connections in through your firewall for the
extra performance ;-) .

Maybe it's too late for these fixes 2.6.19, but they should certainly
make 2.6.19.1 IMHO.
-- 
Daniel J Blueman

Re: Poor NFSv4 first impressions

[Bill Davidsen]

Daniel J Blueman wrote:
> Jeff Garzik wrote:
>> Being a big user of NFS at home, and a big fan of NFSv4, it was high
>> time that I converted my home network from NFSv3 to NFSv4.
>>
>> Unfortunately applications started breaking left and right.  vim
>> noticeably malfunctioned, trying repeatedly to create a swapfile (sorta
>> like a lockfile).  Mozilla Thunderbird would crash reproducibly whenever
>> it tried anything remotely major with a mailbox, such as compressing
>> folders (removing deleted messages).
> [snip]
> 
> This has all the symptoms to an open EACCES NFSv4 bug in 2.6.18/19.
> This is fixed in:
> 
> http://www.citi.umich.edu/projects/nfsv4/linux/kernel-patches/2.6.19-rc3-2/linux-2.6.19-rc3-CITI_NFS4_ALL-2.diff 
> 
> (see http://www.citi.umich.edu/projects/nfsv4/linux/).
> 
> With this patch, I can run just great with NFSv4 home dir (etc)
> mounts; without, I get the symptom of many 0-byte temporary/lock files
> being created and often the inability to create files (!). Be sure to
> allow callback delegation connections in through your firewall for the
> extra performance ;-) .
> 
> Maybe it's too late for these fixes 2.6.19, but they should certainly
> make 2.6.19.1 IMHO.

If NFSv4 really works that poorly without the patches, perhaps they 
should go in 2.6.19 at the start. I'm surprised others aren't having 
this problem, I thought there was more test use.


-- 
Bill Davidsen <davidsen@tmr.com>
   Obscure bug of 2004: BASH BUFFER OVERFLOW - if bash is being run by a
normal user and is setuid root, with the "vi" line edit mode selected,
and the character set is "big5," an off-by-one errors occurs during
wildcard (glob) expansion.

Re: Poor NFSv4 first impressions

[J. Bruce Fields]

On Mon, Nov 06, 2006 at 12:03:54PM +0000, Daniel J Blueman wrote:
> This has all the symptoms to an open EACCES NFSv4 bug in 2.6.18/19.
> This is fixed in:
> 
> http://www.citi.umich.edu/projects/nfsv4/linux/kernel-patches/2.6.19-rc3-2/linux-2.6.19-rc3-CITI_NFS4_ALL-2.diff
> (see http://www.citi.umich.edu/projects/nfsv4/linux/).
> 
> With this patch, I can run just great with NFSv4 home dir (etc)
> mounts; without, I get the symptom of many 0-byte temporary/lock files
> being created and often the inability to create files (!). Be sure to
> allow callback delegation connections in through your firewall for the
> extra performance ;-) .
> 
> Maybe it's too late for these fixes 2.6.19, but they should certainly
> make 2.6.19.1 IMHO.

Yeah, bad patch management on my part, apologies, I should have pushed
it as soon as I noticed the problem.

Investigating the problem revealed some ugliness (and some races which
will need further work), and I had hoped to have a more complete fix
before now.  Oh well.

Two patches follow; the first does a very simple cleanup, the second
solves the immediate problem in the most straightforward way I can see,
but is a bit of a hack.

--b.

[PATCH 0/1] nfsd4: reindent do_open_lookup()

[J. Bruce Fields]

Minor rearrangement, cleanup of do_open_lookup().  No change in behavior.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/nfs4proc.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0a7bbdc..4a73f5b 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -106,27 +106,25 @@ do_open_lookup(struct svc_rqst *rqstp, s
 					open->op_fname.len, &open->op_iattr,
 					&resfh, open->op_createmode,
 					(u32 *)open->op_verf.data, &open->op_truncate);
-	}
-	else {
+	} else {
 		status = nfsd_lookup(rqstp, current_fh,
 				     open->op_fname.data, open->op_fname.len, &resfh);
 		fh_unlock(current_fh);
 	}
+	if (status)
+		goto out;
 
-	if (!status) {
-		set_change_info(&open->op_cinfo, current_fh);
+	set_change_info(&open->op_cinfo, current_fh);
 
-		/* set reply cache */
-		fh_dup2(current_fh, &resfh);
-		open->op_stateowner->so_replay.rp_openfh_len =
-			resfh.fh_handle.fh_size;
-		memcpy(open->op_stateowner->so_replay.rp_openfh,
-				&resfh.fh_handle.fh_base,
-				resfh.fh_handle.fh_size);
+	/* set reply cache */
+	fh_dup2(current_fh, &resfh);
+	open->op_stateowner->so_replay.rp_openfh_len = resfh.fh_handle.fh_size;
+	memcpy(open->op_stateowner->so_replay.rp_openfh,
+			&resfh.fh_handle.fh_base, resfh.fh_handle.fh_size);
 
-		status = do_open_permission(rqstp, current_fh, open, MAY_NOP);
-	}
+	status = do_open_permission(rqstp, current_fh, open, MAY_NOP);
 
+out:
 	fh_put(&resfh);
 	return status;
 }
-- 
1.4.3.3.g01929

[PATCH 2/2] nfsd4: fix open-create permissions

[J. Bruce Fields]

In the case where an open creates the file, we shouldn't be rechecking
permissions to open the file; the open succeeds regardless of what the
new file's mode bits say.

This patch fixes the problem, but only by introducing yet another parameter
to nfsd_create_v3.  This is ugly.  This will be fixed by later patches.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/nfs3proc.c        |    2 +-
 fs/nfsd/nfs4proc.c        |    6 ++++--
 fs/nfsd/vfs.c             |    4 +++-
 include/linux/nfsd/nfsd.h |    2 +-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 64db601..7f5bad0 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -258,7 +258,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp
 	/* Now create the file and set attributes */
 	nfserr = nfsd_create_v3(rqstp, dirfhp, argp->name, argp->len,
 				attr, newfhp,
-				argp->createmode, argp->verf, NULL);
+				argp->createmode, argp->verf, NULL, NULL);
 
 	RETURN_STATUS(nfserr);
 }
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4a73f5b..50bc942 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -93,6 +93,7 @@ do_open_lookup(struct svc_rqst *rqstp, s
 {
 	struct svc_fh resfh;
 	__be32 status;
+	int created = 0;
 
 	fh_init(&resfh, NFS4_FHSIZE);
 	open->op_truncate = 0;
@@ -105,7 +106,7 @@ do_open_lookup(struct svc_rqst *rqstp, s
 		status = nfsd_create_v3(rqstp, current_fh, open->op_fname.data,
 					open->op_fname.len, &open->op_iattr,
 					&resfh, open->op_createmode,
-					(u32 *)open->op_verf.data, &open->op_truncate);
+					(u32 *)open->op_verf.data, &open->op_truncate, &created);
 	} else {
 		status = nfsd_lookup(rqstp, current_fh,
 				     open->op_fname.data, open->op_fname.len, &resfh);
@@ -122,7 +123,8 @@ do_open_lookup(struct svc_rqst *rqstp, s
 	memcpy(open->op_stateowner->so_replay.rp_openfh,
 			&resfh.fh_handle.fh_base, resfh.fh_handle.fh_size);
 
-	status = do_open_permission(rqstp, current_fh, open, MAY_NOP);
+	if (!created)
+		status = do_open_permission(rqstp, current_fh, open, MAY_NOP);
 
 out:
 	fh_put(&resfh);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f21e917..1a7ad8c 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1237,7 +1237,7 @@ __be32
 nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		char *fname, int flen, struct iattr *iap,
 		struct svc_fh *resfhp, int createmode, u32 *verifier,
-	        int *truncp)
+	        int *truncp, int *created)
 {
 	struct dentry	*dentry, *dchild = NULL;
 	struct inode	*dirp;
@@ -1331,6 +1331,8 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 	host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
 	if (host_err < 0)
 		goto out_nfserr;
+	if (created)
+		*created = 1;
 
 	if (EX_ISSYNC(fhp->fh_export)) {
 		err = nfserrno(nfsd_sync_dir(dentry));
diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
index eb23114..edb54c3 100644
--- a/include/linux/nfsd/nfsd.h
+++ b/include/linux/nfsd/nfsd.h
@@ -89,7 +89,7 @@ __be32		nfsd_access(struct svc_rqst *, s
 __be32		nfsd_create_v3(struct svc_rqst *, struct svc_fh *,
 				char *name, int len, struct iattr *attrs,
 				struct svc_fh *res, int createmode,
-				u32 *verifier, int *truncp);
+				u32 *verifier, int *truncp, int *created);
 __be32		nfsd_commit(struct svc_rqst *, struct svc_fh *,
 				loff_t, unsigned long);
 #endif /* CONFIG_NFSD_V3 */
-- 
1.4.3.3.g01929

Re: [PATCH 0/1] nfsd4: reindent do_open_lookup()

[J. Bruce Fields]

Uh, [PATCH 0/1] should be [PATCH 1/2].  Some days I can count, other
days....

--b.

Fwd: [PATCH 2/2] nfsd4: fix open-create permissions

[Daniel J Blueman]

Linus, Trond,

What is the chance of this patch making it into the final 2.6.19?

WIthout it, there is a serious NFSv4 open() regression; I've been
running it on client and server for ~1 week under load and it resolves
the condition w/o side-effects. See the LKML thread "Poor NFSv4 first
impressions" for further details.

Daniel

---------- Forwarded message ----------
From: J. Bruce Fields <bfields@fieldses.org>
Date: 06-Nov-2006 16:24
Subject: [PATCH 2/2] nfsd4: fix open-create permissions
To: Daniel J Blueman <daniel.blueman@gmail.com>
Cc: nfsv4@linux-nfs.org, Linux Kernel <linux-kernel@vger.kernel.org>,
Jeff Garzik <jeff@garzik.org>, Neil Brown <neilb@suse.de>


In the case where an open creates the file, we shouldn't be rechecking
permissions to open the file; the open succeeds regardless of what the
new file's mode bits say.

This patch fixes the problem, but only by introducing yet another parameter
to nfsd_create_v3.  This is ugly.  This will be fixed by later patches.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/nfs3proc.c        |    2 +-
 fs/nfsd/nfs4proc.c        |    6 ++++--
 fs/nfsd/vfs.c             |    4 +++-
 include/linux/nfsd/nfsd.h |    2 +-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 64db601..7f5bad0 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -258,7 +258,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp
        /* Now create the file and set attributes */
        nfserr = nfsd_create_v3(rqstp, dirfhp, argp->name, argp->len,
                                attr, newfhp,
-                               argp->createmode, argp->verf, NULL);
+                               argp->createmode, argp->verf, NULL, NULL);

        RETURN_STATUS(nfserr);
 }
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4a73f5b..50bc942 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -93,6 +93,7 @@ do_open_lookup(struct svc_rqst *rqstp, s
 {
        struct svc_fh resfh;
        __be32 status;
+       int created = 0;

        fh_init(&resfh, NFS4_FHSIZE);
        open->op_truncate = 0;
@@ -105,7 +106,7 @@ do_open_lookup(struct svc_rqst *rqstp, s
                status = nfsd_create_v3(rqstp, current_fh, open->op_fname.data,
                                        open->op_fname.len, &open->op_iattr,
                                        &resfh, open->op_createmode,
-                                       (u32 *)open->op_verf.data,
&open->op_truncate);
+                                       (u32 *)open->op_verf.data,
&open->op_truncate, &created);
        } else {
                status = nfsd_lookup(rqstp, current_fh,
                                     open->op_fname.data,
open->op_fname.len, &resfh);
@@ -122,7 +123,8 @@ do_open_lookup(struct svc_rqst *rqstp, s
        memcpy(open->op_stateowner->so_replay.rp_openfh,
                        &resfh.fh_handle.fh_base, resfh.fh_handle.fh_size);

-       status = do_open_permission(rqstp, current_fh, open, MAY_NOP);
+       if (!created)
+               status = do_open_permission(rqstp, current_fh, open, MAY_NOP);

 out:
        fh_put(&resfh);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f21e917..1a7ad8c 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1237,7 +1237,7 @@ __be32
 nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp,
                char *fname, int flen, struct iattr *iap,
                struct svc_fh *resfhp, int createmode, u32 *verifier,
-               int *truncp)
+               int *truncp, int *created)
 {
        struct dentry   *dentry, *dchild = NULL;
        struct inode    *dirp;
@@ -1331,6 +1331,8 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
        host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
        if (host_err < 0)
                goto out_nfserr;
+       if (created)
+               *created = 1;

        if (EX_ISSYNC(fhp->fh_export)) {
                err = nfserrno(nfsd_sync_dir(dentry));
diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
index eb23114..edb54c3 100644
--- a/include/linux/nfsd/nfsd.h
+++ b/include/linux/nfsd/nfsd.h
@@ -89,7 +89,7 @@ __be32                nfsd_access(struct svc_rqst *, s
 __be32         nfsd_create_v3(struct svc_rqst *, struct svc_fh *,
                                char *name, int len, struct iattr *attrs,
                                struct svc_fh *res, int createmode,
-                               u32 *verifier, int *truncp);
+                               u32 *verifier, int *truncp, int *created);
 __be32         nfsd_commit(struct svc_rqst *, struct svc_fh *,
                                loff_t, unsigned long);
 #endif /* CONFIG_NFSD_V3 */
--
1.4.3.3.g01929

-- 
Daniel J Blueman

Re: Poor NFSv4 first impressions

[Pavel Machek]

> With this patch, I can run just great with NFSv4 home dir (etc)
> mounts; without, I get the symptom of many 0-byte temporary/lock files
> being created and often the inability to create files (!). Be sure to
> allow callback delegation connections in through your firewall for the
> extra performance ;-) .
> 
> Maybe it's too late for these fixes 2.6.19, but they should certainly
> make 2.6.19.1 IMHO.

If they are good enough for 2.6.19.1, they should _definitely_ go into
2.6.19.


								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Fwd: [PATCH 2/2] nfsd4: fix open-create permissions

[Jeff Garzik]

Daniel J Blueman wrote:
> Linus, Trond,
> 
> What is the chance of this patch making it into the final 2.6.19?
> 
> WIthout it, there is a serious NFSv4 open() regression; I've been
> running it on client and server for ~1 week under load and it resolves
> the condition w/o side-effects. See the LKML thread "Poor NFSv4 first
> impressions" for further details.

strong ACK, provided that someone who knows the NFSv4 server code well 
(Neil B?) gives it an ACK.

	Jeff

Re: Fwd: [PATCH 2/2] nfsd4: fix open-create permissions

[Andrew Morton]

On Wed, 08 Nov 2006 08:33:50 -0500
Jeff Garzik <jeff@garzik.org> wrote:

> Daniel J Blueman wrote:
> > Linus, Trond,
> > 
> > What is the chance of this patch making it into the final 2.6.19?
> > 
> > WIthout it, there is a serious NFSv4 open() regression; I've been
> > running it on client and server for ~1 week under load and it resolves
> > the condition w/o side-effects. See the LKML thread "Poor NFSv4 first
> > impressions" for further details.
> 
> strong ACK, provided that someone who knows the NFSv4 server code well 
> (Neil B?) gives it an ACK.
> 

Neil has acked it.  This is in my for-2.6.19 queue, probably later today.