open error path failure...
in [Kernel]
|
Prev: NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2]
Next: nvram: Convert nvram_ioctl to unlocked_ioctl, remove smp_lock.h
From: Al Viro on 29 Mar 2010 15:10 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). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Daniel J Blueman on 29 Mar 2010 15:30 Hi Al, On Mon, Mar 29, 2010 at 8:03 PM, Al Viro <viro(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Trond Myklebust on 29 Mar 2010 17:30 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Daniel J Blueman on 31 Mar 2010 07:30 On Mon, Mar 29, 2010 at 10:22 PM, Trond Myklebust <trond.myklebust(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Chuck Lever on 31 Mar 2010 10:30
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |