2006-11-06 06:15:25

by Jeff Garzik

[permalink] [raw]
Subject: Poor NFSv4 first impressions

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



2006-11-06 12:03:57

by Daniel J Blueman

[permalink] [raw]
Subject: Re: Poor NFSv4 first impressions

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

2006-11-06 16:07:40

by Bill Davidsen

[permalink] [raw]
Subject: Re: Poor NFSv4 first impressions

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 <[email protected]>
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.

2006-11-06 16:17:54

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Poor NFSv4 first impressions

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.

2006-11-06 16:26:16

by J. Bruce Fields

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

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

--b.

2006-11-06 16:23:49

by J. Bruce Fields

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

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

Signed-off-by: J. Bruce Fields <[email protected]>
---
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

2006-11-06 16:25:00

by J. Bruce Fields

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

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 <[email protected]>
---
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

2006-11-06 16:48:27

by Daniel J Blueman

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

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 <[email protected]>
Date: 06-Nov-2006 16:24
Subject: [PATCH 2/2] nfsd4: fix open-create permissions
To: Daniel J Blueman <[email protected]>
Cc: [email protected], Linux Kernel <[email protected]>,
Jeff Garzik <[email protected]>, Neil Brown <[email protected]>


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 <[email protected]>
---
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

2006-11-07 07:20:15

by Pavel Machek

[permalink] [raw]
Subject: Re: Poor NFSv4 first impressions

> 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

2006-11-08 13:34:05

by Jeff Garzik

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

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



2006-11-08 18:07:10

by Andrew Morton

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

On Wed, 08 Nov 2006 08:33:50 -0500
Jeff Garzik <[email protected]> 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.