Bug#406902: kernel NFS data loss
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);
Reply to: