Bug#406902: kernel NFS data loss

[Paul Szabo <psz@maths.usyd.edu.au>]

I ran some tests today, and it seemed (but not conclusive) that the
problem only occurs when the client is a multi-CPU SMP machine. Looking
at kernel source code, I noticed what I thought were oddities.

Do you think the following "patch" against 2.6.8-16sarge7 code would be
useful? I have not yet tested whether this works at all, or whether it
improves anything.

Thanks,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia


--- fs/nfs/dir.c.bak	2004-08-14 15:36:58.000000000 +1000
+++ fs/nfs/dir.c	2007-07-20 13:17:33.387030060 +1000
@@ -681,14 +681,15 @@
  */
 static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
 {
+	/* PSz 20 Jul 07  Do not we need lock_kernel() for nfs_renew_times()? */
+	lock_kernel();
 	if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
-		lock_kernel();
 		inode->i_nlink--;
 		nfs_complete_unlink(dentry);
-		unlock_kernel();
 	}
 	/* When creating a negative dentry, we want to renew d_time */
 	nfs_renew_times(dentry);
+	unlock_kernel();
 	iput(inode);
 }
 
@@ -832,9 +833,12 @@
 		}
 	}
 no_entry:
+	/* PSz 20 Jul 07  Do not we need lock_kernel() for d_add() and nfs_renew_times()? */
+	lock_kernel();
 	d_add(dentry, inode);
 	nfs_renew_times(dentry);
 	nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
+	unlock_kernel();
 out:
 	BUG_ON(error > 0);
 	return ERR_PTR(error);
@@ -882,8 +886,12 @@
 	unlock_kernel();
 out:
 	dput(parent);
-	if (!ret)
+	if (!ret) {
+		/* PSz 20 Jul 07  Do not we need lock_kernel() for d_drop()? */
+		lock_kernel();
 		d_drop(dentry);
+		unlock_kernel();
+	}
 	return ret;
 no_open:
 	dput(parent);
@@ -990,6 +998,7 @@
 	}
 	inode = nfs_fhget(dentry->d_sb, fhandle, fattr);
 	if (inode) {
+/* PSz 20 Jul 07  The whole nfs_instantiate() is only ever called within lock_kernel() */
 		d_instantiate(dentry, inode);
 		nfs_renew_times(dentry);
 		nfs_set_verifier(dentry, nfs_save_change_attribute(dentry->d_parent->d_inode));
@@ -1200,6 +1209,7 @@
 				dir, &qsilly);
 	nfs_end_data_update(dir);
 	if (!error) {
+/* PSz 20 Jul 07  The whole nfs_sillyrename() is only ever called within lock_kernel() */
 		nfs_renew_times(dentry);
 		nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
 		d_move(dentry, sdentry);