[GIT PULL -mm] Unionfs branch management code

[GIT PULL -mm] Unionfs branch management code

[Josef 'Jeff' Sipek]

The following patches introduce new branch-management code into Unionfs as
well as fix a number of stability issues and resource leaks.  For detailed
announcement, see end of this email.

As before, there is a git repo at:

git://git.kernel.org/pub/scm/linux/kernel/git/jsipek/unionfs.git

(master.kernel.org:/pub/scm/linux/kernel/git/jsipek/unionfs.git)

There are 21 new commits:

	Unionfs: Don't inline do_remount_{add,del,mode}_option
	Unionfs: Added several BUG_ONs to assert dentry validity
	Unionfs: Properly handle stale inodes passed to unionfs_permission
	Unionfs: Pass lowernd to lower ->revalidate function
	Unionfs: vfsmount reference counting fixes
	Unionfs: unionfs_create needs to revalidate the dentry
	Unionfs: Decrement totalopens counter on error in unionfs_open
	Unionfs: Document unionfs_d_release locking
	Unionfs: Remove the older incgen ioctl
	Unionfs: Grab the unionfs sb private data lock around branch info users
	Unionfs: Rewrite unionfs_d_revalidate
	Unionfs: Introduce unionfs_mnt{get,put}
	Unionfs: Bulk of branch-management remount code
	Unionfs: Introduce branch-id code
	Unionfs: Actually verify if dentry's info node is locked
	Unionfs: Provide more helpful info on branch leaks during unmount
	Unionfs: Rename unionfs_data sbcount field to more appropriate open_files
	Unionfs: Proper comment on rwsem field
	Unionfs: Documentation updates for branch-management
	fs: Export drop_pagecache_sb symbol
	fs: Introduce path{get,put}

Thanks,

Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>

---

Announcement:

We're pleased to announce that we merged new branch-management code into
Unionfs 2.0.  Using this code, you can add, remove, and/or change the mode
of branches.  We allow multiple branch-management operations at the same
time and they are applied atomically.

The new branch-management interface uses the remount functionality.  It is
NOT compatible with the older Unionfs 1.x branch-management interface, which
used ioctl()s.  The older interface was not atomic for multiple operations,
and had a fundamental consistency problem with open files being revalidated.
The new remount-based branch-management interface fixes all of that, and
also saves users from having to use another utility (unionctl) to control
their branches: you can now use plain /sbin/mount for all your
branch-management commands.  Additionally, we now allow the entire union to
be remounted read-only or read-write.  Here are a few examples of how to use
the new interface:

To delete a branch /foo, regardless where it is in the current union:

# mount -t unionfs -o remount,del=/foo none MOUNTPOINT

To insert (add) a branch /foo before /bar:

# mount -t unionfs -o remount,add=/bar:/foo none MOUNTPOINT

To insert (add) a branch /foo (in "rw" mode) at the very beginning (i.e., a
new highest-priority branch), you can use the above syntax, or use a short
hand version as follows:

# mount -t unionfs -o remount,add=/foo none MOUNTPOINT

To change the mode of one existing branch, say /foo, from read-only to
read-write, and change /bar from read-write to read-only:

# mount -t unionfs -o remount,mode=/foo=rw,mode=/bar=ro none MOUNTPOINT


The above are just a few examples.  You can mix and match the branch
management commands.  For more documentation and examples, see the following
file in the latest patch:

	Documentation/filesystems/unionfs/usage.txt

Finally, the patch below includes bug fixes to a number of bugs we've
discovered during our thorough testing of the branch-management code.  One
of the challenges in branch management is that it can change the composition
of a live, running file system *while* it has open files and cached objects.
We tested the code as much as possible to ensure that even under intense
multi-threaded workloads, generation number and stale cache entries are
handled properly.

[PATCH 01/21] fs: Introduce path{get,put}

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezkcs.sunysb.edu>

Export drop_pagecache_sb symbol (for branch-management).

Signed-off-by: Erez Zadok <ezkcs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipekcs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 include/linux/namei.h |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/linux/namei.h b/include/linux/namei.h
index 941be96..92b422b 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -3,6 +3,7 @@
 
 #include <linux/dcache.h>
 #include <linux/linkage.h>
+#include <linux/mount.h>
 
 struct vfsmount;
 
@@ -107,4 +108,16 @@ static inline char *nd_get_link(struct nameidata *nd)
 	return nd->saved_names[nd->depth];
 }
 
+static inline void pathget(struct path *path)
+{
+	mntget(path->mnt);
+	dget(path->dentry);
+}
+
+static inline void pathput(struct path *path)
+{
+	dput(path->dentry);
+	mntput(path->mnt);
+}
+
 #endif /* _LINUX_NAMEI_H */
-- 
1.5.0.3.268.g3dda

[PATCH 02/21] fs: Export drop_pagecache_sb symbol

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezkcs.sunysb.edu>

Signed-off-by: Erez Zadok <ezkcs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipekcs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/drop_caches.c   |    4 +++-
 include/linux/mm.h |    1 +
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 03ea769..6a7aa05 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -3,6 +3,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/fs.h>
 #include <linux/writeback.h>
@@ -12,7 +13,7 @@
 /* A global variable is a bit ugly, but it keeps the code simple */
 int sysctl_drop_caches;
 
-static void drop_pagecache_sb(struct super_block *sb)
+void drop_pagecache_sb(struct super_block *sb)
 {
 	struct inode *inode;
 
@@ -24,6 +25,7 @@ static void drop_pagecache_sb(struct super_block *sb)
 	}
 	spin_unlock(&inode_lock);
 }
+EXPORT_SYMBOL(drop_pagecache_sb);
 
 void drop_pagecache(void)
 {
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 60e0e4a..c680669 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1157,6 +1157,7 @@ int drop_caches_sysctl_handler(struct ctl_table *, int, struct file *,
 					void __user *, size_t *, loff_t *);
 unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask,
 			unsigned long lru_pages);
+extern void drop_pagecache_sb(struct super_block *);
 void drop_pagecache(void);
 void drop_slab(void);
 
-- 
1.5.0.3.268.g3dda

[PATCH 03/21] Unionfs: Documentation updates for branch-management

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Describe dynamic branch-management introduced by subsequent patches.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 Documentation/filesystems/unionfs/concepts.txt |    5 ++
 Documentation/filesystems/unionfs/issues.txt   |   22 +++++++-
 Documentation/filesystems/unionfs/usage.txt    |   61 +++++++++++++++++++++++-
 3 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/Documentation/filesystems/unionfs/concepts.txt b/Documentation/filesystems/unionfs/concepts.txt
index d417576..83d45b9 100644
--- a/Documentation/filesystems/unionfs/concepts.txt
+++ b/Documentation/filesystems/unionfs/concepts.txt
@@ -1,3 +1,6 @@
+Unionfs 2.0 CONCEPTS:
+=====================
+
 This file describes the concepts needed by a namespace unification file
 system.
 
@@ -68,3 +71,5 @@ though to this copy. The copy must be made on a higher priority branch so
 that lookup and readdir return this newer "version" of the file rather than
 the original (see duplicate elimination).
 
+
+For more information, see <http://unionfs.filesystems.org/>.
diff --git a/Documentation/filesystems/unionfs/issues.txt b/Documentation/filesystems/unionfs/issues.txt
index b070175..a434fee 100644
--- a/Documentation/filesystems/unionfs/issues.txt
+++ b/Documentation/filesystems/unionfs/issues.txt
@@ -1,5 +1,5 @@
-KNOWN Unionfs ISSUES:
-=====================
+KNOWN Unionfs 2.0 ISSUES:
+=========================
 
 1. The NFS server returns -EACCES for read-only exports, instead of -EROFS.
    This means we can't reliably detect a read-only NFS export.
@@ -14,10 +14,26 @@ KNOWN Unionfs ISSUES:
    introducing small VFS/MM changes that would allow a file system to handle
    cache coherency correctly.
 
+   Unionfs 2.0 has a temporary workaround for this.  You can force Unionfs
+   to increase the superblock generation number, and hence purge all cached
+   Unionfs objects, which would then  be re-gotten from the lower branches.
+   This should ensure cache consistency.  To increase the generation number,
+   executed the command:
+
+	mount -t unionfs -o remount,incgen none MOUNTPOINT
+
+   Note that the older way of incrementing the generation number using an
+   ioctl, is no longer supported in Unionfs 2.0.  Ioctls in general are not
+   encouraged.  Plus, an ioctl is per-file concept, whereas the generation
+   number is a per-file-system concept.  Worse, such an ioctl requires an
+   open file, which then has to be invalidated by the very nature of the
+   generation number increase (read: the old generation increase ioctl was
+   pretty racy).
+
 3. Unionfs should not use lookup_one_len() on the underlying f/s as it
    confuses NFS.  Currently, unionfs_lookup() passes lookup intents to the
    lower file-system, this eliminates part of the problem.  The remaining
    calls to lookup_one_len may need to be changed to pass an intent.
 
 
-For more information, see <unionfs.filesystems.org>.
+For more information, see <http://unionfs.filesystems.org/>.
diff --git a/Documentation/filesystems/unionfs/usage.txt b/Documentation/filesystems/unionfs/usage.txt
index 14e0856..13fbcea 100644
--- a/Documentation/filesystems/unionfs/usage.txt
+++ b/Documentation/filesystems/unionfs/usage.txt
@@ -28,4 +28,63 @@ Example:
 	dirs=/writable_branch=rw:/read-only_branch=ro
 
 
-For more information, see unionfs.filesystems.org.
+DYNAMIC BRANCH MANAGEMENT AND REMOUNTS
+======================================
+
+You can remount a union and change its overall mode, or reconfigure the
+branches, as follows.
+
+To downgrade a union from read-write to read-only:
+
+# mount -t unionfs -o remount,ro none MOUNTPOINT
+
+To upgrade a union from read-only to read-write:
+
+# mount -t unionfs -o remount,rw none MOUNTPOINT
+
+To delete a branch /foo, regardless where it is in the current union:
+
+# mount -t unionfs -o del=/foo none MOUNTPOINT
+
+To insert (add) a branch /foo before /bar:
+
+# mount -t unionfs -o remount,add=/bar:/foo none MOUNTPOINT
+
+To insert (add) a branch /foo (with the "rw" mode flag) before /bar:
+
+# mount -t unionfs -o remount,add=/bar:/foo=rw none MOUNTPOINT
+
+To insert (add) a branch /foo (in "rw" mode) at the very beginning (i.e., a
+new highest-priority branch), you can use the above syntax, or use a short
+hand version as follows:
+
+# mount -t unionfs -o remount,add=/foo none MOUNTPOINT
+
+To append a branch to the very end (new lowest-priority branch):
+
+# mount -t unionfs -o remount,add=:/foo none MOUNTPOINT
+
+To append a branch to the very end (new lowest-priority branch), in
+read-only mode:
+
+# mount -t unionfs -o remount,add=:/foo:ro none MOUNTPOINT
+
+Finally, to change the mode of one existing branch, say /foo, from read-only
+to read-write, and change /bar from read-write to read-only:
+
+# mount -t unionfs -o remount,mode=/foo=rw,mode=/bar=ro none MOUNTPOINT
+
+
+CACHE CONSISTENCY
+=================
+
+If you modify any file on any of the lower branches directly, while there is
+a Unionfs 2.0 mounted above any of those branches, you should tell Unionfs
+to purge its caches and re-get the objects.  To do that, you have to
+incremenet the generation number of the superblock using the following
+command:
+
+# mount -t unionfs -o remount,remount,incgen none MOUNTPOINT
+
+
+For more information, see <http://unionfs.filesystems.org/>.
-- 
1.5.0.3.268.g3dda

[PATCH 04/21] Unionfs: Proper comment on rwsem field

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/union.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index bae3c76..5676394 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -130,7 +130,7 @@ struct unionfs_sb_info {
 	int bend;
 
 	atomic_t generation;
-	struct rw_semaphore rwsem;
+	struct rw_semaphore rwsem; /* protects access to data+id fields */
 
 	struct unionfs_data *data;
 };
-- 
1.5.0.3.268.g3dda

[PATCH 05/21] Unionfs: Rename unionfs_data sbcount field to more appropriate open_files

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/fanout.h |    8 ++++----
 fs/unionfs/union.h  |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index e2acb75..3d1dd4c 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -106,22 +106,22 @@ static inline void unionfs_set_lower_super(struct super_block *sb, struct super_
 /* Branch count macros. */
 static inline int branch_count(const struct super_block *sb, int index)
 {
-	return atomic_read(&UNIONFS_SB(sb)->data[index].sbcount);
+	return atomic_read(&UNIONFS_SB(sb)->data[index].open_files);
 }
 
 static inline void set_branch_count(struct super_block *sb, int index, int val)
 {
-	atomic_set(&UNIONFS_SB(sb)->data[index].sbcount, val);
+	atomic_set(&UNIONFS_SB(sb)->data[index].open_files, val);
 }
 
 static inline void branchget(struct super_block *sb, int index)
 {
-	atomic_inc(&UNIONFS_SB(sb)->data[index].sbcount);
+	atomic_inc(&UNIONFS_SB(sb)->data[index].open_files);
 }
 
 static inline void branchput(struct super_block *sb, int index)
 {
-	atomic_dec(&UNIONFS_SB(sb)->data[index].sbcount);
+	atomic_dec(&UNIONFS_SB(sb)->data[index].open_files);
 }
 
 /* Dentry macros */
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 5676394..df9b8c0 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -121,7 +121,7 @@ struct unionfs_dentry_info {
 /* These are the pointers to our various objects. */
 struct unionfs_data {
 	struct super_block *sb;
-	atomic_t sbcount;
+	atomic_t open_files;	/* number of open files on branch */
 	int branchperms;
 };
 
-- 
1.5.0.3.268.g3dda

[PATCH 06/21] Unionfs: Provide more helpful info on branch leaks during unmount

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
[jsipek: no need to take a read lock on the superblock private data]
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/super.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 571b589..037c47d 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -97,6 +97,7 @@ static void unionfs_put_super(struct super_block *sb)
 {
 	int bindex, bstart, bend;
 	struct unionfs_sb_info *spd;
+	int leaks = 0;
 
 	spd = UNIONFS_SB(sb);
 	if (!spd)
@@ -107,7 +108,12 @@ static void unionfs_put_super(struct super_block *sb)
 
 	/* Make sure we have no leaks of branchget/branchput. */
 	for (bindex = bstart; bindex <= bend; bindex++)
-		BUG_ON(branch_count(sb, bindex) != 0);
+		if (branch_count(sb, bindex) != 0) {
+			printk("unionfs: branch %d has %d references left!\n",
+			       bindex, branch_count(sb,bindex));
+			leaks = 1;
+		}
+	BUG_ON(leaks != 0);
 
 	kfree(spd->data);
 	kfree(spd);
-- 
1.5.0.3.268.g3dda

[PATCH 07/21] Unionfs: Actually verify if dentry's info node is locked

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/fanout.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 3d1dd4c..e8c0fee 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -205,6 +205,7 @@ static inline void unionfs_unlock_dentry(struct dentry *d)
 
 static inline void verify_locked(struct dentry *d)
 {
+	BUG_ON(!mutex_is_locked(&UNIONFS_D(d)->lock));
 }
 
 #endif	/* _FANOUT_H */
-- 
1.5.0.3.268.g3dda

[PATCH 08/21] Unionfs: Introduce branch-id code

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Each branch gets a unique ID, which helps during branch additions,
deletions, and changes, to locate where branches were moved to, and perform
proper reference-counting.  This is useful even if the same directory was
added more than once to union.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |   62 +++++++++++++++++++++++++++++++++++++++++++++-
 fs/unionfs/fanout.h     |   22 ++++++++++++++++-
 fs/unionfs/main.c       |    1 +
 fs/unionfs/union.h      |    4 ++-
 4 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index aa7c75d..8d0f8d1 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -86,6 +86,31 @@ out:
 	return err;
 }
 
+/*
+ * Find new index of matching branch with an open file, since branches could
+ * have been added/deleted causing the one with open files to shift.
+ *
+ * @file: current file whose branches may have changed
+ * @bindex: index of branch within current file (could be old branch)
+ * @new_sb: the new superblock which may have new branch IDs
+ * Returns index of newly found branch (0 or greater), -1 otherwise.
+ */
+static int find_new_branch_index(struct file *file, int bindex,
+				 struct super_block *new_sb)
+{
+	int old_branch_id = UNIONFS_F(file)->saved_branch_ids[bindex];
+	int i;
+
+	for (i = 0; i < sbmax(new_sb); i++)
+		if (old_branch_id == branch_id(new_sb, i))
+			return i;
+	/*
+	 * XXX: maybe we should BUG_ON if not found new branch index?
+	 * (really that should never happen).
+	 */
+	return -1;
+}
+
 /* put all references held by upper struct file and free lower file pointer
  * array
  */
@@ -93,6 +118,7 @@ static void cleanup_file(struct file *file)
 {
 	int bindex, bstart, bend;
 	struct file **lf;
+	struct super_block *sb = file->f_dentry->d_sb;
 
 	lf = UNIONFS_F(file)->lower_files;
 	bstart = fbstart(file);
@@ -100,13 +126,29 @@ static void cleanup_file(struct file *file)
 
 	for (bindex = bstart; bindex <= bend; bindex++) {
 		if (unionfs_lower_file_idx(file, bindex)) {
-			branchput(file->f_dentry->d_sb, bindex);
+			int i;	/* holds (possibly) updated branch index */
+			i = find_new_branch_index(file, bindex, sb);
+			if (i < 0)
+				printk(KERN_ERR "unionfs: no supberlock for file %p\n",
+				       file);
+			else {
+				unionfs_read_lock(sb);
+				branchput(sb, i);
+				unionfs_read_unlock(sb);
+				/* XXX: is it correct to use sb->s_root here? */
+				unionfs_mntput(sb->s_root, i);
+				/* XXX: mntget b/c fput below will call mntput */
+				unionfs_mntget(sb->s_root, bindex);
+			}
 			fput(unionfs_lower_file_idx(file, bindex));
 		}
 	}
 
 	UNIONFS_F(file)->lower_files = NULL;
 	kfree(lf);
+	kfree(UNIONFS_F(file)->saved_branch_ids);
+	/* set to NULL because caller needs to know if to kfree on error */
+	UNIONFS_F(file)->saved_branch_ids = NULL;
 }
 
 /* open all lower files for a given file */
@@ -270,6 +312,12 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
 			err = -ENOMEM;
 			goto out;
 		}
+		size = sizeof(int) * sbmax(sb);
+		UNIONFS_F(file)->saved_branch_ids = kzalloc(size, GFP_KERNEL);
+		if (!UNIONFS_F(file)->saved_branch_ids) {
+			err = -ENOMEM;
+			goto out;
+		}
 
 		if (S_ISDIR(dentry->d_inode->i_mode)) {
 			/* We need to open all the files. */
@@ -297,8 +345,10 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
 	}
 
 out:
-	if (err)
+	if (err) {
 		kfree(UNIONFS_F(file)->lower_files);
+		kfree(UNIONFS_F(file)->saved_branch_ids);
+	}
 out_nofree:
 	unionfs_unlock_dentry(dentry);
 	unionfs_read_unlock(dentry->d_sb);
@@ -418,6 +468,12 @@ int unionfs_open(struct inode *inode, struct file *file)
 		err = -ENOMEM;
 		goto out;
 	}
+	size = sizeof(int) * sbmax(inode->i_sb);
+	UNIONFS_F(file)->saved_branch_ids = kzalloc(size, GFP_KERNEL);
+	if (!UNIONFS_F(file)->saved_branch_ids) {
+		err = -ENOMEM;
+		goto out;
+	}
 
 	dentry = file->f_dentry;
 	unionfs_lock_dentry(dentry);
@@ -456,6 +512,7 @@ int unionfs_open(struct inode *inode, struct file *file)
 out:
 	if (err) {
 		kfree(UNIONFS_F(file)->lower_files);
+		kfree(UNIONFS_F(file)->saved_branch_ids);
 		kfree(UNIONFS_F(file));
 	}
 out_nofree:
@@ -487,6 +544,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 		}
 	}
 	kfree(fileinfo->lower_files);
+	kfree(fileinfo->saved_branch_ids);
 
 	if (fileinfo->rdstate) {
 		fileinfo->rdstate->access = jiffies;
diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index e8c0fee..9e4a35f 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -32,12 +32,29 @@ static inline struct unionfs_inode_info *UNIONFS_I(const struct inode *inode)
 #define sbstart(sb) 0
 #define sbend(sb) (UNIONFS_SB(sb)->bend)
 #define sbmax(sb) (UNIONFS_SB(sb)->bend + 1)
+#define sbhbid(sb) (UNIONFS_SB(sb)->high_branch_id)
 
 /* File to private Data */
 #define UNIONFS_F(file) ((struct unionfs_file_info *)((file)->private_data))
 #define fbstart(file) (UNIONFS_F(file)->bstart)
 #define fbend(file) (UNIONFS_F(file)->bend)
 
+/* macros to manipulate branch IDs in stored in our superblock */
+static inline int branch_id(struct super_block *sb, int index)
+{
+	return UNIONFS_SB(sb)->data[index].branch_id;
+}
+
+static inline void set_branch_id(struct super_block *sb, int index, int val)
+{
+	UNIONFS_SB(sb)->data[index].branch_id = val;
+}
+
+static inline void new_branch_id(struct super_block *sb, int index)
+{
+	set_branch_id(sb, index, ++UNIONFS_SB(sb)->high_branch_id);
+}
+
 /* File to lower file. */
 static inline struct file *unionfs_lower_file(const struct file *f)
 {
@@ -52,11 +69,14 @@ static inline struct file *unionfs_lower_file_idx(const struct file *f, int inde
 static inline void unionfs_set_lower_file_idx(struct file *f, int index, struct file *val)
 {
 	UNIONFS_F(f)->lower_files[index] = val;
+	/* save branch ID (may be redundant?) */
+	UNIONFS_F(f)->saved_branch_ids[index] =
+		branch_id((f)->f_dentry->d_sb, index);
 }
 
 static inline void unionfs_set_lower_file(struct file *f, struct file *val)
 {
-	UNIONFS_F(f)->lower_files[fbstart(f)] = val;
+	unionfs_set_lower_file_idx((f), fbstart(f), (val));
 }
 
 /* Inode to lower inode. */
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index a37916d..ed264c7 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -515,6 +515,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
 	UNIONFS_SB(sb)->bend = -1;
 	atomic_set(&UNIONFS_SB(sb)->generation, 1);
 	init_rwsem(&UNIONFS_SB(sb)->rwsem);
+	UNIONFS_SB(sb)->high_branch_id = -1; /* -1 == invalid branch ID */
 
 	hidden_root_info = unionfs_parse_options(sb, raw_data);
 	if (IS_ERR(hidden_root_info)) {
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index df9b8c0..7bd6306 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -80,6 +80,7 @@ struct unionfs_file_info {
 
 	struct unionfs_dir_state *rdstate;
 	struct file **lower_files;
+	int *saved_branch_ids; /* IDs of branches when file was opened */
 };
 
 /* unionfs inode data in memory */
@@ -123,6 +124,7 @@ struct unionfs_data {
 	struct super_block *sb;
 	atomic_t open_files;	/* number of open files on branch */
 	int branchperms;
+	int branch_id;		/* unique branch ID at re/mount time */
 };
 
 /* unionfs super-block data in memory */
@@ -131,7 +133,7 @@ struct unionfs_sb_info {
 
 	atomic_t generation;
 	struct rw_semaphore rwsem; /* protects access to data+id fields */
-
+	int high_branch_id;	/* last unique branch ID given */
 	struct unionfs_data *data;
 };
 
-- 
1.5.0.3.268.g3dda

[PATCH 09/21] Unionfs: Bulk of branch-management remount code

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/main.c  |   52 +++--
 fs/unionfs/super.c |  612 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/unionfs/union.h |    6 +
 3 files changed, 646 insertions(+), 24 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index ed264c7..1c93b13 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -181,7 +181,7 @@ void unionfs_reinterpose(struct dentry *dentry)
  * 2) it exists
  * 3) is a directory
  */
-static int check_branch(struct nameidata *nd)
+int check_branch(struct nameidata *nd)
 {
 	if (!strcmp(nd->dentry->d_sb->s_type->name, "unionfs"))
 		return -EINVAL;
@@ -211,20 +211,29 @@ static int is_branch_overlap(struct dentry *dent1, struct dentry *dent2)
 	return (dent == dent1);
 }
 
-/* parse branch mode */
-static int parse_branch_mode(char *name)
+/*
+ * Parse branch mode helper function
+ */
+int __parse_branch_mode(const char *name)
 {
-	int perms;
-	int l = strlen(name);
-	if (!strcmp(name + l - 3, "=ro")) {
-		perms = MAY_READ;
-		name[l - 3] = '\0';
-	} else if (!strcmp(name + l - 3, "=rw")) {
-		perms = MAY_READ | MAY_WRITE;
-		name[l - 3] = '\0';
-	} else
-		perms = MAY_READ | MAY_WRITE;
+	if (!name)
+		return 0;
+	if (!strcmp(name, "ro"))
+		return MAY_READ;
+	if (!strcmp(name, "rw"))
+		return (MAY_READ | MAY_WRITE);
+	return 0;
+}
 
+/*
+ * Parse "ro" or "rw" options, but default to "rw" of no mode options
+ * was specified.
+ */
+int parse_branch_mode(const char *name)
+{
+	int perms =  __parse_branch_mode(name);
+	if (perms == 0)
+		perms = MAY_READ | MAY_WRITE;
 	return perms;
 }
 
@@ -271,18 +280,22 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
 		goto out;
 	}
 
-	/* now parsing the string b1:b2=rw:b3=ro:b4 */
+	/* now parsing a string such as "b1:b2=rw:b3=ro:b4" */
 	branches = 0;
 	while ((name = strsep(&options, ":")) != NULL) {
 		int perms;
+		char *mode = strchr(name, '=');
 
-		if (!*name)
+		if (!name || !*name)
 			continue;
 
 		branches++;
 
-		/* strip off =rw or =ro if it is specified. */
-		perms = parse_branch_mode(name);
+		/* strip off '=' if any */
+		if (mode)
+			*mode++ = '\0';
+
+		perms = parse_branch_mode(mode);
 		if (!bindex && !(perms & MAY_WRITE)) {
 			err = -EINVAL;
 			goto out;
@@ -305,8 +318,11 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
 		hidden_root_info->lower_paths[bindex].dentry = nd.dentry;
 		hidden_root_info->lower_paths[bindex].mnt = nd.mnt;
 
+		unionfs_write_lock(sb);
 		set_branchperms(sb, bindex, perms);
 		set_branch_count(sb, bindex, 0);
+		new_branch_id(sb, bindex);
+		unionfs_write_unlock(sb);
 
 		if (hidden_root_info->bstart < 0)
 			hidden_root_info->bstart = bindex;
@@ -387,7 +403,7 @@ static struct unionfs_dentry_info *unionfs_parse_options(struct super_block *sb,
 		char *endptr;
 		int intval;
 
-		if (!*optname)
+		if (!optname || !*optname)
 			continue;
 
 		optarg = strchr(optname, '=');
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 037c47d..7f0d174 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -143,14 +143,614 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return err;
 }
 
-/* We don't support a standard text remount. Eventually it would be nice to
- * support a full-on remount, so that you can have all of the directories
- * change at once, but that would require some pretty complicated matching
- * code.
+/* handle mode changing during remount */
+static int do_remount_mode_option(char *optarg, int cur_branches,
+				  struct unionfs_data *new_data,
+				  struct path *new_lower_paths)
+{
+	int err = -EINVAL;
+	int perms, idx;
+	char *modename = strchr(optarg, '=');
+	struct nameidata nd;
+
+	/* by now, optarg contains the branch name */
+	if (!*optarg) {
+		printk("unionfs: no branch specified for mode change.\n");
+		goto out;
+	}
+	if (!modename) {
+		printk("unionfs: branch \"%s\" requires a mode.\n", optarg);
+		goto out;
+	}
+	*modename++ = '\0';
+	perms = __parse_branch_mode(modename);
+	if (perms == 0) {
+		printk("unionfs: invalid mode \"%s\" for \"%s\".\n",
+		       modename, optarg);
+		goto out;
+	}
+
+	/*
+	 * Find matching branch index.  For now, this assumes that nothing
+	 * has been mounted on top of this Unionfs stack.  Once we have /odf
+	 * and cache-coherency resolved, we'll address the branch-path
+	 * uniqueness.
+	 */
+	err = path_lookup(optarg, LOOKUP_FOLLOW, &nd);
+	if (err) {
+		printk(KERN_WARNING "unionfs: error accessing "
+		       "hidden directory \"%s\" (error %d)\n",
+		       optarg, err);
+		goto out;
+	}
+	for (idx=0; idx<cur_branches; idx++)
+		if (nd.mnt == new_lower_paths[idx].mnt &&
+		    nd.dentry == new_lower_paths[idx].dentry)
+			break;
+	path_release(&nd);	/* no longer needed */
+	if (idx == cur_branches) {
+		err = -ENOENT;	/* err may have been reset above */
+		printk(KERN_WARNING "unionfs: branch \"%s\" "
+		       "not found\n", optarg);
+		goto out;
+	}
+	/* check/change mode for existing branch */
+	/* we don't warn if perms==branchperms */
+	new_data[idx].branchperms = perms;
+	err = 0;
+out:
+	return err;
+}
+
+/* handle branch deletion during remount */
+static int do_remount_del_option(char *optarg, int cur_branches,
+				 struct unionfs_data *new_data,
+				 struct path *new_lower_paths)
+{
+	int err = -EINVAL;
+	int idx;
+	struct nameidata nd;
+	/* optarg contains the branch name to delete */
+
+	/*
+	 * Find matching branch index.  For now, this assumes that nothing
+	 * has been mounted on top of this Unionfs stack.  Once we have /odf
+	 * and cache-coherency resolved, we'll address the branch-path
+	 * uniqueness.
+	 */
+	err = path_lookup(optarg, LOOKUP_FOLLOW, &nd);
+	if (err) {
+		printk(KERN_WARNING "unionfs: error accessing "
+		       "hidden directory \"%s\" (error %d)\n",
+		       optarg, err);
+		goto out;
+	}
+	for (idx=0; idx < cur_branches; idx++)
+		if (nd.mnt == new_lower_paths[idx].mnt &&
+		    nd.dentry == new_lower_paths[idx].dentry)
+			break;
+	path_release(&nd);	/* no longer needed */
+	if (idx == cur_branches) {
+		printk(KERN_WARNING "unionfs: branch \"%s\" "
+		       "not found\n", optarg);
+		err = -ENOENT;
+		goto out;
+	}
+	/* check if there are any open files on the branch to be deleted */
+	if (atomic_read(&new_data[idx].open_files) > 0) {
+		err = -EBUSY;
+		goto out;
+	}
+
+	/*
+	 * Now we have to delete the branch.  First, release any handles it
+	 * has.  Then, move the remaining array indexes past "idx" in
+	 * new_data and new_lower_paths one to the left.  Finally, adjust
+	 * cur_branches.
+	 */
+	pathput(&new_lower_paths[idx]);
+
+	if (idx < cur_branches - 1) {
+		/* if idx==cur_branches-1, we delete last branch: easy */
+		memmove(&new_data[idx], &new_data[idx+1],
+			(cur_branches - 1 - idx) * sizeof(struct unionfs_data));
+		memmove(&new_lower_paths[idx], &new_lower_paths[idx+1],
+			(cur_branches - 1 - idx) * sizeof(struct path));
+	}
+
+	err = 0;
+out:
+	return err;
+}
+
+/* handle branch insertion during remount */
+static int do_remount_add_option(char *optarg, int cur_branches,
+				 struct unionfs_data *new_data,
+				 struct path *new_lower_paths,
+				 int *high_branch_id)
+{
+	int err = -EINVAL;
+	int perms;
+	int idx = 0;		/* default: insert at beginning */
+	char *new_branch , *modename = NULL;
+	struct nameidata nd;
+
+	/*
+	 * optarg can be of several forms:
+	 *
+	 * /bar:/foo		insert /foo before /bar
+	 * /bar:/foo=ro		insert /foo in ro mode before /bar
+	 * /foo			insert /foo in the beginning (prepend)
+	 * :/foo		insert /foo at the end (append)
+	 */
+	if (*optarg == ':') {	/* append? */
+		new_branch = optarg + 1; /* skip ':' */
+		idx = cur_branches;
+		goto found_insertion_point;
+	}
+	new_branch = strchr(optarg, ':');
+	if (!new_branch) {	/* prepend? */
+		new_branch = optarg;
+		goto found_insertion_point;
+	}
+	*new_branch++ = '\0';	/* holds path+mode of new branch */
+
+	/*
+	 * Find matching branch index.  For now, this assumes that nothing
+	 * has been mounted on top of this Unionfs stack.  Once we have /odf
+	 * and cache-coherency resolved, we'll address the branch-path
+	 * uniqueness.
+	 */
+	err = path_lookup(optarg, LOOKUP_FOLLOW, &nd);
+	if (err) {
+		printk(KERN_WARNING "unionfs: error accessing "
+		       "hidden directory \"%s\" (error %d)\n",
+		       optarg, err);
+		goto out;
+	}
+	for (idx=0; idx < cur_branches; idx++)
+		if (nd.mnt == new_lower_paths[idx].mnt &&
+		    nd.dentry == new_lower_paths[idx].dentry)
+			break;
+	path_release(&nd);	/* no longer needed */
+	if (idx == cur_branches) {
+		printk(KERN_WARNING "unionfs: branch \"%s\" "
+		       "not found\n", optarg);
+		err = -ENOENT;
+		goto out;
+	}
+
+	/*
+	 * At this point idx will hold the index where the new branch should
+	 * be inserted before.
+	 */
+found_insertion_point:
+	/* find the mode for the new branch */
+	if (new_branch)
+		modename = strchr(new_branch, '=');
+	if (modename)
+		*modename++ = '\0';
+	perms = parse_branch_mode(modename);
+
+	if (!new_branch || !*new_branch) {
+		printk(KERN_WARNING "unionfs: null new branch\n");
+		err = -EINVAL;
+		goto out;
+	}
+	err = path_lookup(new_branch, LOOKUP_FOLLOW, &nd);
+	if (err) {
+		printk(KERN_WARNING "unionfs: error accessing "
+		       "hidden directory \"%s\" (error %d)\n",
+		       new_branch, err);
+		goto out;
+	}
+	/* it's probably safe to check_mode the new branch to insert */
+	if ((err = check_branch(&nd))) {
+		printk(KERN_WARNING "unionfs: hidden directory "
+		       "\"%s\" is not a valid branch\n", optarg);
+		path_release(&nd);
+		goto out;
+	}
+
+	/*
+	 * Now we have to insert the new branch.  But first, move the bits
+	 * to make space for the new branch, if needed.  Finally, adjust
+	 * cur_branches.
+	 * We don't release nd here; it's kept until umount/remount.
+	 */
+	if (idx < cur_branches) {
+		/* if idx==cur_branches, we append: easy */
+		memmove(&new_data[idx+1], &new_data[idx],
+			(cur_branches - idx) * sizeof(struct unionfs_data));
+		memmove(&new_lower_paths[idx+1], &new_lower_paths[idx],
+			(cur_branches - idx) * sizeof(struct path));
+	}
+	new_lower_paths[idx].dentry = nd.dentry;
+	new_lower_paths[idx].mnt = nd.mnt;
+
+	new_data[idx].sb = nd.dentry->d_sb;
+	atomic_set(&new_data[idx].open_files, 0);
+	new_data[idx].branchperms = perms;
+	new_data[idx].branch_id = ++*high_branch_id; /* assign new branch ID */
+
+	err = 0;
+out:
+	return err;
+}
+
+
+/*
+ * Support branch management options on remount.
+ *
+ * See Documentation/filesystems/unionfs/ for details.
+ *
+ * @flags: numeric mount options
+ * @options: mount options string
+ *
+ * This function can rearrange a mounted union dynamically, adding and
+ * removing branches, including changing branch modes.  Clearly this has to
+ * be done safely and atomically.  Luckily, the VFS already calls this
+ * function with lock_super(sb) and lock_kernel() held, preventing
+ * concurrent mixing of new mounts, remounts, and unmounts.  Moreover,
+ * do_remount_sb(), our caller function, already called shrink_dcache_sb(sb)
+ * to purge dentries/inodes from our superblock, and also called
+ * fsync_super(sb) to purge any dirty pages.  So we're good.
+ *
+ * XXX: however, our remount code may also need to invalidate mapped pages
+ * so as to force them to be re-gotten from the (newly reconfigured) lower
+ * branches.  This has to wait for proper mmap and cache coherency support
+ * in the VFS.
+ *
  */
-static int unionfs_remount_fs(struct super_block *sb, int *flags, char *data)
+static int unionfs_remount_fs(struct super_block *sb, int *flags,
+			      char *options)
 {
-	return -ENOSYS;
+	int err = 0;
+	int i;
+	char *optionstmp, *tmp_to_free;	/* kstrdup'ed of "options" */
+	char *optname;
+	int cur_branches;	/* no. of current branches */
+	int new_branches;	/* no. of branches actually left in the end */
+	int add_branches;	/* est. no. of branches to add */
+	int del_branches;	/* est. no. of branches to del */
+	int max_branches;	/* max possible no. of branches */
+	struct unionfs_data *new_data = NULL, *tmp_data = NULL;
+	struct path *new_lower_paths = NULL, *tmp_lower_paths = NULL;
+	int new_high_branch_id;	/* new high branch ID */
+
+	unionfs_write_lock(sb);
+
+	/*
+	 * The VFS will take care of "ro" and "rw" flags, so anything else
+	 * is an error.  So we need to check if any other flags may have
+	 * been passed (none are allowed/supported as of now).
+	 */
+	if ((*flags & ~MS_RDONLY) != 0) {
+		printk(KERN_WARNING
+		       "unionfs: remount flags 0x%x unsupported\n", *flags);
+		err = -EINVAL;
+		goto out_error;
+	}
+
+	/*
+	 * If 'options' is NULL, it's probably because the user just changed
+	 * the union to a "ro" or "rw" and the VFS took care of it.  So
+	 * nothing to do and we're done.
+	 */
+	if (options[0] == '\0')
+		goto out_error;
+
+	/*
+	 * Find out how many branches we will have in the end, counting
+	 * "add" and "del" commands.  Copy the "options" string because
+	 * strsep modifies the string and we need it later.
+	 */
+	optionstmp = tmp_to_free = kstrdup(options, GFP_KERNEL);
+	if (!optionstmp) {
+		err = -ENOMEM;
+		goto out_free;
+	}
+	new_branches = cur_branches = sbmax(sb); /* current no. branches */
+	add_branches = del_branches = 0;
+	new_high_branch_id = sbhbid(sb); /* save current high_branch_id */
+	while ((optname = strsep(&optionstmp, ",")) != NULL) {
+		char *optarg;
+
+		if (!optname || !*optname)
+			continue;
+
+		optarg = strchr(optname, '=');
+		if (optarg)
+			*optarg++ = '\0';
+
+		if (!strcmp("add", optname))
+			add_branches++;
+		else if (!strcmp("del", optname))
+			del_branches++;
+	}
+	kfree(tmp_to_free);
+	/* after all changes, will we have at least one branch left? */
+	if ((new_branches + add_branches - del_branches) < 1) {
+		printk(KERN_WARNING
+		       "unionfs: no branches left after remount\n");
+		err = -EINVAL;
+		goto out_free;
+	}
+
+	/*
+	 * Since we haven't actually parsed all the add/del options, nor
+	 * have we checked them for errors, we don't know for sure how many
+	 * branches we will have after all changes have taken place.  In
+	 * fact, the total number of branches left could be less than what
+	 * we have now.  So we need to allocate space for a temporary
+	 * placeholder that is at least as large as the maximum number of
+	 * branches we *could* have, which is the current number plus all
+	 * the additions.  Once we're done with these temp placeholders, we
+	 * may have to re-allocate the final size, copy over from the temp,
+	 * and then free the temps (done near the end of this function).
+	 */
+	max_branches = cur_branches + add_branches;
+	/* allocate space for new pointers to hidden dentry */
+	tmp_data = kcalloc(max_branches,
+			   sizeof(struct unionfs_data), GFP_KERNEL);
+	if (!tmp_data) {
+		err = -ENOMEM;
+		goto out_free;
+	}
+	/* allocate space for new pointers to lower paths */
+	tmp_lower_paths = kcalloc(max_branches,
+			   sizeof(struct path), GFP_KERNEL);
+	if (!tmp_lower_paths) {
+		err = -ENOMEM;
+		goto out_free;
+	}
+	/* copy current info into new placeholders, incrementing refcnts */
+	memcpy(tmp_data, UNIONFS_SB(sb)->data,
+	       cur_branches * sizeof(struct unionfs_data));
+	memcpy(tmp_lower_paths, UNIONFS_D(sb->s_root)->lower_paths,
+	       cur_branches * sizeof(struct path));
+	for (i=0; i<cur_branches; i++)
+		pathget(&tmp_lower_paths[i]); /* drop refs at end of fxn */
+
+	/*******************************************************************
+	 * For each branch command, do path_lookup on the requested branch,
+	 * and apply the change to a temp branch list.  To handle errors, we
+	 * already dup'ed the old arrays (above), and increased the refcnts
+	 * on various f/s objects.  So now we can do all the path_lookups
+	 * and branch-management commands on the new arrays.  If it fail mid
+	 * way, we free the tmp arrays and *put all objects.  If we succeed,
+	 * then we free old arrays and *put its objects, and then replace
+	 * the arrays with the new tmp list (we may have to re-allocate the
+	 * memory because the temp lists could have been larger than what we
+	 * actually needed).
+	 *******************************************************************/
+
+	while ((optname = strsep(&options, ",")) != NULL) {
+		char *optarg;
+
+		if (!optname || !*optname)
+			continue;
+		/*
+		 * At this stage optname holds a comma-delimited option, but
+		 * without the commas.  Next, we need to break the string on
+		 * the '=' symbol to separate CMD=ARG, where ARG itself can
+		 * be KEY=VAL.  For example, in mode=/foo=rw, CMD is "mode",
+		 * KEY is "/foo", and VAL is "rw".
+		 */
+		optarg = strchr(optname, '=');
+		if (optarg)
+			*optarg++ = '\0';
+		/* incgen remount option (instead of old ioctl) */
+		if (!strcmp("incgen", optname)) {
+			err = 0;
+			goto out_no_change;
+		}
+
+		/*
+		 * All of our options take an argument now.  (Insert ones
+		 * that don't above this check.)  So at this stage optname
+		 * contains the CMD part and optarg contains the ARG part.
+		 */
+		if (!optarg || !*optarg) {
+			printk("unionfs: all remount options require "
+			       "an argument (%s).\n", optname);
+			err = -EINVAL;
+			goto out_release;
+		}
+
+		if (!strcmp("add", optname)) {
+			err = do_remount_add_option(optarg, new_branches,
+						    tmp_data,
+						    tmp_lower_paths,
+						    &new_high_branch_id);
+			if (err)
+				goto out_release;
+			new_branches++;
+			if (new_branches > UNIONFS_MAX_BRANCHES) {
+				printk("unionfs: command exceeds %d branches\n",
+				       UNIONFS_MAX_BRANCHES);
+				err = -E2BIG;
+				goto out_release;
+			}
+			continue;
+		}
+		if (!strcmp("del", optname)) {
+			err = do_remount_del_option(optarg, new_branches,
+						    tmp_data,
+						    tmp_lower_paths);
+			if (err)
+				goto out_release;
+			new_branches--;
+			continue;
+		}
+		if (!strcmp("mode", optname)) {
+			err = do_remount_mode_option(optarg, new_branches,
+						     tmp_data,
+						     tmp_lower_paths);
+			if (err)
+				goto out_release;
+			continue;
+		}
+
+		/*
+		 * When you use "mount -o remount,ro", mount(8) will
+		 * reportedly pass the original dirs= string from
+		 * /proc/mounts.  So for now, we have to ignore dirs= and
+		 * not consider it an error, unless we want to allow users
+		 * to pass dirs= in remount.  Note that to allow the VFS to
+		 * actually process the ro/rw remount options, we have to
+		 * return 0 from this function.
+		 */
+		if (!strcmp("dirs", optname)) {
+			printk(KERN_WARNING
+			       "unionfs: remount ignoring option \"%s\".\n",
+			       optname);
+			continue;
+		}
+
+		err = -EINVAL;
+		printk(KERN_WARNING
+		       "unionfs: unrecognized option \"%s\"\n", optname);
+		goto out_release;
+	}
+
+out_no_change:
+
+	/******************************************************************
+	 * WE'RE ALMOST DONE: see if we need to allocate a small-sized new
+	 * vector, copy the vectors to their correct place, release the
+	 * refcnt of the older ones, and return.
+	 * Also handle invalidating any pgaes that will have to be re-read.
+	 *******************************************************************/
+
+	/*
+	 * Allocate space for actual pointers, if needed.  By the time we
+	 * finish this block of code, new_branches and new_lower_paths will
+	 * have the correct size.  None of this code below would be needed
+	 * if the kernel had a realloc() function, at least one capable of
+	 * shrinking/truncating an allocation's size (hint, hint).
+	 */
+	if (new_branches < max_branches) {
+
+		/* allocate space for new pointers to hidden dentry */
+		new_data = kcalloc(new_branches,
+				   sizeof(struct unionfs_data), GFP_KERNEL);
+		if (!new_data) {
+			err = -ENOMEM;
+			goto out_release;
+		}
+		/* allocate space for new pointers to lower paths */
+		new_lower_paths = kcalloc(new_branches,
+					  sizeof(struct path), GFP_KERNEL);
+		if (!new_lower_paths) {
+			err = -ENOMEM;
+			goto out_release;
+		}
+		/*
+		 * copy current info into new placeholders, incrementing
+		 * refcounts.
+		 */
+		memcpy(new_data, tmp_data,
+		       new_branches * sizeof(struct unionfs_data));
+		memcpy(new_lower_paths, tmp_lower_paths,
+		       new_branches * sizeof(struct path));
+		/*
+		 * Since we already hold various refcnts on the objects, we
+		 * don't need to redo it here.  Just free the older memory
+		 * and re-point the pointers.
+		 */
+		kfree(tmp_data);
+		kfree(tmp_lower_paths);
+		/* no need to nullify pointers here */
+	} else {
+		/* number of branches didn't change, no need to re-alloc */
+		new_data = tmp_data;
+		new_lower_paths = tmp_lower_paths;
+	}
+
+	/*
+	 * OK, just before we actually put the new set of branches in place,
+	 * we need to ensure that our own f/s has no dirty objects left.
+	 * Luckily, do_remount_sb() already calls shrink_dcache_sb(sb) and
+	 * fsync_super(sb), taking care of dentries, inodes, and dirty
+	 * pages.  So all that's left is for us to invalidate any leftover
+	 * (non-dirty) pages to ensure that they will be re-read from the
+	 * new lower branches (and to support mmap).
+	 */
+
+	/*
+	 * No we call drop_pagecache_sb() to invalidate all pages in this
+	 * super.  This function calls invalidate_inode_pages(mapping),
+	 * which calls invalidate_mapping_pages(): the latter, however, will
+	 * not invalidate pages which are dirty, locked, under writeback, or
+	 * mapped into pagetables.  We shouldn't have to worry about dirty
+	 * or under-writeback pages, because do_remount_sb() called
+	 * fsync_super() which would not have returned until all dirty pages
+	 * were flushed.
+	 *
+	 * But do w have to worry about locked pages?  Is there any chance
+	 * that in here we'll get locked pages?
+	 *
+	 * XXX: what about pages mapped into pagetables?  Are these pages
+	 * which user processes may have mmap(2)'ed?  If so, then we need to
+	 * invalidate those too, no?  Maybe we'll have to write our own
+	 * version of invalidate_mapping_pages() which also handled mapped
+	 * pages.
+	 *
+	 * XXX: Alternatively, maybe we should call truncate_inode_pages(),
+	 * which use two passes over the pages list, and will truncate all
+	 * pages.
+	 */
+	drop_pagecache_sb(sb);
+
+	/* copy new vectors into their correct place */
+	tmp_data = UNIONFS_SB(sb)->data;
+	UNIONFS_SB(sb)->data = new_data;
+	new_data = NULL;	/* so don't free good pointers below */
+	tmp_lower_paths = UNIONFS_D(sb->s_root)->lower_paths;
+	UNIONFS_D(sb->s_root)->lower_paths = new_lower_paths;
+	new_lower_paths = NULL;	/* so don't free good pointers below */
+
+	/* update our unionfs_sb_info and root dentry index of last branch */
+	i = sbmax(sb);		/* save no. of branches to release at end */
+	sbend(sb) = new_branches - 1;
+	set_dbend(sb->s_root, new_branches - 1);
+	UNIONFS_D(sb->s_root)->bcount = new_branches;
+	new_branches = i;	/* no. of branches to release below */
+
+	/* maxbytes may have changed */
+	sb->s_maxbytes = unionfs_lower_super_idx(sb, 0)->s_maxbytes;
+	/* update high branch ID */
+	sbhbid(sb) = new_high_branch_id;
+
+	/* update our sb->generation for revalidating objects */
+	i = atomic_inc_return(&UNIONFS_SB(sb)->generation);
+	atomic_set(&UNIONFS_D(sb->s_root)->generation, i);
+	atomic_set(&UNIONFS_I(sb->s_root->d_inode)->generation, i);
+	printk("unionfs: new generation number %d\n", i);
+	err = 0;		/* reset to success */
+
+	/*
+	 * The code above falls through to the next label, and releases the
+	 * refcnts of the older ones (stored in tmp_*): if we fell through
+	 * here, it means success.  However, if we jump directly to this
+	 * label from any error above, then an error occurred after we
+	 * grabbed various refcnts, and so we have to release the
+	 * temporarily constructed structures.
+	 */
+out_release:
+	/* no need to cleanup/release anything in tmp_data */
+	if (tmp_lower_paths)
+		for (i=0; i<new_branches; i++)
+			pathput(&tmp_lower_paths[i]);
+out_free:
+	kfree(tmp_lower_paths);
+	kfree(tmp_data);
+	kfree(new_lower_paths);
+	kfree(new_data);
+out_error:
+	unionfs_write_unlock(sb);
+	return err;
 }
 
 /*
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 7bd6306..715a3ad 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -60,6 +60,9 @@
 /* number of times we try to get a unique temporary file name */
 #define GET_TMPNAM_MAX_RETRY	5
 
+/* maximum number of branches we support, to avoid memory blowup */
+#define UNIONFS_MAX_BRANCHES	128
+
 /* Operations vectors defined in specific files. */
 extern struct file_operations unionfs_main_fops;
 extern struct file_operations unionfs_dir_fops;
@@ -408,6 +411,9 @@ static inline int is_robranch(const struct dentry *dentry)
  * EXTERNALS:
  */
 extern char *alloc_whname(const char *name, int len);
+extern int check_branch(struct nameidata *nd);
+extern int __parse_branch_mode(const char *name);
+extern int parse_branch_mode(const char *name);
 
 /* These two functions are here because it is kind of daft to copy and paste the
  * contents of the two functions to 32+ places in unionfs
-- 
1.5.0.3.268.g3dda

[PATCH 10/21] Unionfs: Introduce unionfs_mnt{get,put}

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Helper inline functions to perform Unionfs's mntget/put ops on lower
branches.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
[jsipek: cleanup branching in unionfs_mnt{get,put} and compile fixes]
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |    8 ++++----
 fs/unionfs/copyup.c     |    6 +++---
 fs/unionfs/dentry.c     |    2 +-
 fs/unionfs/dirhelper.c  |    2 +-
 fs/unionfs/lookup.c     |   25 +++++++++++++------------
 fs/unionfs/main.c       |    7 ++++---
 fs/unionfs/union.h      |   35 ++++++++++++++++++++++++++++++++++-
 7 files changed, 60 insertions(+), 25 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 8d0f8d1..c20dd6f 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -169,7 +169,7 @@ static int open_all_files(struct file *file)
 			continue;
 
 		dget(hidden_dentry);
-		mntget(unionfs_lower_mnt_idx(dentry, bindex));
+		unionfs_mntget(dentry, bindex);
 		branchget(sb, bindex);
 
 		hidden_file = dentry_open(hidden_dentry,
@@ -214,7 +214,7 @@ static int open_highest_file(struct file *file, int willwrite)
 	}
 
 	dget(hidden_dentry);
-	mntget(unionfs_lower_mnt_idx(dentry, bstart));
+	unionfs_mntget(dentry, bstart);
 	branchget(sb, bstart);
 	hidden_file = dentry_open(hidden_dentry,
 			unionfs_lower_mnt_idx(dentry, bstart), file->f_flags);
@@ -371,7 +371,7 @@ static int __open_dir(struct inode *inode, struct file *file)
 			continue;
 
 		dget(hidden_dentry);
-		mntget(unionfs_lower_mnt_idx(file->f_dentry, bindex));
+		unionfs_mntget(file->f_dentry, bindex);
 		hidden_file = dentry_open(hidden_dentry,
 				unionfs_lower_mnt_idx(file->f_dentry, bindex),
 				file->f_flags);
@@ -431,7 +431,7 @@ static int __open_file(struct inode *inode, struct file *file)
 	/* dentry_open will decrement mnt refcnt if err.
 	 * otherwise fput() will do an mntput() for us upon file close.
 	 */
-	mntget(unionfs_lower_mnt_idx(file->f_dentry, bstart));
+	unionfs_mntget(file->f_dentry, bstart);
 	hidden_file = dentry_open(hidden_dentry,
 				  unionfs_lower_mnt_idx(file->f_dentry, bstart),
 				  hidden_flags);
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index e24d940..4ccef81 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -206,7 +206,7 @@ static int __copyup_reg_data(struct dentry *dentry,
 	int err = 0;
 
 	/* open old file */
-	mntget(unionfs_lower_mnt_idx(dentry, old_bindex));
+	unionfs_mntget(dentry, old_bindex);
 	branchget(sb, old_bindex);
 	input_file = dentry_open(old_hidden_dentry,
 				unionfs_lower_mnt_idx(dentry, old_bindex),
@@ -223,7 +223,7 @@ static int __copyup_reg_data(struct dentry *dentry,
 
 	/* open new file */
 	dget(new_hidden_dentry);
-	mntget(unionfs_lower_mnt_idx(dentry, new_bindex));
+	unionfs_mntget(dentry, new_bindex);
 	branchget(sb, new_bindex);
 	output_file = dentry_open(new_hidden_dentry,
 				unionfs_lower_mnt_idx(dentry, new_bindex),
@@ -555,7 +555,7 @@ static void __cleanup_dentry(struct dentry * dentry, int bindex,
 			dput(unionfs_lower_dentry_idx(dentry, i));
 			unionfs_set_lower_dentry_idx(dentry, i, NULL);
 
-			mntput(unionfs_lower_mnt_idx(dentry, i));
+			unionfs_mntput(dentry, i);
 			unionfs_set_lower_mnt_idx(dentry, i, NULL);
 		} else {
 			if (new_bstart < 0)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index cae4897..5ee1451 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -206,7 +206,7 @@ static void unionfs_d_release(struct dentry *dentry)
 	bend = dbend(dentry);
 	for (bindex = bstart; bindex <= bend; bindex++) {
 		dput(unionfs_lower_dentry_idx(dentry, bindex));
-		mntput(unionfs_lower_mnt_idx(dentry, bindex));
+		unionfs_mntput(dentry, bindex);
 
 		unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
 		unionfs_set_lower_mnt_idx(dentry, bindex, NULL);
diff --git a/fs/unionfs/dirhelper.c b/fs/unionfs/dirhelper.c
index 0da8a8e..bb5f7bc 100644
--- a/fs/unionfs/dirhelper.c
+++ b/fs/unionfs/dirhelper.c
@@ -225,7 +225,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
 			continue;
 
 		dget(hidden_dentry);
-		mntget(unionfs_lower_mnt_idx(dentry, bindex));
+		unionfs_mntget(dentry, bindex);
 		branchget(sb, bindex);
 		hidden_file =
 		    dentry_open(hidden_dentry, unionfs_lower_mnt_idx(dentry, bindex),
diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 967bb5b..0572247 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -79,7 +79,8 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 	struct dentry *parent_dentry = NULL;
 	int bindex, bstart, bend, bopaque;
 	int dentry_count = 0;	/* Number of positive dentries. */
-	int first_dentry_offset = -1;
+	int first_dentry_offset = -1; /* -1 is uninitialized */
+	struct dentry *first_dentry = NULL;
 	struct dentry *first_hidden_dentry = NULL;
 	struct vfsmount *first_hidden_mnt = NULL;
 	int locked_parent = 0;
@@ -176,7 +177,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 						  namelen + UNIONFS_WHLEN);
 		if (IS_ERR(wh_hidden_dentry)) {
 			dput(first_hidden_dentry);
-			mntput(first_hidden_mnt);
+			unionfs_mntput(first_dentry, first_dentry_offset);
 			err = PTR_ERR(wh_hidden_dentry);
 			goto out_free;
 		}
@@ -194,7 +195,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 			       " %d.\n", wh_hidden_dentry->d_inode->i_mode);
 			dput(wh_hidden_dentry);
 			dput(first_hidden_dentry);
-			mntput(first_hidden_mnt);
+			unionfs_mntput(first_dentry, first_dentry_offset);
 			goto out_free;
 		}
 
@@ -210,7 +211,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 					       namelen, nd);
 		if (IS_ERR(hidden_dentry)) {
 			dput(first_hidden_dentry);
-			mntput(first_hidden_mnt);
+			unionfs_mntput(first_dentry, first_dentry_offset);
 			err = PTR_ERR(hidden_dentry);
 			goto out_free;
 		}
@@ -224,9 +225,8 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 				/* FIXME: following line needs to be changed
 				 * to allow mountpoint crossing
 				 */
-				first_hidden_mnt = mntget(
-					unionfs_lower_mnt_idx(parent_dentry,
-								bindex));
+				first_dentry = parent_dentry;
+				first_hidden_mnt = unionfs_mntget(parent_dentry, bindex);
 				first_dentry_offset = bindex;
 			} else
 				dput(hidden_dentry);
@@ -245,7 +245,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 		 * mountpoint crossing
 		 */
 		unionfs_set_lower_mnt_idx(dentry, bindex,
-			mntget(unionfs_lower_mnt_idx(parent_dentry, bindex)));
+					  unionfs_mntget(parent_dentry, bindex));
 		set_dbend(dentry, bindex);
 
 		/* update parent directory's atime with the bindex */
@@ -266,7 +266,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 		opaque = is_opaque_dir(dentry, bindex);
 		if (opaque < 0) {
 			dput(first_hidden_dentry);
-			mntput(first_hidden_mnt);
+			unionfs_mntput(first_dentry, first_dentry_offset);
 			err = opaque;
 			goto out_free;
 		} else if (opaque) {
@@ -309,7 +309,8 @@ out_negative:
 		/* FIXME: the following line needs to be changed to allow
 		 * mountpoint crossing
 		 */
-		first_hidden_mnt = mntget(unionfs_lower_mnt_idx(dentry, bindex));
+		first_dentry = dentry;
+		first_hidden_mnt = unionfs_mntget(dentry, bindex);
 	}
 	unionfs_set_lower_dentry_idx(dentry, first_dentry_offset, first_hidden_dentry);
 	unionfs_set_lower_mnt_idx(dentry, first_dentry_offset, first_hidden_mnt);
@@ -330,7 +331,7 @@ out_positive:
 	 * vfsmount - throw it out.
 	 */
 	dput(first_hidden_dentry);
-	mntput(first_hidden_mnt);
+	unionfs_mntput(first_dentry, first_dentry_offset);
 
 	/* Partial lookups need to reinterpose, or throw away older negs. */
 	if (lookupmode == INTERPOSE_PARTIAL) {
@@ -365,7 +366,7 @@ out_free:
 		bend = dbend(dentry);
 		for (bindex = bstart; bindex <= bend; bindex++) {
 			dput(unionfs_lower_dentry_idx(dentry, bindex));
-			mntput(unionfs_lower_mnt_idx(dentry, bindex));
+			unionfs_mntput(dentry, bindex);
 		}
 	}
 	kfree(UNIONFS_D(dentry)->lower_paths);
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 1c93b13..b80b554 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -358,6 +358,7 @@ out:
 		for (i = 0; i < branches; i++)
 			if (hidden_root_info->lower_paths[i].dentry) {
 				dput(hidden_root_info->lower_paths[i].dentry);
+				/* initializing: can't use unionfs_mntput here */
 				mntput(hidden_root_info->lower_paths[i].mnt);
 			}
 
@@ -466,9 +467,8 @@ out_error:
 			m = hidden_root_info->lower_paths[bindex].mnt;
 
 			dput(d);
-
-			if (m)
-				mntput(m);
+			/* initializing: can't use unionfs_mntput here */
+			mntput(m);
 		}
 	}
 
@@ -618,6 +618,7 @@ out_dput:
 			m = hidden_root_info->lower_paths[bindex].mnt;
 
 			dput(d);
+			/* initializing: can't use unionfs_mntput here */
 			mntput(m);
 		}
 		kfree(hidden_root_info->lower_paths);
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 715a3ad..53c7c2c 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -434,5 +434,38 @@ static inline void unlock_dir(struct dentry *dir)
 
 extern int make_dir_opaque(struct dentry *dir, int bindex);
 
-#endif	/* not _UNION_H_ */
+static inline struct vfsmount *unionfs_mntget(struct dentry *dentry, int bindex)
+{
+	struct vfsmount *mnt;
+	if (!dentry) {
+		if (bindex < 0)
+			return NULL;
+		BUG_ON(bindex < 0);
+	}
+	mnt = unionfs_lower_mnt_idx(dentry, bindex);
+	if (!mnt) {
+		if (bindex < 0)
+			return NULL;
+		BUG_ON(mnt && bindex < 0);
+	}
+	mnt = mntget(mnt);
+	return mnt;
+}
 
+static inline void unionfs_mntput(struct dentry *dentry, int bindex)
+{
+	struct vfsmount *mnt;
+	if (!dentry) {
+		if (bindex < 0)
+			return;
+		BUG_ON(dentry && bindex < 0);
+	}
+	mnt = unionfs_lower_mnt_idx(dentry, bindex);
+	if (!mnt) {
+		if (bindex < 0)
+			return;
+		BUG_ON(mnt && bindex < 0);
+	}
+	mntput(mnt);
+}
+#endif	/* not _UNION_H_ */
-- 
1.5.0.3.268.g3dda

[PATCH 11/21] Unionfs: Rewrite unionfs_d_revalidate

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Rewrite unionfs_d_revalidate code to avoid stack-unfriendly recursion: split
into a call to revalidate just one dentry, and an interative driver function
to revalidate an entire dentry-parent chain.

Fix vfsmount ref leaks which prevented lower f/s from being unmounted after
generation increment, esp. during heavy loads.

Fix one deadlock between revalidation code and VFS.

Better documentation of what the code does.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
[jsipek: compile & whitespace fixes]
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |   12 +++-
 fs/unionfs/dentry.c     |  153 +++++++++++++++++++++++++++++++++++++++--------
 fs/unionfs/union.h      |    1 +
 3 files changed, 136 insertions(+), 30 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index c20dd6f..9b6128c 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -266,7 +266,11 @@ static int do_delayed_copyup(struct file *file, struct dentry *dentry)
 	return err;
 }
 
-/* revalidate the stuct file */
+/*
+ * Revalidate the struct file
+ * @file: file to revalidate
+ * @willwrite: 1 if caller may cause changes to the file; 0 otherwise.
+ */
 int unionfs_file_revalidate(struct file *file, int willwrite)
 {
 	struct super_block *sb;
@@ -280,8 +284,9 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
 	dentry = file->f_dentry;
 	unionfs_lock_dentry(dentry);
 	sb = dentry->d_sb;
-	unionfs_read_lock(sb);
-	if (!__unionfs_d_revalidate(dentry, NULL) && !d_deleted(dentry)) {
+
+	/* first revalidate the dentry inside struct file */
+	if (!__unionfs_d_revalidate_chain(dentry, NULL) && !d_deleted(dentry)) {
 		err = -ESTALE;
 		goto out_nofree;
 	}
@@ -351,7 +356,6 @@ out:
 	}
 out_nofree:
 	unionfs_unlock_dentry(dentry);
-	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 5ee1451..c841f08 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -18,10 +18,15 @@
 
 #include "union.h"
 
+
 /*
- * returns 1 if valid, 0 otherwise.
+ * Revalidate a single dentry.
+ * Assume that dentry's info node is locked.
+ * Assume that parent(s) are all valid already, but
+ * the child may not yet be valid.
+ * Returns 1 if valid, 0 otherwise.
  */
-int __unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
+static int __unionfs_d_revalidate_one(struct dentry *dentry, struct nameidata *nd)
 {
 	int valid = 1;		/* default is valid (1); invalid is 0. */
 	struct dentry *hidden_dentry;
@@ -29,7 +34,6 @@ int __unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 	int sbgen, dgen;
 	int positive = 0;
 	int locked = 0;
-	int restart = 0;
 	int interpose_flag;
 
 	struct nameidata lowernd; /* TODO: be gentler to the stack */
@@ -39,7 +43,6 @@ int __unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 	else
 		memset(&lowernd, 0, sizeof(struct nameidata));
 
-restart:
 	verify_locked(dentry);
 
 	/* if the dentry is unhashed, do NOT revalidate */
@@ -62,28 +65,12 @@ restart:
 		struct dentry *result;
 		int pdgen;
 
-		unionfs_read_lock(dentry->d_sb);
-		locked = 1;
-
 		/* The root entry should always be valid */
 		BUG_ON(IS_ROOT(dentry));
 
 		/* We can't work correctly if our parent isn't valid. */
 		pdgen = atomic_read(&UNIONFS_D(dentry->d_parent)->generation);
-		if (!restart && (pdgen != sbgen)) {
-			unionfs_read_unlock(dentry->d_sb);
-			locked = 0;
-			/* We must be locked before our parent. */
-			if (!
-			    (dentry->d_parent->d_op->
-			     d_revalidate(dentry->d_parent, nd))) {
-				valid = 0;
-				goto out;
-			}
-			restart = 1;
-			goto restart;
-		}
-		BUG_ON(pdgen != sbgen);
+		BUG_ON(pdgen != sbgen);	/* should never happen here */
 
 		/* Free the pointers for our inodes and this dentry. */
 		bstart = dbstart(dentry);
@@ -102,7 +89,17 @@ restart:
 		interpose_flag = INTERPOSE_REVAL_NEG;
 		if (positive) {
 			interpose_flag = INTERPOSE_REVAL;
-			mutex_lock(&dentry->d_inode->i_mutex);
+			/*
+			 * During BRM, the VFS could already hold a lock on
+			 * a file being read, so don't lock it again
+			 * (deadlock), but if you lock it in this function,
+			 * then release it here too.
+			 */
+			if (!mutex_is_locked(&dentry->d_inode->i_mutex)) {
+				mutex_lock(&dentry->d_inode->i_mutex);
+				locked = 1;
+			}
+
 			bstart = ibstart(dentry->d_inode);
 			bend = ibend(dentry->d_inode);
 			if (bstart >= 0) {
@@ -118,7 +115,8 @@ restart:
 			UNIONFS_I(dentry->d_inode)->lower_inodes = NULL;
 			ibstart(dentry->d_inode) = -1;
 			ibend(dentry->d_inode) = -1;
-			mutex_unlock(&dentry->d_inode->i_mutex);
+			if (locked)
+				mutex_unlock(&dentry->d_inode->i_mutex);
 		}
 
 		result = unionfs_lookup_backend(dentry, &lowernd, interpose_flag);
@@ -169,8 +167,111 @@ restart:
 	}
 
 out:
-	if (locked)
-		unionfs_read_unlock(dentry->d_sb);
+	return valid;
+}
+
+/*
+ * Revalidate a parent chain of dentries, then the actual node.
+ * Assumes that dentry is locked, but will lock all parents if/when needed.
+ */
+int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd)
+{
+	int valid = 0;		/* default is invalid (0); valid is 1. */
+	struct dentry **chain = NULL; /* chain of dentries to reval */
+	int chain_len = 0;
+	struct dentry *dtmp;
+	int sbgen, dgen, i;
+	int saved_bstart, saved_bend, bindex;
+
+	/* find length of chain needed to revalidate */
+	/* XXX: should I grab some global (dcache?) lock? */
+	chain_len = 0;
+	sbgen = atomic_read(&UNIONFS_SB(dentry->d_sb)->generation);
+	dtmp = dentry->d_parent;
+	dgen = atomic_read(&UNIONFS_D(dtmp)->generation);
+	while (sbgen != dgen) {
+		/* The root entry should always be valid */
+		BUG_ON(IS_ROOT(dtmp));
+		chain_len++;
+		dtmp = dtmp->d_parent;
+		dgen = atomic_read(&UNIONFS_D(dtmp)->generation);
+	}
+	if (chain_len == 0) {
+		goto out_this;	/* shortcut if parents are OK */
+	}
+
+	/*
+	 * Allocate array of dentries to reval.  We could use linked lists,
+	 * but the number of entries we need to alloc here is often small,
+	 * and short lived, so locality will be better.
+	 */
+	chain = kzalloc(chain_len * sizeof(struct dentry *), GFP_KERNEL);
+	if (!chain) {
+		printk("unionfs: no more memory in %s\n", __FUNCTION__);
+		goto out;
+	}
+
+	/*
+	 * lock all dentries in chain, in child to parent order.
+	 * if failed, then sleep for a little, then retry.
+	 */
+	dtmp = dentry->d_parent;
+	for (i=chain_len-1; i>=0; i--) {
+		chain[i] = dget(dtmp);
+		dtmp = dtmp->d_parent;
+	}
+
+	/*
+	 * call __unionfs_d_revalidate() on each dentry, but in parent to
+	 * child order.
+	 */
+	for (i=0; i<chain_len; i++) {
+		unionfs_lock_dentry(chain[i]);
+		saved_bstart = dbstart(chain[i]);
+		saved_bend = dbend(chain[i]);
+		sbgen = atomic_read(&UNIONFS_SB(dentry->d_sb)->generation);
+		dgen = atomic_read(&UNIONFS_D(chain[i])->generation);
+
+		valid = __unionfs_d_revalidate_one(chain[i], nd);
+		/* XXX: is this the correct mntput condition?! */
+		if (valid && chain_len > 0 &&
+		    sbgen != dgen && dentry->d_inode &&
+		    S_ISDIR(dentry->d_inode->i_mode)) {
+			for (bindex = saved_bstart; bindex <= saved_bend; bindex++)
+				unionfs_mntput(chain[i], bindex);
+		}
+		unionfs_unlock_dentry(chain[i]);
+
+		if (!valid) {
+			goto out_free;
+		}
+	}
+
+
+ out_this:
+	/* finally, lock this dentry and revalidate it */
+	verify_locked(dentry);
+	dgen = atomic_read(&UNIONFS_D(dentry)->generation);
+	saved_bstart = dbstart(dentry);
+	saved_bend = dbend(dentry);
+	valid = __unionfs_d_revalidate_one(dentry, nd);
+
+	if (valid && chain_len > 0 &&
+	    sbgen != dgen && dentry->d_inode &&
+	    S_ISDIR(dentry->d_inode->i_mode)) {
+		for (bindex = saved_bstart; bindex <= saved_bend; bindex++)
+			unionfs_mntput(dentry, bindex);
+	}
+
+ out_free:
+	/* unlock/dput all dentries in chain and return status */
+	if (chain_len > 0) {
+		for (i=0; i<chain_len; i++) {
+			dput(chain[i]);
+		}
+		kfree(chain);
+	}
+ out:
 	return valid;
 }
 
@@ -179,7 +280,7 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 	int err;
 
 	unionfs_lock_dentry(dentry);
-	err = __unionfs_d_revalidate(dentry, nd);
+	err = __unionfs_d_revalidate_chain(dentry, nd);
 	unionfs_unlock_dentry(dentry);
 
 	return err;
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 53c7c2c..66ffe88 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -302,6 +302,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry);
 int unionfs_rmdir(struct inode *dir, struct dentry *dentry);
 
 int __unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd);
+int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd);
 
 /* The values for unionfs_interpose's flag. */
 #define INTERPOSE_DEFAULT	0
-- 
1.5.0.3.268.g3dda

[PATCH 12/21] Unionfs: Grab the unionfs sb private data lock around branch info users

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Locking/concurrency/race fixes.  Use the unionfs superblock rwsem, and grab
the read lock around every op that uses branch-related information, such as
branch counters.  Grab the write rwsem lock in operations which attempt to
change branch information, such as when adding/deleting branches.  This
will, for example, cause branch-management remount commands (which are
infrequent) to block a bit until all in-progress file operations on open
files are done.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
[jsipek: whitespace fixes & more locks/unlocks]
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |   25 ++++++++++++++++++++++---
 fs/unionfs/copyup.c     |   14 ++++++++++----
 fs/unionfs/dirfops.c    |    4 ++++
 fs/unionfs/dirhelper.c  |    6 ++++++
 fs/unionfs/file.c       |   14 ++++++++++++++
 fs/unionfs/inode.c      |   10 +++++++---
 fs/unionfs/main.c       |    4 ++++
 fs/unionfs/super.c      |    6 ++++++
 fs/unionfs/union.h      |   10 +++++++---
 9 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 9b6128c..6606cba 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -170,7 +170,9 @@ static int open_all_files(struct file *file)
 
 		dget(hidden_dentry);
 		unionfs_mntget(dentry, bindex);
+		unionfs_read_lock(sb);
 		branchget(sb, bindex);
+		unionfs_read_unlock(sb);
 
 		hidden_file = dentry_open(hidden_dentry,
 				unionfs_lower_mnt_idx(dentry, bindex),
@@ -215,7 +217,9 @@ static int open_highest_file(struct file *file, int willwrite)
 
 	dget(hidden_dentry);
 	unionfs_mntget(dentry, bstart);
+	unionfs_read_lock(sb);
 	branchget(sb, bstart);
+	unionfs_read_unlock(sb);
 	hidden_file = dentry_open(hidden_dentry,
 			unionfs_lower_mnt_idx(dentry, bstart), file->f_flags);
 	if (IS_ERR(hidden_file)) {
@@ -256,7 +260,9 @@ static int do_delayed_copyup(struct file *file, struct dentry *dentry)
 		bend = fbend(file);
 		for (bindex = bstart; bindex <= bend; bindex++) {
 			if (unionfs_lower_file_idx(file, bindex)) {
+				unionfs_read_lock(dentry->d_sb);
 				branchput(dentry->d_sb, bindex);
+				unionfs_read_unlock(dentry->d_sb);
 				fput(unionfs_lower_file_idx(file, bindex));
 				unionfs_set_lower_file_idx(file, bindex, NULL);
 			}
@@ -387,7 +393,9 @@ static int __open_dir(struct inode *inode, struct file *file)
 		/* The branchget goes after the open, because otherwise
 		 * we would miss the reference on release.
 		 */
+		unionfs_read_lock(inode->i_sb);
 		branchget(inode->i_sb, bindex);
+		unionfs_read_unlock(inode->i_sb);
 	}
 
 	return 0;
@@ -443,7 +451,9 @@ static int __open_file(struct inode *inode, struct file *file)
 		return PTR_ERR(hidden_file);
 
 	unionfs_set_lower_file(file, hidden_file);
+	unionfs_read_lock(inode->i_sb);
 	branchget(inode->i_sb, bstart);
+	unionfs_read_unlock(inode->i_sb);
 
 	return 0;
 }
@@ -456,6 +466,7 @@ int unionfs_open(struct inode *inode, struct file *file)
 	int bindex = 0, bstart = 0, bend = 0;
 	int size;
 
+	unionfs_read_lock(inode->i_sb);
 	file->private_data = kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
 	if (!UNIONFS_F(file)) {
 		err = -ENOMEM;
@@ -481,7 +492,6 @@ int unionfs_open(struct inode *inode, struct file *file)
 
 	dentry = file->f_dentry;
 	unionfs_lock_dentry(dentry);
-	unionfs_read_lock(inode->i_sb);
 
 	bstart = fbstart(file) = dbstart(dentry);
 	bend = fbend(file) = dbend(dentry);
@@ -504,14 +514,15 @@ int unionfs_open(struct inode *inode, struct file *file)
 			if (!hidden_file)
 				continue;
 
+			unionfs_read_lock(file->f_dentry->d_sb);
 			branchput(file->f_dentry->d_sb, bindex);
+			unionfs_read_unlock(file->f_dentry->d_sb);
 			/* fput calls dput for hidden_dentry */
 			fput(hidden_file);
 		}
 	}
 
 	unionfs_unlock_dentry(dentry);
-	unionfs_read_unlock(inode->i_sb);
 
 out:
 	if (err) {
@@ -520,6 +531,7 @@ out:
 		kfree(UNIONFS_F(file));
 	}
 out_nofree:
+	unionfs_read_unlock(inode->i_sb);
 	return err;
 }
 
@@ -532,6 +544,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 	int bindex, bstart, bend;
 	int fgen;
 
+	unionfs_read_lock(inode->i_sb);
 	/* fput all the hidden files */
 	fgen = atomic_read(&fileinfo->generation);
 	bstart = fbstart(file);
@@ -565,6 +578,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 		fileinfo->rdstate = NULL;
 	}
 	kfree(fileinfo);
+	unionfs_read_unlock(inode->i_sb);
 	return 0;
 }
 
@@ -593,6 +607,7 @@ static long do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	}
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
@@ -600,6 +615,7 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	long err;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
 
@@ -635,8 +651,11 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 	struct dentry *dentry = file->f_dentry;
 	int bindex, bstart, bend;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
+
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
+
 	if (!atomic_dec_and_test(&UNIONFS_I(dentry->d_inode)->totalopens))
 		goto out;
 
@@ -664,6 +683,6 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 out_lock:
 	unionfs_unlock_dentry(dentry);
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
-
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 4ccef81..411553a 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -207,7 +207,9 @@ static int __copyup_reg_data(struct dentry *dentry,
 
 	/* open old file */
 	unionfs_mntget(dentry, old_bindex);
+	unionfs_read_lock(sb);
 	branchget(sb, old_bindex);
+	unionfs_read_unlock(sb);
 	input_file = dentry_open(old_hidden_dentry,
 				unionfs_lower_mnt_idx(dentry, old_bindex),
 				O_RDONLY | O_LARGEFILE);
@@ -224,7 +226,9 @@ static int __copyup_reg_data(struct dentry *dentry,
 	/* open new file */
 	dget(new_hidden_dentry);
 	unionfs_mntget(dentry, new_bindex);
+	unionfs_read_lock(sb);
 	branchget(sb, new_bindex);
+	unionfs_read_unlock(sb);
 	output_file = dentry_open(new_hidden_dentry,
 				unionfs_lower_mnt_idx(dentry, new_bindex),
 				O_WRONLY | O_LARGEFILE);
@@ -295,13 +299,17 @@ out_close_out:
 	fput(output_file);
 
 out_close_in2:
+	unionfs_read_lock(sb);
 	branchput(sb, new_bindex);
+	unionfs_read_unlock(sb);
 
 out_close_in:
 	fput(input_file);
 
 out:
+	unionfs_read_lock(sb);
 	branchput(sb, old_bindex);
+	unionfs_read_unlock(sb);
 
 	return err;
 }
@@ -350,8 +358,6 @@ static int copyup_named_dentry(struct inode *dir, struct dentry *dentry,
 
 	sb = dir->i_sb;
 
-	unionfs_read_lock(sb);
-
 	if ((err = is_robranch_super(sb, new_bindex)))
 		goto out;
 
@@ -443,7 +449,9 @@ out_unlink:
 		/* need to close the file */
 
 		fput(*copyup_file);
+		unionfs_read_lock(sb);
 		branchput(sb, new_bindex);
+		unionfs_read_unlock(sb);
 	}
 
 	/*
@@ -469,8 +477,6 @@ out_free:
 	kfree(symbuf);
 
 out:
-	unionfs_read_unlock(sb);
-
 	return err;
 }
 
diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c
index 8f568c7..6ff32a0 100644
--- a/fs/unionfs/dirfops.c
+++ b/fs/unionfs/dirfops.c
@@ -94,6 +94,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
 	int bend;
 	loff_t offset;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
 
@@ -175,6 +176,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
 		file->f_pos = rdstate2offset(uds);
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
@@ -192,6 +194,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
 	struct unionfs_dir_state *rdstate;
 	loff_t err;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
 
@@ -245,6 +248,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
 	}
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/dirhelper.c b/fs/unionfs/dirhelper.c
index bb5f7bc..bd15eb4 100644
--- a/fs/unionfs/dirhelper.c
+++ b/fs/unionfs/dirhelper.c
@@ -226,14 +226,18 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
 
 		dget(hidden_dentry);
 		unionfs_mntget(dentry, bindex);
+		unionfs_read_lock(sb);
 		branchget(sb, bindex);
+		unionfs_read_unlock(sb);
 		hidden_file =
 		    dentry_open(hidden_dentry, unionfs_lower_mnt_idx(dentry, bindex),
 				O_RDONLY);
 		if (IS_ERR(hidden_file)) {
 			err = PTR_ERR(hidden_file);
 			dput(hidden_dentry);
+			unionfs_read_lock(sb);
 			branchput(sb, bindex);
+			unionfs_read_unlock(sb);
 			goto out;
 		}
 
@@ -248,7 +252,9 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
 
 		/* fput calls dput for hidden_dentry */
 		fput(hidden_file);
+		unionfs_read_lock(sb);
 		branchput(sb, bindex);
+		unionfs_read_unlock(sb);
 
 		if (err < 0)
 			goto out;
diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index 9ce092d..84d6bab 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -27,6 +27,7 @@ static loff_t unionfs_llseek(struct file *file, loff_t offset, int origin)
 	loff_t err;
 	struct file *hidden_file = NULL;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
 
@@ -48,6 +49,7 @@ static loff_t unionfs_llseek(struct file *file, loff_t offset, int origin)
 		file->f_version++;
 	}
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
@@ -58,6 +60,7 @@ static ssize_t unionfs_read(struct file * file, char __user * buf,
 	loff_t pos = *ppos;
 	int err;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
 
@@ -70,6 +73,7 @@ static ssize_t unionfs_read(struct file * file, char __user * buf,
 	*ppos = pos;
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
@@ -123,12 +127,14 @@ static ssize_t unionfs_write(struct file * file, const char __user * buf,
 {
 	int err = 0;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
 
 	err = __unionfs_write(file, buf, count, ppos);
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
@@ -143,6 +149,7 @@ static unsigned int unionfs_poll(struct file *file, poll_table * wait)
 	unsigned int mask = DEFAULT_POLLMASK;
 	struct file *hidden_file = NULL;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if (unionfs_file_revalidate(file, 0)) {
 		/* We should pretend an error happend. */
 		mask = POLLERR | POLLIN | POLLOUT;
@@ -157,6 +164,7 @@ static unsigned int unionfs_poll(struct file *file, poll_table * wait)
 	mask = hidden_file->f_op->poll(hidden_file, wait);
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return mask;
 }
 
@@ -184,6 +192,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 	int err = 0;
 	int willwrite;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	/* This might could be deferred to mmap's writepage. */
 	willwrite = ((vma->vm_flags | VM_SHARED | VM_WRITE) == vma->vm_flags);
 	if ((err = unionfs_file_revalidate(file, willwrite)))
@@ -192,6 +201,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 	err = __do_mmap(file, vma);
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
@@ -200,6 +210,7 @@ static int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
 	int err;
 	struct file *hidden_file = NULL;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
 
@@ -215,6 +226,7 @@ static int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
 	mutex_unlock(&hidden_file->f_dentry->d_inode->i_mutex);
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
@@ -223,6 +235,7 @@ static int unionfs_fasync(int fd, struct file *file, int flag)
 	int err = 0;
 	struct file *hidden_file = NULL;
 
+	unionfs_read_lock(file->f_dentry->d_sb);
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
 
@@ -232,6 +245,7 @@ static int unionfs_fasync(int fd, struct file *file, int flag)
 		err = hidden_file->f_op->fasync(fd, hidden_file, flag);
 
 out:
+	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 1b2e8a8..6dfc16f 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -789,9 +789,13 @@ static int inode_permission(struct inode *inode, int mask, struct nameidata *nd,
 		retval = inode->i_op->permission(inode, submask, nd);
 		if ((retval == -EACCES) && (submask & MAY_WRITE) &&
 		    (!strcmp("nfs", (inode)->i_sb->s_type->name)) &&
-		    (nd) && (nd->mnt) && (nd->mnt->mnt_sb) &&
-		    (branchperms(nd->mnt->mnt_sb, bindex) & MAY_NFSRO)) {
-			retval = generic_permission(inode, submask, NULL);
+		    (nd) && (nd->mnt) && (nd->mnt->mnt_sb)) {
+			int perms;
+			unionfs_read_lock(nd->mnt->mnt_sb);
+			perms = branchperms(nd->mnt->mnt_sb, bindex);
+			unionfs_read_unlock(nd->mnt->mnt_sb);
+			if (perms & MAY_NFSRO)
+				retval = generic_permission(inode, submask, NULL);
 		}
 	} else
 		retval = generic_permission(inode, submask, NULL);
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index b80b554..4fffafa 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -556,11 +556,15 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
 
 		d = hidden_root_info->lower_paths[bindex].dentry;
 
+		unionfs_write_lock(sb);
 		unionfs_set_lower_super_idx(sb, bindex, d->d_sb);
+		unionfs_write_unlock(sb);
 	}
 
 	/* Unionfs: Max Bytes is the maximum bytes from highest priority branch */
+	unionfs_read_lock(sb);
 	sb->s_maxbytes = unionfs_lower_super_idx(sb, 0)->s_maxbytes;
+	unionfs_read_unlock(sb);
 
 	sb->s_op = &unionfs_sops;
 
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 7f0d174..5d11908 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -131,7 +131,9 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 	sb = dentry->d_sb;
 
+	unionfs_read_lock(sb);
 	hidden_sb = unionfs_lower_super_idx(sb, sbstart(sb));
+	unionfs_read_unlock(sb);
 	err = vfs_statfs(hidden_sb->s_root, buf);
 
 	buf->f_type = UNIONFS_SUPER_MAGIC;
@@ -884,7 +886,9 @@ static void unionfs_umount_begin(struct vfsmount *mnt, int flags)
 	bend = sbend(sb);
 	for (bindex = bstart; bindex <= bend; bindex++) {
 		hidden_mnt = unionfs_lower_mnt_idx(sb->s_root, bindex);
+		unionfs_read_lock(sb);
 		hidden_sb = unionfs_lower_super_idx(sb, bindex);
+		unionfs_read_unlock(sb);
 
 		if (hidden_mnt && hidden_sb && hidden_sb->s_op &&
 		    hidden_sb->s_op->umount_begin)
@@ -922,7 +926,9 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
 			goto out;
 		}
 
+		unionfs_read_lock(sb);
 		perms = branchperms(sb, bindex);
+		unionfs_read_unlock(sb);
 
 		seq_printf(m, "%s=%s", path,
 			   perms & MAY_WRITE ? "rw" : "ro");
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 66ffe88..faaa87e 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -301,7 +301,6 @@ extern int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 int unionfs_unlink(struct inode *dir, struct dentry *dentry);
 int unionfs_rmdir(struct inode *dir, struct dentry *dentry);
 
-int __unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd);
 int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd);
 
 /* The values for unionfs_interpose's flag. */
@@ -368,7 +367,11 @@ static inline int set_branchperms(struct super_block *sb, int index, int perms)
 /* Is this file on a read-only branch? */
 static inline int is_robranch_super(const struct super_block *sb, int index)
 {
-	return (!(branchperms(sb, index) & MAY_WRITE)) ? -EROFS : 0;
+	int ret;
+	unionfs_read_lock(sb);
+  	ret = (!(branchperms(sb, index) & MAY_WRITE)) ? -EROFS : 0;
+	unionfs_read_unlock(sb);
+	return ret;
 }
 
 /* Is this file on a read-only branch? */
@@ -378,10 +381,11 @@ static inline int is_robranch_idx(const struct dentry *dentry, int index)
 
 	BUG_ON(index < 0);
 
+	unionfs_read_lock(dentry->d_sb);
 	if ((!(branchperms(dentry->d_sb, index) & MAY_WRITE)) ||
 	    IS_RDONLY(unionfs_lower_dentry_idx(dentry, index)->d_inode))
 		err = -EROFS;
-
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
-- 
1.5.0.3.268.g3dda

[PATCH 13/21] Unionfs: Remove the older incgen ioctl

[Josef 'Jeff' Sipek]

The new remount code now has the "incgen" functionality.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/branchman.c  |   21 ---------------------
 fs/unionfs/commonfops.c |    6 ++----
 2 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/fs/unionfs/branchman.c b/fs/unionfs/branchman.c
index 83d443a..6912be9 100644
--- a/fs/unionfs/branchman.c
+++ b/fs/unionfs/branchman.c
@@ -18,27 +18,6 @@
 
 #include "union.h"
 
-/* increase the superblock generation count; effectively invalidating every
- * upper inode, dentry and file object */
-int unionfs_ioctl_incgen(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	struct super_block *sb;
-	int gen;
-
-	sb = file->f_dentry->d_sb;
-
-	unionfs_write_lock(sb);
-
-	gen = atomic_inc_return(&UNIONFS_SB(sb)->generation);
-
-	atomic_set(&UNIONFS_D(sb->s_root)->generation, gen);
-	atomic_set(&UNIONFS_I(sb->s_root->d_inode)->generation, gen);
-
-	unionfs_write_unlock(sb);
-
-	return gen;
-}
-
 /* return to userspace the branch indices containing the file in question
  *
  * We use fd_set and therefore we are limited to the number of the branches
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 6606cba..8453f2d 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -623,10 +623,8 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	switch (cmd) {
 	case UNIONFS_IOCTL_INCGEN:
 		/* Increment the superblock generation count */
-		err = -EACCES;
-		if (!capable(CAP_SYS_ADMIN))
-			goto out;
-		err = unionfs_ioctl_incgen(file, cmd, arg);
+		printk("unionfs: incgen ioctl deprecated; use \"-o remount,incgen\"\n");
+		err = -ENOSYS;
 		break;
 
 	case UNIONFS_IOCTL_QUERYFILE:
-- 
1.5.0.3.268.g3dda

[PATCH 14/21] Unionfs: Document unionfs_d_release locking

[Josef 'Jeff' Sipek]

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/dentry.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index c841f08..4756b78 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -286,6 +286,10 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 	return err;
 }
 
+/*
+ * At this point no one can reference this dentry, so we don't have to be
+ * careful about concurrent access.
+ */
 static void unionfs_d_release(struct dentry *dentry)
 {
 	int bindex, bstart, bend;
-- 
1.5.0.3.268.g3dda

[PATCH 15/21] Unionfs: Decrement totalopens counter on error in unionfs_open

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 8453f2d..c9df99d 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -509,6 +509,7 @@ int unionfs_open(struct inode *inode, struct file *file)
 
 	/* freeing the allocated resources, and fput the opened files */
 	if (err) {
+		atomic_dec(&UNIONFS_I(dentry->d_inode)->totalopens);
 		for (bindex = bstart; bindex <= bend; bindex++) {
 			hidden_file = unionfs_lower_file_idx(file, bindex);
 			if (!hidden_file)
-- 
1.5.0.3.268.g3dda

[PATCH 16/21] Unionfs: unionfs_create needs to revalidate the dentry

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

We have to read-lock the superblock rwsem, and we have to revalidate the
parent dentry and this one.  A branch-management operation could have taken
place, mid-way through a VFS operation that eventually reaches
unionfs_create().  So we have to ensure consistency, just as we do with the
file operations.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/inode.c |   29 ++++++++++++++++++++++++++++-
 1 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 6dfc16f..edd226f 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -28,9 +28,35 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
 	struct dentry *hidden_parent_dentry = NULL;
 	int bindex = 0, bstart;
 	char *name = NULL;
-
+	int valid = 0;
+
+	/*
+	 * We have to read-lock the superblock rwsem, and we have to
+	 * revalidate the parent dentry and this one.  A branch-management
+	 * operation could have taken place, mid-way through a VFS operation
+	 * that eventually reaches here.  So we have to ensure consistency,
+	 * just as we do with the file operations.
+	 *
+	 * XXX: we may need to do this for all other inode ops that take a
+	 * dentry.
+	 */
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
+	unionfs_lock_dentry(dentry->d_parent);
+	valid = __unionfs_d_revalidate_chain(dentry->d_parent, nd);
+	unionfs_unlock_dentry(dentry->d_parent);
+	if (!valid) {
+		err = -ENOENT;	/* same as what real_lookup does */
+		goto out;
+	}
+	valid = __unionfs_d_revalidate_chain(dentry, nd);
+	/*
+	 * It's only a bug if this dentry was not negative and couldn't be
+	 * revalidated (shouldn't happen).
+	 */
+	BUG_ON(!valid && dentry->d_inode);
+
 	/* We start out in the leftmost branch. */
 	bstart = dbstart(dentry);
 	hidden_dentry = unionfs_lower_dentry(dentry);
@@ -184,6 +210,7 @@ out:
 	kfree(name);
 
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
-- 
1.5.0.3.268.g3dda

[PATCH 17/21] Unionfs: vfsmount reference counting fixes

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/dentry.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 4756b78..b08842f 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -235,8 +235,8 @@ int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd)
 		valid = __unionfs_d_revalidate_one(chain[i], nd);
 		/* XXX: is this the correct mntput condition?! */
 		if (valid && chain_len > 0 &&
-		    sbgen != dgen && dentry->d_inode &&
-		    S_ISDIR(dentry->d_inode->i_mode)) {
+		    sbgen != dgen && chain[i]->d_inode &&
+		    S_ISDIR(chain[i]->d_inode->i_mode)) {
 			for (bindex = saved_bstart; bindex <= saved_bend; bindex++)
 				unionfs_mntput(chain[i], bindex);
 		}
@@ -256,9 +256,7 @@ int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd)
 	saved_bend = dbend(dentry);
 	valid = __unionfs_d_revalidate_one(dentry, nd);
 
-	if (valid && chain_len > 0 &&
-	    sbgen != dgen && dentry->d_inode &&
-	    S_ISDIR(dentry->d_inode->i_mode)) {
+	if (valid && chain_len > 0 && sbgen != dgen) {
 		for (bindex = saved_bstart; bindex <= saved_bend; bindex++)
 			unionfs_mntput(dentry, bindex);
 	}
-- 
1.5.0.3.268.g3dda

[PATCH 18/21] Unionfs: Pass lowernd to lower ->revalidate function

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/dentry.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index b08842f..9eb143d 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -150,8 +150,7 @@ static int __unionfs_d_revalidate_one(struct dentry *dentry, struct nameidata *n
 		if (!hidden_dentry || !hidden_dentry->d_op
 		    || !hidden_dentry->d_op->d_revalidate)
 			continue;
-
-		if (!hidden_dentry->d_op->d_revalidate(hidden_dentry, nd))
+		if (!hidden_dentry->d_op->d_revalidate(hidden_dentry, &lowernd))
 			valid = 0;
 	}
 
-- 
1.5.0.3.268.g3dda

[PATCH 19/21] Unionfs: Properly handle stale inodes passed to unionfs_permission

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/inode.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index edd226f..5c452b6 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -843,8 +843,21 @@ static int unionfs_permission(struct inode *inode, int mask,
 	const int is_file = !S_ISDIR(inode->i_mode);
 	const int write_mask = (mask & MAY_WRITE) && !(mask & MAY_READ);
 
+	unionfs_read_lock(inode->i_sb);
+
 	bstart = ibstart(inode);
 	bend = ibend(inode);
+	if (bstart < 0 || bend < 0) {
+		/*
+		 * With branch-management, we can get a stale inode here.
+		 * If so, we return ESTALE back to link_path_walk, which
+		 * would discard the dcache entry and re-lookup the
+		 * dentry+inode.  This should be equivalent to issuing
+		 * __unionfs_d_revalidate_chain on nd.dentry here.
+		 */
+		err = -ESTALE;	/* force revalidate */
+		goto out;
+	}
 
 	for (bindex = bstart; bindex <= bend; bindex++) {
 		hidden_inode = unionfs_lower_inode_idx(inode, bindex);
@@ -881,6 +894,7 @@ static int unionfs_permission(struct inode *inode, int mask,
 	}
 
 out:
+	unionfs_read_unlock(inode->i_sb);
 	return err;
 }
 
-- 
1.5.0.3.268.g3dda

[PATCH 20/21] Unionfs: Added several BUG_ONs to assert dentry validity

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

This should help catch races between the VFS and the branch-management code.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/inode.c  |   15 +++++++++++++++
 fs/unionfs/rename.c |    3 +++
 fs/unionfs/super.c  |    2 ++
 fs/unionfs/union.h  |   13 +++++++++++++
 fs/unionfs/unlink.c |    4 ++++
 fs/unionfs/xattr.c  |    8 ++++++++
 6 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 5c452b6..97dad8c 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -249,6 +249,9 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
 	struct dentry *whiteout_dentry;
 	char *name = NULL;
 
+	BUG_ON(!is_valid_dentry(new_dentry));
+	BUG_ON(!is_valid_dentry(old_dentry));
+
 	double_lock_dentry(new_dentry, old_dentry);
 
 	hidden_new_dentry = unionfs_lower_dentry(new_dentry);
@@ -373,6 +376,8 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
 	int bindex = 0, bstart;
 	char *name = NULL;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 
 	/* We start out in the leftmost branch. */
@@ -495,6 +500,8 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
 	int whiteout_unlinked = 0;
 	struct sioq_args args;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 	bstart = dbstart(dentry);
 
@@ -622,6 +629,8 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
 	char *name = NULL;
 	int whiteout_unlinked = 0;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 	bstart = dbstart(dentry);
 
@@ -724,6 +733,8 @@ static int unionfs_readlink(struct dentry *dentry, char __user * buf,
 	int err;
 	struct dentry *hidden_dentry;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
@@ -749,6 +760,8 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 	int len = PAGE_SIZE, err;
 	mm_segment_t old_fs;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	/* This is freed by the put_link method assuming a successful call. */
 	buf = kmalloc(len, GFP_KERNEL);
 	if (!buf) {
@@ -908,6 +921,8 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
 	int i;
 	int copyup = 0;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 	bstart = dbstart(dentry);
 	bend = dbend(dentry);
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index c9aa040..0044492 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -379,6 +379,9 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	int err = 0;
 	struct dentry *wh_dentry;
 
+	BUG_ON(!is_valid_dentry(old_dentry));
+	BUG_ON(!is_valid_dentry(new_dentry));
+
 	double_lock_dentry(old_dentry, new_dentry);
 
 	if (!S_ISDIR(old_dentry->d_inode->i_mode))
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 5d11908..ec5706b 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -129,6 +129,8 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	int err	= 0;
 	struct super_block *sb, *hidden_sb;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	sb = dentry->d_sb;
 
 	unionfs_read_lock(sb);
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index faaa87e..b6fa0a2 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -399,6 +399,19 @@ static inline int is_robranch(const struct dentry *dentry)
 	return is_robranch_idx(dentry, index);
 }
 
+/*
+ * Check if dentry is valid or not, as per our generation numbers.
+ * @dentry: dentry to check.
+ * Returns 1 (valid) or 0 (invalid/stale).
+ */
+static inline int is_valid_dentry(struct dentry *dentry)
+{
+	BUG_ON(!UNIONFS_D(dentry));
+	BUG_ON(!UNIONFS_SB(dentry->d_sb));
+	return (atomic_read(&UNIONFS_D(dentry)->generation) ==
+		atomic_read(&UNIONFS_SB(dentry->d_sb)->generation));
+}
+
 /* What do we use for whiteouts. */
 #define UNIONFS_WHPFX ".wh."
 #define UNIONFS_WHLEN 4
diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index f5b250a..dd1dd9c 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -74,6 +74,8 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
 {
 	int err = 0;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 
 	err = unionfs_unlink_whiteout(dir, dentry);
@@ -122,6 +124,8 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)
 	int err = 0;
 	struct unionfs_dir_state *namelist = NULL;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 
 	/* check if this unionfs directory is empty or not */
diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index d57f079..6e1f4bd 100644
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -56,6 +56,8 @@ ssize_t unionfs_getxattr(struct dentry * dentry, const char *name, void *value,
 	struct dentry *hidden_dentry = NULL;
 	int err = -EOPNOTSUPP;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 
 	hidden_dentry = unionfs_lower_dentry(dentry);
@@ -75,6 +77,8 @@ int unionfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 	struct dentry *hidden_dentry = NULL;
 	int err = -EOPNOTSUPP;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
@@ -92,6 +96,8 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
 	struct dentry *hidden_dentry = NULL;
 	int err = -EOPNOTSUPP;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
@@ -110,6 +116,8 @@ ssize_t unionfs_listxattr(struct dentry * dentry, char *list, size_t size)
 	int err = -EOPNOTSUPP;
 	char *encoded_list = NULL;
 
+	BUG_ON(!is_valid_dentry(dentry));
+
 	unionfs_lock_dentry(dentry);
 
 	hidden_dentry = unionfs_lower_dentry(dentry);
-- 
1.5.0.3.268.g3dda

[PATCH 21/21] Unionfs: Don't inline do_remount_{add,del,mode}_option

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@cs.sunysb.edu>

gcc4 decided to inline do_remount_{add,del,mode}_option creating an 600 byte
stack abuser on a x86_64 test box.

Reported by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/lookup.c |    2 +-
 fs/unionfs/super.c  |   20 ++++++++++----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 0572247..0fc5993 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -30,7 +30,7 @@ static int is_validname(const char *name)
 }
 
 /* The rest of these are utility functions for lookup. */
-static int is_opaque_dir(struct dentry *dentry, int bindex)
+static noinline int is_opaque_dir(struct dentry *dentry, int bindex)
 {
 	int err = 0;
 	struct dentry *hidden_dentry;
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index ec5706b..e6a6cc1 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -148,9 +148,9 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 }
 
 /* handle mode changing during remount */
-static int do_remount_mode_option(char *optarg, int cur_branches,
-				  struct unionfs_data *new_data,
-				  struct path *new_lower_paths)
+static noinline int do_remount_mode_option(char *optarg, int cur_branches,
+					   struct unionfs_data *new_data,
+					   struct path *new_lower_paths)
 {
 	int err = -EINVAL;
 	int perms, idx;
@@ -207,9 +207,9 @@ out:
 }
 
 /* handle branch deletion during remount */
-static int do_remount_del_option(char *optarg, int cur_branches,
-				 struct unionfs_data *new_data,
-				 struct path *new_lower_paths)
+static noinline int do_remount_del_option(char *optarg, int cur_branches,
+					  struct unionfs_data *new_data,
+					  struct path *new_lower_paths)
 {
 	int err = -EINVAL;
 	int idx;
@@ -268,10 +268,10 @@ out:
 }
 
 /* handle branch insertion during remount */
-static int do_remount_add_option(char *optarg, int cur_branches,
-				 struct unionfs_data *new_data,
-				 struct path *new_lower_paths,
-				 int *high_branch_id)
+static noinline int do_remount_add_option(char *optarg, int cur_branches,
+					  struct unionfs_data *new_data,
+					  struct path *new_lower_paths,
+					  int *high_branch_id)
 {
 	int err = -EINVAL;
 	int perms;
-- 
1.5.0.3.268.g3dda

Re: [GIT PULL -mm] Unionfs branch management code

[Andrew Morton]

On Mon,  9 Apr 2007 10:53:51 -0400 "Josef 'Jeff' Sipek" <jsipek@cs.sunysb.edu> wrote:

> The following patches introduce new branch-management code into Unionfs as
> well as fix a number of stability issues and resource leaks.

I have a mental note that unionfs is in the "stuck" state, due to general
agreement that we should implement this functionality at the VFS level, one
reason for which is unionfs's upper-vs-lower coherency problems.

Am I wrong?

Re: [GIT PULL -mm] Unionfs branch management code

[Josef Sipek]

On Mon, Apr 09, 2007 at 10:49:48AM -0700, Andrew Morton wrote:
> On Mon,  9 Apr 2007 10:53:51 -0400 "Josef 'Jeff' Sipek" <jsipek@cs.sunysb.edu> wrote:
> 
> > The following patches introduce new branch-management code into Unionfs as
> > well as fix a number of stability issues and resource leaks.

First, a quick note...Unionfs used to have branch management code, but the
code was so crufty that we decided to spare the eyes (and brains) of kernel
developers at large by ripping it out. This series of patches just
reintroduces the functionality in a sane way. With that said...

> I have a mental note that unionfs is in the "stuck" state, due to general
> agreement that we should implement this functionality at the VFS level, one
> reason for which is unionfs's upper-vs-lower coherency problems.

Right. The upper-vs-lower coherency problem is indeed a problem, but it is
not a _Unionfs_ problem, but rather a _stackable filesystems_ problem
(eCrypfs suffers from the same issue people are just less likely to trip
over it as no one in their right mind modifies encrypted data by hand). If
we hope to have Linux do stacking (which, I think makes sense), we need to
make few changes to the kernel to allow stackable filesystems to work
better, and safer. We're working on an OLS paper which discusses some of
these ideas (some of which we got at LSF back in February) - for example, do
we want to have strong or weak cache coherency? (When the lower pages
change, do we want to have the VM enforce coherency, or can we use more of
an NFS-like coherency model - checking {a,c,m}time.)

Josef "Jeff" Sipek.

-- 
Intellectuals solve problems; geniuses prevent them
		- Albert Einstein

Re: [GIT PULL -mm] Unionfs branch management code

[Shaya Potter]

Andrew Morton wrote:
> On Mon,  9 Apr 2007 10:53:51 -0400 "Josef 'Jeff' Sipek" <jsipek@cs.sunysb.edu> wrote:
> 
>> The following patches introduce new branch-management code into Unionfs as
>> well as fix a number of stability issues and resource leaks.
> 
> I have a mental note that unionfs is in the "stuck" state, due to general
> agreement that we should implement this functionality at the VFS level, one
> reason for which is unionfs's upper-vs-lower coherency problems.

How can a union file system with a decent set of useful semantics be 
fully implemented at the VFS layer in a "clean" manner?

For instance, a major use of unionfs is live CDs, namely unionfs w/ a 
read-only and read-write layer.  Unionfs enables files to be copied up 
from the read-only layer to the read-write layer.

Does one really want to implement "copyup" in the VFS?

just my 2 agarot.

Re: [GIT PULL -mm] Unionfs branch management code

[Josef Sipek]

On Tue, Apr 10, 2007 at 01:22:52PM -0400, Shaya Potter wrote:
> Andrew Morton wrote:
> >On Mon,  9 Apr 2007 10:53:51 -0400 "Josef 'Jeff' Sipek" 
> ><jsipek@cs.sunysb.edu> wrote:
> >
> >>The following patches introduce new branch-management code into Unionfs as
> >>well as fix a number of stability issues and resource leaks.
> >
> >I have a mental note that unionfs is in the "stuck" state, due to general
> >agreement that we should implement this functionality at the VFS level, one
> >reason for which is unionfs's upper-vs-lower coherency problems.
> 
> How can a union file system with a decent set of useful semantics be 
> fully implemented at the VFS layer in a "clean" manner?

Unioning is quite odd. It uses concepts, some of which do indeed belong in
the VFS (like actual merging of the lower directories), but others that most
definitely do not (like whiteouts).

> For instance, a major use of unionfs is live CDs, namely unionfs w/ a 
> read-only and read-write layer.  Unionfs enables files to be copied up 
> from the read-only layer to the read-write layer.
> 
> Does one really want to implement "copyup" in the VFS?
 
I don't think that copyup is the problem, but whiteouts...oh boy.
Whiteouts/some kind of persistent storage is most definitely a filesystem
construct, and it does not belong in the VFS.

Josef "Jeff" Sipek.

-- 
If I have trouble installing Linux, something is wrong. Very wrong.
		- Linus Torvalds

Re: [GIT PULL -mm] Unionfs branch management code

[Jan Engelhardt]

On Apr 9 2007 10:53, Josef 'Jeff' Sipek wrote:
>
>The following patches introduce new branch-management code into Unionfs as
>well as fix a number of stability issues and resource leaks.  For detailed
>announcement, see end of this email.

I have to seriously ask: why don't we consider aufs? Without
readvertising its simple codebase, unionfs was (version 1.5pre) not
able to be used as a root filesystem when one of its branches is nfs.
(I figure I should build a testcase for that know that I had
a way to reproduce it for some time.)


Jan
--