* Re: [2.6.34-rc2 NFS4 oops] open error path failure...
@ 2010-03-29 19:03 ` Al Viro
0 siblings, 0 replies; 9+ messages in thread
From: Al Viro @ 2010-03-29 19:03 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: Trond Myklebust, linux-nfs, Linux Kernel
On Mon, Mar 29, 2010 at 07:36:45PM +0100, Daniel J Blueman wrote:
> Hi Trond,
>
> When open fails and should return EPERM [1], instead we see an oops
> [2]. I see this on 2.6.34-rc1 and -rc2 mainline; NFS4 server is
> mainline 2.6.33.1.
>
> Let me know if you can't reproduce it and I'll provide some analysis
> from this end.
Joy... ERR_PTR(-EPERM) in nd.intent.file, and whoever had called
lookup_instantiate_filp() hadn't bothered to check the return value.
OK, I think I see what's going on. Replace
lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
return 1;
with
lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
return state;
in fs/nfs/nfs4proc.c:nfs4_open_revalidate() and see if everything works
properly (or just lose the lookup_instantiate_filp() in there and simply
return state).
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [2.6.34-rc2 NFS4 oops] open error path failure...
2010-03-29 19:03 ` Al Viro
@ 2010-03-29 19:21 ` Daniel J Blueman
-1 siblings, 0 replies; 9+ messages in thread
From: Daniel J Blueman @ 2010-03-29 19:21 UTC (permalink / raw)
To: Al Viro; +Cc: Trond Myklebust, linux-nfs, Linux Kernel
Hi Al,
On Mon, Mar 29, 2010 at 8:03 PM, Al Viro <viro@zeniv.linux.org.uk> wrot=
e:
> On Mon, Mar 29, 2010 at 07:36:45PM +0100, Daniel J Blueman wrote:
>> Hi Trond,
>>
>> When open fails and should return EPERM [1], instead we see an oops
>> [2]. I see this on 2.6.34-rc1 and -rc2 mainline; NFS4 server is
>> mainline 2.6.33.1.
>>
>> Let me know if you can't reproduce it and I'll provide some analysis
>> from this end.
>
> Joy... =A0ERR_PTR(-EPERM) in nd.intent.file, and whoever had called
> lookup_instantiate_filp() hadn't bothered to check the return value.
>
> OK, I think I see what's going on. =A0Replace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lookup=
_instantiate_filp(nd, (struct dentry *)state, NULL);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return=
1;
> with
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lookup=
_instantiate_filp(nd, (struct dentry *)state, NULL);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return=
state;
> in fs/nfs/nfs4proc.c:nfs4_open_revalidate() and see if everything wor=
ks
> properly (or just lose the lookup_instantiate_filp() in there and sim=
ply
> return state).
That did the trick!
Looks like I should have reported this when I first encountered it,
but I guess it goes to show there needs to be some NFS validation
(LTP?). I'm still trying to chase down a rare NFSv4 dentry "Stale NFS
file handle" issue I've seen.
Thanks,
Daniel
--=20
Daniel J Blueman
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [2.6.34-rc2 NFS4 oops] open error path failure...
@ 2010-03-29 19:21 ` Daniel J Blueman
0 siblings, 0 replies; 9+ messages in thread
From: Daniel J Blueman @ 2010-03-29 19:21 UTC (permalink / raw)
To: Al Viro; +Cc: Trond Myklebust, linux-nfs, Linux Kernel
Hi Al,
On Mon, Mar 29, 2010 at 8:03 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Mar 29, 2010 at 07:36:45PM +0100, Daniel J Blueman wrote:
>> Hi Trond,
>>
>> When open fails and should return EPERM [1], instead we see an oops
>> [2]. I see this on 2.6.34-rc1 and -rc2 mainline; NFS4 server is
>> mainline 2.6.33.1.
>>
>> Let me know if you can't reproduce it and I'll provide some analysis
>> from this end.
>
> Joy... ERR_PTR(-EPERM) in nd.intent.file, and whoever had called
> lookup_instantiate_filp() hadn't bothered to check the return value.
>
> OK, I think I see what's going on. Replace
> lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
> return 1;
> with
> lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
> return state;
> in fs/nfs/nfs4proc.c:nfs4_open_revalidate() and see if everything works
> properly (or just lose the lookup_instantiate_filp() in there and simply
> return state).
That did the trick!
Looks like I should have reported this when I first encountered it,
but I guess it goes to show there needs to be some NFS validation
(LTP?). I'm still trying to chase down a rare NFSv4 dentry "Stale NFS
file handle" issue I've seen.
Thanks,
Daniel
--
Daniel J Blueman
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [2.6.34-rc2 NFS4 oops] open error path failure...
2010-03-29 19:03 ` Al Viro
(?)
(?)
@ 2010-03-29 21:22 ` Trond Myklebust
2010-03-31 11:20 ` Daniel J Blueman
-1 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2010-03-29 21:22 UTC (permalink / raw)
To: Al Viro; +Cc: Daniel J Blueman, linux-nfs, Linux Kernel
On Mon, 2010-03-29 at 20:03 +0100, Al Viro wrote:
> On Mon, Mar 29, 2010 at 07:36:45PM +0100, Daniel J Blueman wrote:
> > Hi Trond,
> >
> > When open fails and should return EPERM [1], instead we see an oops
> > [2]. I see this on 2.6.34-rc1 and -rc2 mainline; NFS4 server is
> > mainline 2.6.33.1.
> >
> > Let me know if you can't reproduce it and I'll provide some analysis
> > from this end.
>
> Joy... ERR_PTR(-EPERM) in nd.intent.file, and whoever had called
> lookup_instantiate_filp() hadn't bothered to check the return value.
>
> OK, I think I see what's going on. Replace
> lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
> return 1;
> with
> lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
> return state;
> in fs/nfs/nfs4proc.c:nfs4_open_revalidate() and see if everything works
> properly (or just lose the lookup_instantiate_filp() in there and simply
> return state).
>
So this raises a point. Originally, the d_revalidate() call was required
to return a boolean 0 or 1. Nowadays it allows the filesystem to return
an error value instead.
Should we therefore rewrite the NFS implementation to propagate errors
like ESTALE (when it means the parent directory is gone), EACCES, EPERM
and EIO instead of the current behaviour of just dropping the dentry and
hence forcing a lookup?
Trond
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [2.6.34-rc2 NFS4 oops] open error path failure...
2010-03-29 21:22 ` Trond Myklebust
@ 2010-03-31 11:20 ` Daniel J Blueman
2010-03-31 14:21 ` Chuck Lever
0 siblings, 1 reply; 9+ messages in thread
From: Daniel J Blueman @ 2010-03-31 11:20 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Al Viro, linux-nfs, Linux Kernel
On Mon, Mar 29, 2010 at 10:22 PM, Trond Myklebust
<trond.myklebust@fys.uio.no> wrote:
> On Mon, 2010-03-29 at 20:03 +0100, Al Viro wrote:
>> On Mon, Mar 29, 2010 at 07:36:45PM +0100, Daniel J Blueman wrote:
>> > Hi Trond,
>> >
>> > When open fails and should return EPERM [1], instead we see an oops
>> > [2]. I see this on 2.6.34-rc1 and -rc2 mainline; NFS4 server is
>> > mainline 2.6.33.1.
>> >
>> > Let me know if you can't reproduce it and I'll provide some analysis
>> > from this end.
>>
>> Joy... ERR_PTR(-EPERM) in nd.intent.file, and whoever had called
>> lookup_instantiate_filp() hadn't bothered to check the return value.
>>
>> OK, I think I see what's going on. Replace
>> lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
>> return 1;
>> with
>> lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
>> return state;
>> in fs/nfs/nfs4proc.c:nfs4_open_revalidate() and see if everything works
>> properly (or just lose the lookup_instantiate_filp() in there and simply
>> return state).
>
> So this raises a point. Originally, the d_revalidate() call was required
> to return a boolean 0 or 1. Nowadays it allows the filesystem to return
> an error value instead.
>
> Should we therefore rewrite the NFS implementation to propagate errors
> like ESTALE (when it means the parent directory is gone), EACCES, EPERM
> and EIO instead of the current behaviour of just dropping the dentry and
> hence forcing a lookup?
Passing the error back without forcing a lookup sounds like a good
win, if it can avoid a comparatively expensive roundtrip to the server
(iff the dentry is fresh enough). Is this possible?
Talking of expensive, I see latencytop show >16000ms latency for
writing pages when I have a workload that does large buffered I/O to
an otherwise uncongested server. The gigabit network is saturated, and
reads often stall for 1000-4000ms (!). Client has the default 16 TCP
request slots, and server has 8 nfsds - the server is far from disk or
processor-saturated. I'll see if there is any useful debugging I can
get about this.
Thanks,
Daniel
--
Daniel J Blueman
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [2.6.34-rc2 NFS4 oops] open error path failure...
2010-03-31 11:20 ` Daniel J Blueman
@ 2010-03-31 14:21 ` Chuck Lever
2010-04-02 10:34 ` Daniel J Blueman
0 siblings, 1 reply; 9+ messages in thread
From: Chuck Lever @ 2010-03-31 14:21 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: Trond Myklebust, Al Viro, linux-nfs, Linux Kernel
On 03/31/2010 07:20 AM, Daniel J Blueman wrote:
> Talking of expensive, I see latencytop show>16000ms latency for
> writing pages when I have a workload that does large buffered I/O to
> an otherwise uncongested server. The gigabit network is saturated, and
> reads often stall for 1000-4000ms (!). Client has the default 16 TCP
> request slots, and server has 8 nfsds - the server is far from disk or
> processor-saturated. I'll see if there is any useful debugging I can
> get about this.
That latency is pretty much guaranteed to be due to a long RPC backlog
queue on the client. Bumping the size of the slot table to 128 and
increasing the number of NFSD threads may help.
--
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [2.6.34-rc2 NFS4 oops] open error path failure...
2010-03-31 14:21 ` Chuck Lever
@ 2010-04-02 10:34 ` Daniel J Blueman
0 siblings, 0 replies; 9+ messages in thread
From: Daniel J Blueman @ 2010-04-02 10:34 UTC (permalink / raw)
To: Chuck Lever; +Cc: Trond Myklebust, Al Viro, linux-nfs, Linux Kernel
On Wed, Mar 31, 2010 at 3:21 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> On 03/31/2010 07:20 AM, Daniel J Blueman wrote:
>>
>> Talking of expensive, I see latencytop show>16000ms latency for
>> writing pages when I have a workload that does large buffered I/O to
>> an otherwise uncongested server. The gigabit network is saturated, and
>> reads often stall for 1000-4000ms (!). Client has the default 16 TCP
>> request slots, and server has 8 nfsds - the server is far from disk or
>> processor-saturated. I'll see if there is any useful debugging I can
>> get about this.
>
> That latency is pretty much guaranteed to be due to a long RPC backlog queue
> on the client. Bumping the size of the slot table to 128 and increasing the
> number of NFSD threads may help.
Increasing these values did help quite a bit, though I was still
seeing 5000-8000ms at nfs_wait_bit_uninterruptible() [1] and close().
Then again, I also was seeing 'Scheduler waiting for cpu' taking up to
3000ms! I suspect processor throttling, due to exceeding thermal
limits.
Thanks,
Daniel
--- [1]
nfs_wait_bit_uninterruptible
nfs_wait_on_request
nfs_wait_in_requests_locked
nfs_sync_mapping_wait
nfs_write_mapping
nfs_wb_nocommit
nfs_getattr
vfs_getattr
vfs_fstat
sys_newfstat
system_call_fastpath
--
Daniel J Blueman
^ permalink raw reply [flat|nested] 9+ messages in thread