commit 968a0f3d76dd1e59aec25077cfbe4fd47220c566
Author: Erez Zadok <[EMAIL PROTECTED]>
Date: Thu Jan 10 07:09:27 2008 -0500
Unionfs: prevent false lockdep warnings in stacking
A stackable file system like unionfs often performs an operation on a lower
file system, by calling a vfs_* method, having been called possibly by the
very same method from the VFS. Both calls to the vfs_* method grab a lock
in the same lock class, and hence lockdep complains. This warning is a
false positive in instances where unionfs only calls the vfs_* method on
lower objects; there's a strict lock ordering here: upper objects first,
then lower objects.
We want to prevent these false positives so that lockdep will not shutdown
so it'd still be able to warn us about potentially true locking problems.
So, we temporarily turn off lockdep ONLY AROUND the calls to vfs methods to
which we pass lower objects, and only for those instances where lockdep
complained. While this solution may seem unclean, it is not without
precedent: other places in the kernel also do similar temporary disabling,
of course after carefully having checked that it is the right thing to do.
In the long run, lockdep needs to be taught how to handle about stacking.
Then this patch can be removed. It is likely that such lockdep-stacking
support will do essentially the same as this patch: consider the same
ordering (upper then lower) and consider upper vs. lower locks to be in
different classes.
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
diff --git a/Documentation/filesystems/unionfs/issues.txt
b/Documentation/filesystems/unionfs/issues.txt
index bb6ab05..f4b7e7e 100644
--- a/Documentation/filesystems/unionfs/issues.txt
+++ b/Documentation/filesystems/unionfs/issues.txt
@@ -17,8 +17,12 @@ KNOWN Unionfs 2.x ISSUES:
an upper object, and then a lower object, in a strict order to avoid
locking problems; in addition, Unionfs, as a fan-out file system, may
have to lock several lower inodes. We are currently looking into Lockdep
- to see how to make it aware of stackable file systems. In the meantime,
- if you get any warnings from Lockdep, you can safely ignore them (or feel
- free to report them to the Unionfs maintainers, just to be sure).
+ to see how to make it aware of stackable file systems. For now, we
+ temporarily disable lockdep when calling vfs methods on lower objects,
+ but only for those places where lockdep complained. While this solution
+ may seem unclean, it is not without precedent: other places in the kernel
+ also do similar temporary disabling, of course after carefully having
+ checked that it is the right thing to do. Anyway, you get any warnings
+ from Lockdep, please report them to the Unionfs maintainers.
For more information, see <http://unionfs.filesystems.org/>.
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 96b3b95..33c2d33 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -297,11 +297,14 @@ static int __copyup_reg_data(struct dentry *dentry,
break;
}
+ /* see Documentation/filesystems/unionfs/issues.txt */
+ lockdep_off();
write_bytes =
output_file->f_op->write(output_file,
(char __user *)buf,
read_bytes,
&output_file->f_pos);
+ lockdep_on();
if ((write_bytes < 0) || (write_bytes < read_bytes)) {
err = write_bytes;
break;
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index b378f79..ac90756 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -205,13 +205,11 @@ static int unionfs_link(struct dentry *old_dentry, struct
inode *dir,
lower_new_dentry = unionfs_lower_dentry(new_dentry);
if (dbstart(old_dentry) > dbstart(new_dentry)) {
-
/* don't copyup if new_dentry doesn't exist in union */
if (dbstart(new_dentry) == 0 &&
dbstart(new_dentry) == dbend(new_dentry) &&
(!unionfs_lower_dentry_idx(new_dentry, 0) ||
!unionfs_lower_dentry_idx(new_dentry, 0)->d_inode)) {
-
set_dbstart(new_dentry, dbstart(old_dentry));
set_dbend(new_dentry, dbstart(old_dentry));
create_p = 1;
@@ -221,7 +219,6 @@ static int unionfs_link(struct dentry *old_dentry, struct
inode *dir,
unionfs_mntput(new_dentry, 0);
unionfs_set_lower_mnt_idx(new_dentry, 0, NULL);
}
-
} else {
err = -EROFS;
goto docopyup;
@@ -251,9 +248,13 @@ static int unionfs_link(struct dentry *old_dentry, struct
inode *dir,
BUG_ON(dbstart(old_dentry) != dbstart(new_dentry));
lower_dir_dentry = lock_parent(lower_new_dentry);
err = is_robranch(old_dentry);
- if (!err)
+ if (!err) {
+ /* see Documentation/filesystems/unionfs/issues.txt */
+ lockdep_off();
err = vfs_link(lower_old_dentry, lower_dir_dentry->d_inode,
lower_new_dentry);
+ lockdep_on();
+ }
unlock_dir(lower_dir_dentry);
docopyup:
@@ -283,10 +284,13 @@ docopyup:
lower_dir_dentry =
lock_parent(lower_new_dentry);
+ /* see Documentation/filesystems/unionfs/issues.txt */
+ lockdep_off();
/* do vfs_link */
err = vfs_link(lower_old_dentry,
lower_dir_dentry->d_inode,
lower_new_dentry);
+ lockdep_on();
unlock_dir(lower_dir_dentry);
goto check_link;
}
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 8a0a269..e0eacf1 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -99,21 +99,21 @@ static int __unionfs_rename(struct inode *old_dir, struct
dentry *old_dentry,
}
}
+ err = is_robranch_super(old_dentry->d_sb, bindex);
+ if (err)
+ goto out;
+
dget(lower_old_dentry);
lower_old_dir_dentry = dget_parent(lower_old_dentry);
lower_new_dir_dentry = dget_parent(lower_new_dentry);
+ /* see Documentation/filesystems/unionfs/issues.txt */
+ lockdep_off();
lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
-
- err = is_robranch_super(old_dentry->d_sb, bindex);
- if (err)
- goto out_unlock;
-
err = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry,
lower_new_dir_dentry->d_inode, lower_new_dentry);
-
-out_unlock:
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
+ lockdep_on();
dput(lower_old_dir_dentry);
dput(lower_new_dir_dentry);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 5df95d5..baa2e81 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -863,7 +863,11 @@ static void unionfs_clear_inode(struct inode *inode)
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (!lower_inode)
continue;
+ unionfs_set_lower_inode_idx(inode, bindex, NULL);
+ /* see Documentation/filesystems/unionfs/issues.txt */
+ lockdep_off();
iput(lower_inode);
+ lockdep_on();
}
}
diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index 4619930..b009711 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -45,10 +45,13 @@ static int unionfs_do_unlink(struct inode *dir, struct
dentry *dentry)
/* avoid destroying the lower inode if the file is in use */
dget(lower_dentry);
err = is_robranch_super(dentry->d_sb, bindex);
- if (!err)
+ if (!err) {
+ /* see Documentation/filesystems/unionfs/issues.txt */
+ lockdep_off();
err = vfs_unlink(lower_dir_dentry->d_inode,
lower_dentry);
- else
+ lockdep_on();
+ } else
err = -EROFS;
/* if vfs_unlink succeeded, update our inode's times */
if (!err)
@@ -163,10 +166,13 @@ static int unionfs_do_rmdir(struct inode *dir, struct
dentry *dentry)
/* avoid destroying the lower inode if the file is in use */
dget(lower_dentry);
err = is_robranch_super(dentry->d_sb, bindex);
- if (!err)
+ if (!err) {
+ /* see Documentation/filesystems/unionfs/issues.txt */
+ lockdep_off();
err = vfs_rmdir(lower_dir_dentry->d_inode,
lower_dentry);
- else
+ lockdep_on();
+ } else
err = -EROFS;
dput(lower_dentry);
_______________________________________________
unionfs-cvs mailing list: http://unionfs.filesystems.org/
[email protected]
http://www.fsl.cs.sunysb.edu/mailman/listinfo/unionfs-cvs
