Re: [pnfs] [GIT BISECT] first bad commit: 1f36f774 Switch !O_CREATcase to use of do_last()

[Boaz Harrosh]

On 03/24/2010 08:56 PM, Al Viro wrote:
> On Wed, Mar 24, 2010 at 02:26:29PM -0400, Doug Nazar wrote:
>> I also see it....didn't bisect it yet since I was in a rush and had
>> a simple work around. I was doing a git pull between two
>> repositories on a NFS 4 w/krb5 security. Got the error about
>> .git/objects/pack (error: unable to open object pack directory:
>> .git/objects/pack: Is a directory). If I then run 'ls
>> .git/objects/pack', and then the 'git pull' again it works.
> 
> Very interesting...  The damn thing *is* a directory, which should have
> made NFS skip all lookup_instantiate_filp() tricks completely.  IOW, it's
> not hitting anything intent-related at that case.
> 
> I really wonder where the hell does EISDIR come from; no matter how screwed
> the cached attributes are, there's not a lot of places where we can return
> that sucker.  It either comes from something in NFS itself, or it's
> may_open() getting MAY_WRITE on that object or it has to see O_CREAT in
> open_flag.  And acc_mode is not modified after it's set in do_filp_open().
> If that calculation (acc_mode by open_flag) would be buggered for O_RDONLY,
> we'd be seeing a lot more breakage, starting with ls(1) ;-)
> 
> One possibility is that shit hits the fan a bit earlier and git really passes
> something odd to that open() as the result of bogus stat(), etc.
> 
> Folks, could you try the following: in do_last() move case LAST_DOT:
> to immediately after follow_dotdot(nd); and see if that changes anything?
> It shouldn't, but if nfs is playing odd tricks with ->d_revalidate() for
> directories acting differently depending on LOOKUP_PARENT in flags...
> 
> IOW, replace
>         case LAST_DOTDOT:  
>                 follow_dotdot(nd);
>                 dir = nd->path.dentry;
>                 if (nd->path.mnt->mnt_sb->s_type->fs_flags & FS_REVAL_DOT) {
>                         if (!dir->d_op->d_revalidate(dir, nd)) {
>                                 error = -ESTALE;
>                                 goto exit;
>                         }
>                 }
>                 /* fallthrough */
>         case LAST_DOT:
>         case LAST_ROOT:
> with
>         case LAST_DOTDOT:  
>                 follow_dotdot(nd);
>                 /* fallthrough */
>         case LAST_DOT:
>                 dir = nd->path.dentry;
>                 if (nd->path.mnt->mnt_sb->s_type->fs_flags & FS_REVAL_DOT) {
>                         if (!dir->d_op->d_revalidate(dir, nd)) {
>                                 error = -ESTALE;
>                                 goto exit;
>                         }
>                 }
>                 /* fallthrough */
>         case LAST_ROOT:
> and see what'll change.

(Below is what I changed)

It makes no difference, fails just the same. Would an "strace" help?

I will now setup an ext2 export. exofs is an heavy ext2 copy paste in every
thing meta-data and nfs-export. I hope to strimline to the basics.

You have asked before, if the failure is reproducible. Yes it fails every
time in exactly the same way. "git status" on that tree has never succeeded
from the 2.6.34-rc2 client.

Boaz
---
diff --git a/fs/namei.c b/fs/namei.c
index 1c0fca6..e8a6a7b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1618,8 +1618,10 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	int error = -EISDIR;
 
 	switch (nd->last_type) {
-	case LAST_DOTDOT:
+	case LAST_DOTDOT:  
 		follow_dotdot(nd);
+		/* fallthrough */
+	case LAST_DOT:
 		dir = nd->path.dentry;
 		if (nd->path.mnt->mnt_sb->s_type->fs_flags & FS_REVAL_DOT) {
 			if (!dir->d_op->d_revalidate(dir, nd)) {
@@ -1628,7 +1630,6 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 			}
 		}
 		/* fallthrough */
-	case LAST_DOT:
 	case LAST_ROOT:
 		if (open_flag & O_CREAT)
 			goto exit;
@@ -1650,6 +1651,12 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 		error = do_lookup(nd, &nd->last, path);
 		if (error)
 			goto exit;
+
+		if (IS_ERR(nd->intent.open.file)) {
+			printk(KERN_ERR "foo: %s", pathname);
+			WARN_ON(1);
+		}
+
 		error = -ENOENT;
 		if (!path->dentry->d_inode)
 			goto exit_dput;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/