I am sending this patch in a separate email because it may need some discussion to hash out. Sometime in the past several months, the pvfs2_lookup() function in namei.c changed (I think along with something not directly related, but I don't recall exactly what happened now).

This change caused several directory related bugs to show up for us on 2.4 and 2.6 kernels. The 2.4 one was more severe, though, because it caused a kernel panic. It could be triggered by the "rename01" test in LTP, or by the following manual steps:

[EMAIL PROTECTED] pvfs2]# mkdir testdir
[EMAIL PROTECTED] pvfs2]# cd testdir
[EMAIL PROTECTED] testdir]# mkdir dir1
[EMAIL PROTECTED] testdir]# mv dir1 dir2
[EMAIL PROTECTED] testdir]# ls -alh
total 12K
drwxr-xr-x    1 root     root         4.0K Dec  1 12:18 .
drwxrwxrwt    1 root     root         4.0K Dec  1 12:17 ..
drwxr-xr-x    1 root     root         4.0K Dec  1 12:17 dir2
[EMAIL PROTECTED] testdir]# rm dir2
<crash>

... so it had something to do with removing a directory that had previously been renamed.

At any rate, I don't know enough about dentries and inodes anymore to truly understand the old logic that used to work or the newer logic that causes us problems. This patch just naively reverts some of the logic in namei.c to the point that it works again for us (without changing anything else that was in that set of commits). With this in place, we don't see any more test case failures or kernel panics on 2.6 or 2.4.

We have been using this patch for several months with success, but it would probably be a good idea for someone more familiar with this code to look at the change more carefully.

-Phil
Index: pvfs2_src/src/kernel/linux-2.6/namei.c
===================================================================
--- pvfs2_src/src/kernel/linux-2.6/namei.c	(revision 2909)
+++ pvfs2_src/src/kernel/linux-2.6/namei.c	(revision 2910)
@@ -164,19 +164,24 @@
 	inode = pvfs2_iget(sb, &new_op->downcall.resp.lookup.refn);
 	if (inode && !is_bad_inode(inode))
 	{
-            struct dentry *res;
+            found_pvfs2_inode = PVFS2_I(inode);
 
+            /* store the retrieved handle and fs_id */
+            found_pvfs2_inode->refn = new_op->downcall.resp.lookup.refn;
+
 	    /* update dentry/inode pair into dcache */
 	    dentry->d_op = &pvfs2_dentry_operations;
 
-            res = pvfs2_d_splice_alias(dentry, inode);
+            pvfs2_d_splice_alias(dentry, inode);
 
             gossip_debug(GOSSIP_NAME_DEBUG, "Lookup success (inode ct = %d)\n",
                         (int)atomic_read(&inode->i_count));
+#if 0
             op_release(new_op);
             if (res) 
                 res->d_op = &pvfs2_dentry_operations;
             return res;
+#endif
 	}
         else if (inode && is_bad_inode(inode))
         {
@@ -227,7 +232,14 @@
     }
 
     op_release(new_op);
-    return NULL;
+    if(ret != -ENOENT)
+    {
+        return ERR_PTR(ret);
+    }
+    else
+    {
+        return NULL;
+    }
 }
 
 /* return 0 on success; non-zero otherwise */
_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers

Reply via email to