[GIT BISECT] first bad commit: 1f36f774 Switch !O_CREAT case to use of do_last()
in [Kernel]
|
Prev: [PATCH 0/5] blkdev: discard optimizations v2 RESEND
Next: [PATCH 4/5] powernow-k8: Fix frequency reporting
From: Boaz Harrosh on 25 Mar 2010 09:40 On 03/25/2010 03:06 PM, Al Viro wrote: > On Thu, Mar 25, 2010 at 02:18:56PM +0200, Benny Halevy wrote: > >> Indeed this error is coming from the server: >> >> nfsd_dispatch: vers 4 proc 1 >> nfsv4 compound op #1/7: 22 (OP_PUTFH) >> nfsd: fh_verify(16: 01010001 00000000 000e6592 345b9f25 00000000 00000000) >> nfsv4 compound op ffff880076734078 opcnt 7 #1: 22: status 0 >> nfsv4 compound op #2/7: 32 (OP_SAVEFH) >> nfsv4 compound op ffff880076734078 opcnt 7 #2: 32: status 0 >> nfsv4 compound op #3/7: 18 (OP_OPEN) >> NFSD: nfsd4_open filename pack op_stateowner (null) >> renewing client (clientid 4bab503e/00000002) >> nfsd: nfsd_lookup(fh 16: 01010001 00000000 000e6592 345b9f25 00000000 00000000, pack) >> nfsd: fh_verify(16: 01010001 00000000 000e6592 345b9f25 00000000 00000000) >> nfsd: fh_compose(exp 08:05/106497 objects/pack, ino=943508) >> nfsd: fh_verify(16: 01010001 00000000 000e6594 345b9f26 00000000 00000000) >> nfsv4 compound op ffff880076734078 opcnt 7 #3: 18: status 21 >> nfsv4 compound returned 21 > > Ho-hum... So it hits the "let's try to open it atomically" path and > gets told to FOAD by server (as it should, of course). > > And if we see different behaviour after ls -l, presumably that's a > difference between ->lookup() and ->d_revalidate() paths on client... > > OK, I think I see what's going on in this case. However, it doesn't > explain everything; my current theory is that we used to get LOOKUP_DIRECTORY > on the last components in O_DIRECTORY opens and we don't do that now. > That used to derail the is_atomic_open(), now it's hit and there we go. > > It's not hard to verify (and it might take care of this testcase), but > I still have questions about the way this code used to work *without* > O_DIRECTORY. > > Let's try this: before do_lookup() call there add > if (*want_dir) > nd->flags |= LOOKUP_DIRECTORY; Yes this fixes it!! 2.6.34-rc2 plus above, now works, horay. (diff attached) > and see how does it behave. > > However, even if it does help, it doesn't explain everything. Normal > open() on a directory without O_DIRECTORY if flags shouldn't fail with > -EISDIR. How did that manage to avoid it all along? --- diff --git a/fs/namei.c b/fs/namei.c index 1c0fca6..434ad2a 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1647,6 +1647,8 @@ static struct file *do_last(struct nameidata *nd, struct path *path, /* just plain open? */ if (!(open_flag & O_CREAT)) { + if (*want_dir) + nd->flags |= LOOKUP_DIRECTORY; error = do_lookup(nd, &nd->last, path); if (error) goto exit; -- 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: Boaz Harrosh on 25 Mar 2010 09:50 On 03/25/2010 03:37 PM, Al Viro wrote: > On Thu, Mar 25, 2010 at 03:30:22PM +0200, Boaz Harrosh wrote: >>> Let's try this: before do_lookup() call there add >>> if (*want_dir) >>> nd->flags |= LOOKUP_DIRECTORY; >> >> Yes this fixes it!! >> 2.6.34-rc2 plus above, now works, horay. (diff attached) >> >>> and see how does it behave. >>> >>> However, even if it does help, it doesn't explain everything. Normal >>> open() on a directory without O_DIRECTORY if flags shouldn't fail with >>> -EISDIR. How did that manage to avoid it all along? > > Does open() of directory _without_ O_DIRECTORY work in e.g. vanilla 2.6.33? > It certainly does for local filesystems and it does for NFSv3; does it work > for NFSv4? In my tests. Every thing is the same safe the client with the above change. So I guess NFSv4 does something different when asked for directory lookup as opposed to files lookup. I guess there is something added/removed to the compound depending on that flag. But I wouldn't know, I am not familiar with this code. NFSv4 someone? Boaz -- 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: Benny Halevy on 25 Mar 2010 10:00 On Mar. 25, 2010, 15:37 +0200, Al Viro <viro(a)ZenIV.linux.org.uk> wrote: > On Thu, Mar 25, 2010 at 03:30:22PM +0200, Boaz Harrosh wrote: >>> Let's try this: before do_lookup() call there add >>> if (*want_dir) >>> nd->flags |= LOOKUP_DIRECTORY; >> Yes this fixes it!! >> 2.6.34-rc2 plus above, now works, horay. (diff attached) >> >>> and see how does it behave. >>> >>> However, even if it does help, it doesn't explain everything. Normal >>> open() on a directory without O_DIRECTORY if flags shouldn't fail with >>> -EISDIR. How did that manage to avoid it all along? > > Does open() of directory _without_ O_DIRECTORY work in e.g. vanilla 2.6.33? > It certainly does for local filesystems and it does for NFSv3; does it work > for NFSv4? No, it doesn't. # mount localhost:/usr0/nfs4export /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost open("/mnt/localhost/server", O_RDONLY) = 3 # mount -t nfs4 localhost:/ /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost open("/mnt/localhost/server", O_RDONLY) = -1 EISDIR (Is a directory) -- 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: Al Viro on 25 Mar 2010 10:10 On Thu, Mar 25, 2010 at 03:52:25PM +0200, Benny Halevy wrote: > On Mar. 25, 2010, 15:37 +0200, Al Viro <viro(a)ZenIV.linux.org.uk> wrote: > > On Thu, Mar 25, 2010 at 03:30:22PM +0200, Boaz Harrosh wrote: > >>> Let's try this: before do_lookup() call there add > >>> if (*want_dir) > >>> nd->flags |= LOOKUP_DIRECTORY; > >> Yes this fixes it!! > >> 2.6.34-rc2 plus above, now works, horay. (diff attached) > >> > >>> and see how does it behave. > >>> > >>> However, even if it does help, it doesn't explain everything. Normal > >>> open() on a directory without O_DIRECTORY if flags shouldn't fail with > >>> -EISDIR. How did that manage to avoid it all along? > > > > Does open() of directory _without_ O_DIRECTORY work in e.g. vanilla 2.6.33? > > It certainly does for local filesystems and it does for NFSv3; does it work > > for NFSv4? > > No, it doesn't. > > # mount localhost:/usr0/nfs4export /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost > open("/mnt/localhost/server", O_RDONLY) = 3 > > # mount -t nfs4 localhost:/ /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost > open("/mnt/localhost/server", O_RDONLY) = -1 EISDIR (Is a directory) Gets better - if you do ls -l /mnt/localhost/server and then repeat that open(), it'll succeed. -- 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: Benny Halevy on 25 Mar 2010 10:10
On Mar. 25, 2010, 16:06 +0200, Al Viro <viro(a)ZenIV.linux.org.uk> wrote: > On Thu, Mar 25, 2010 at 03:52:25PM +0200, Benny Halevy wrote: >> On Mar. 25, 2010, 15:37 +0200, Al Viro <viro(a)ZenIV.linux.org.uk> wrote: >>> On Thu, Mar 25, 2010 at 03:30:22PM +0200, Boaz Harrosh wrote: >>>>> Let's try this: before do_lookup() call there add >>>>> if (*want_dir) >>>>> nd->flags |= LOOKUP_DIRECTORY; >>>> Yes this fixes it!! >>>> 2.6.34-rc2 plus above, now works, horay. (diff attached) >>>> >>>>> and see how does it behave. >>>>> >>>>> However, even if it does help, it doesn't explain everything. Normal >>>>> open() on a directory without O_DIRECTORY if flags shouldn't fail with >>>>> -EISDIR. How did that manage to avoid it all along? >>> Does open() of directory _without_ O_DIRECTORY work in e.g. vanilla 2.6.33? >>> It certainly does for local filesystems and it does for NFSv3; does it work >>> for NFSv4? >> No, it doesn't. >> >> # mount localhost:/usr0/nfs4export /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost >> open("/mnt/localhost/server", O_RDONLY) = 3 >> >> # mount -t nfs4 localhost:/ /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost >> open("/mnt/localhost/server", O_RDONLY) = -1 EISDIR (Is a directory) > > Gets better - if you do ls -l /mnt/localhost/server and then repeat that > open(), it'll succeed. Correct :) -- 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/ |