[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: Benny Halevy on 25 Mar 2010 08:20 On Mar. 25, 2010, 14:07 +0200, Benny Halevy <bhalevy(a)panasas.com> wrote: > On Mar. 25, 2010, 12:54 +0200, Al Viro <viro(a)ZenIV.linux.org.uk> wrote: >> On Thu, Mar 25, 2010 at 10:12:31AM +0000, Al Viro wrote: >>> On Thu, Mar 25, 2010 at 11:39:38AM +0200, Boaz Harrosh wrote: >>> >>>> It makes no difference, fails just the same. Would an "strace" help? >>> It might, especially if you ran it for identical repositories on local >>> fs and on NFS; at least that way it would be possible to see where do >>> they diverge... >> I wonder... What happens if you add >> if (error == -EISDIR) >> printk("blah: %s", pathname); >> right after do_lookup() call in do_last()? That would separate -EISDIR >> coming from NFS from the same thing coming from fs/namei.c... > > Bingo. > It is returned from do_lookup. 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 > > @@ -1648,6 +1654,8 @@ static struct file *do_last(struct nameidata *nd, struct path *path, > /* just plain open? */ > if (!(open_flag & O_CREAT)) { > error = do_lookup(nd, &nd->last, path); > + if (error == -EISDIR) > + printk("%s: do_lookup returned -EISDIR %s\n", __func__, pathname); > if (error) > goto exit; > error = -ENOENT; > > >> _______________________________________________ >> pNFS mailing list >> pNFS(a)linux-nfs.org >> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs > _______________________________________________ > pNFS mailing list > pNFS(a)linux-nfs.org > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs -- 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 09:10 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; 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? -- 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:10 On 03/25/2010 01:55 PM, Al Viro wrote: > On Thu, Mar 25, 2010 at 10:49:24AM +0000, Al Viro wrote: > >> Does that happen with nfsv3 or is that v4-only? I'm going to set up v4 >> server and client and see what happens, but that information could be >> useful... > > *grumble* > I'd managed to forget that rpc.imapd in its infinite wisdom relies on > the FPOS I normally disable. Oh, well... rebuilding testbox kernel > with dnotify enabled ;-/ I just wanted to make sure, on top of 2.6.34-rc2 i did: git revert 781b16775ba0bb55fac0e1757bf0bd87c8879632 Revert "Fix a dumb typo - use of & instead of &&" and: git revert 1f36f774b22a0ceb7dd33eca626746c81a97b6a5 Revert "Switch !O_CREAT case to use of do_last()" The revert is now clean and the problem does not show. Are you able to reproduce this? Anything I can do to help? Thanks 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: Boaz Harrosh on 25 Mar 2010 09:20 On 03/25/2010 03:00 PM, Boaz Harrosh wrote: > On 03/25/2010 01:55 PM, Al Viro wrote: >> On Thu, Mar 25, 2010 at 10:49:24AM +0000, Al Viro wrote: >> >>> Does that happen with nfsv3 or is that v4-only? I'm going to set up v4 >>> server and client and see what happens, but that information could be >>> useful... >> >> *grumble* >> I'd managed to forget that rpc.imapd in its infinite wisdom relies on >> the FPOS I normally disable. Oh, well... rebuilding testbox kernel >> with dnotify enabled ;-/ > > I just wanted to make sure, on top of 2.6.34-rc2 i did: > git revert 781b16775ba0bb55fac0e1757bf0bd87c8879632 > Revert "Fix a dumb typo - use of & instead of &&" > and: > git revert 1f36f774b22a0ceb7dd33eca626746c81a97b6a5 > Revert "Switch !O_CREAT case to use of do_last()" > > The revert is now clean and the problem does not show. > Are you able to reproduce this? Anything I can do to help? > I'm not sure I mentioned this. If you are able to reproduce the error, then after the error in "git status" the tree becomes unusable. Not even from the local_mount. I have not found a method to fix the tree. So what I do is: - Before I start playing I save "cp some_tree/.git/index index.back" - then after the error and tree damage On local mount I: "rm some_tree/.git/index" "cp index.back cp some_tree/.git/index" "cd some_tree; git status" this will fix it so I can try a new iteration > Thanks > 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: Al Viro on 25 Mar 2010 09:40
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? -- 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/ |