linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL -mm] Unionfs cleanups, fixes, and mmap
@ 2007-06-17 19:09 Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 01/16] [PATCH] unionfs section mismatch Josef 'Jeff' Sipek
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm

The following patches consist of mostly cleanups and bug fixes of the
Unionfs code. The major improvement is the new mmap implementation, as well
as a major locking overhaul.

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 16 new commits:

Erez Zadok (9):
      Unionfs: Don't revalidate dropped dentries
      Unionfs: Retry lookup for different silly-renamed files
      Unionfs: Set lower inodes correctly after branch management succeeds
      Unionfs: call statfs on lower file system properly
      MAINTAINERS: Add Erez Zadok as a maintainer of Unionfs
      Unionfs: Add standard copyright comment to include/linux/union_fs.h
      Unionfs: Remove unnecessary #define
      Unionfs: merge find_new_branch_index and branch_id_to_idx into one function
      Unionfs: Revalidate dentries passed to all inode/super operations

Josef 'Jeff' Sipek (5):
      Unionfs: Cleanup new_dentry_private_data
      Unionfs: Change free_dentry_private_info to take a struct dentry
      Unionfs: Add BUG_ONs to unionfs_lower_*
      Unionfs: Change the semantics of sb info's rwsem
      Unionfs: Remove superfluous check for NULL pointer

Randy Dunlap (1):
      unionfs section mismatch

Yiannis Pericleous (1):
      Unionfs: mmap implementation

Thanks,

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 01/16] [PATCH] unionfs section mismatch
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 02/16] Unionfs: Don't revalidate dropped dentries Josef 'Jeff' Sipek
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel
  Cc: akpm, Randy Dunlap, Josef 'Jeff' Sipek

From: Randy Dunlap <randy.dunlap@oracle.com>

Fix section marker in header file:

WARNING: fs/unionfs/unionfs.o(.init.text+0x56): Section mismatch: reference to .exit.text:stop_sioq (between 'init_module' and 'init_sioq')

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

diff --git a/fs/unionfs/sioq.h b/fs/unionfs/sioq.h
index 55b781e..6d0d01f 100644
--- a/fs/unionfs/sioq.h
+++ b/fs/unionfs/sioq.h
@@ -76,7 +76,7 @@ struct sioq_args {
 
 /* Extern definitions for SIOQ functions */
 extern int __init init_sioq(void);
-extern __exit void stop_sioq(void);
+extern void stop_sioq(void);
 extern void run_sioq(work_func_t func, struct sioq_args *args);
 
 /* Extern definitions for our privilege escalation helpers */
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 02/16] Unionfs: Don't revalidate dropped dentries
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 01/16] [PATCH] unionfs section mismatch Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 03/16] Unionfs: Retry lookup for different silly-renamed files Josef 'Jeff' Sipek
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, Josef 'Jeff' Sipek

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

This fixes a harmless but annoying message that unionfs prints if a dropped
dentry is being revalidated, which could happen if you unlink open files.

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

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 1045e51..89a236b 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -305,9 +305,12 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
 	unionfs_lock_dentry(dentry);
 	sb = dentry->d_sb;
 
-	/* first revalidate the dentry inside struct file */
-	if (!__unionfs_d_revalidate_chain(dentry, NULL) &&
-	    !d_deleted(dentry)) {
+	/*
+	 * First revalidate the dentry inside struct file,
+	 * but not unhashed dentries.
+	 */
+	if (!d_deleted(dentry) &&
+	    !__unionfs_d_revalidate_chain(dentry, NULL)) {
 		err = -ESTALE;
 		goto out_nofree;
 	}
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 03/16] Unionfs: Retry lookup for different silly-renamed files
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 01/16] [PATCH] unionfs section mismatch Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 02/16] Unionfs: Don't revalidate dropped dentries Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 04/16] Unionfs: Set lower inodes correctly after branch management succeeds Josef 'Jeff' Sipek
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, Josef 'Jeff' Sipek

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

When we have to copyup an open-but-unlinked file, we have to give it a
temporary name, similar to NFS's silly-renamed files.  So we generate
temporary file names until we find one that doesn't exist, and use it.  The
code had a bug where if the silly-renamed file name already existed, Unionfs
would oops upon copyup to that temp name.

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

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 89a236b..28635d8 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -42,6 +42,7 @@ static int copyup_deleted_file(struct file *file, struct dentry *dentry,
 	sprintf(name, ".unionfs%*.*lx",
 		i_inosize, i_inosize, hidden_dentry->d_inode->i_ino);
 
+retry:
 	/*
 	 * Loop, looking for an unused temp name to copyup to.
 	 *
@@ -68,13 +69,14 @@ static int copyup_deleted_file(struct file *file, struct dentry *dentry,
 			err = PTR_ERR(tmp_dentry);
 			goto out;
 		}
-		/* don't dput here because of do-while condition eval order */
 	} while (tmp_dentry->d_inode != NULL);	/* need negative dentry */
 	dput(tmp_dentry);
 
 	err = copyup_named_file(dentry->d_parent->d_inode, file, name, bstart,
 				bindex, file->f_dentry->d_inode->i_size);
-	if (err)
+	if (err == -EEXIST)
+		goto retry;
+	else if (err)
 		goto out;
 
 	/* bring it to the same state as an unlinked file */
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 04/16] Unionfs: Set lower inodes correctly after branch management succeeds
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (2 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 03/16] Unionfs: Retry lookup for different silly-renamed files Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 05/16] Unionfs: call statfs on lower file system properly Josef 'Jeff' Sipek
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, 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/super.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index fe02941..b3a5e64 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -335,7 +335,13 @@ found_insertion_point:
 		       new_branch, err);
 		goto out;
 	}
-	/* it's probably safe to check_mode the new branch to insert */
+	/*
+	 * It's probably safe to check_mode the new branch to insert.  Note:
+	 * we don't allow inserting branches which are unionfs's by
+	 * themselves (check_branch returns EINVAL in that case).  This is
+	 * because this code base doesn't support stacking unionfs: the ODF
+	 * code base supports that correctly.
+	 */
 	if ((err = check_branch(&nd))) {
 		printk(KERN_WARNING "unionfs: hidden directory "
 		       "\"%s\" is not a valid branch\n", optarg);
@@ -400,15 +406,17 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
 	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 cur_branches = 0;	/* no. of current branches */
+	int new_branches = 0;	/* 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;
+	struct inode **new_lower_inodes = NULL;
 	int new_high_branch_id;	/* new high branch ID */
 	int size;		/* memory allocation size, temp var */
+	int old_ibstart, old_ibend;
 
 	unionfs_write_lock(sb);
 
@@ -640,6 +648,14 @@ out_no_change:
 		goto out_release;
 	}
 
+	/* allocate space for new pointers to lower inodes */
+	new_lower_inodes = kcalloc(new_branches,
+				   sizeof(struct inode *), GFP_KERNEL);
+	if (!new_lower_inodes) {
+		err = -ENOMEM;
+		goto out_release;
+	}
+
 	/*
 	 * 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.
@@ -660,7 +676,7 @@ out_no_change:
 	 * 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
+	 * But do we 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
@@ -687,8 +703,31 @@ out_no_change:
 	i = sbmax(sb);		/* save no. of branches to release at end */
 	sbend(sb) = new_branches - 1;
 	set_dbend(sb->s_root, new_branches - 1);
+	old_ibstart = ibstart(sb->s_root->d_inode);
+	old_ibend = ibend(sb->s_root->d_inode);
+	ibend(sb->s_root->d_inode) = new_branches - 1;
 	UNIONFS_D(sb->s_root)->bcount = new_branches;
-	new_branches = i;	/* no. of branches to release below */
+	new_branches = i; /* no. of branches to release below */
+
+	/*
+	 * Update lower inodes: 3 steps
+	 * 1. grab ref on all new lower inodes
+	 */
+	for (i=dbstart(sb->s_root); i<=dbend(sb->s_root); i++) {
+		struct dentry *lower_dentry =
+			unionfs_lower_dentry_idx(sb->s_root, i);
+		atomic_inc(&lower_dentry->d_inode->i_count);
+		new_lower_inodes[i] = lower_dentry->d_inode;
+	}
+	/* 2. release reference on all older lower inodes */
+	for (i=old_ibstart; i<=old_ibend; i++) {
+		iput(unionfs_lower_inode_idx(sb->s_root->d_inode, i));
+		unionfs_set_lower_inode_idx(sb->s_root->d_inode, i, NULL);
+	}
+	kfree(UNIONFS_I(sb->s_root->d_inode)->lower_inodes);
+	/* 3. update root dentry's inode to new lower_inodes array */
+	UNIONFS_I(sb->s_root->d_inode)->lower_inodes = new_lower_inodes;
+	new_lower_inodes = NULL;
 
 	/* maxbytes may have changed */
 	sb->s_maxbytes = unionfs_lower_super_idx(sb, 0)->s_maxbytes;
@@ -723,6 +762,7 @@ out_free:
 	kfree(tmp_data);
 	kfree(new_lower_paths);
 	kfree(new_data);
+	kfree(new_lower_inodes);
 out_error:
 	unionfs_write_unlock(sb);
 	return err;
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 05/16] Unionfs: call statfs on lower file system properly
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (3 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 04/16] Unionfs: Set lower inodes correctly after branch management succeeds Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 06/16] MAINTAINERS: Add Erez Zadok as a maintainer of Unionfs Josef 'Jeff' Sipek
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, Josef 'Jeff' Sipek

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

Get the correct lower dentry to use to statfs the first branch (always),

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

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index b3a5e64..a7ff06c 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -112,18 +112,23 @@ static void unionfs_put_super(struct super_block *sb)
 static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 {
 	int err	= 0;
-	struct super_block *sb, *hidden_sb;
+	struct super_block *sb;
+	struct dentry *lower_dentry;
 
 	BUG_ON(!is_valid_dentry(dentry));
 
 	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);
+	lower_dentry = unionfs_lower_dentry(sb->s_root);
+	err = vfs_statfs(lower_dentry, buf);
 
+	/* set return buf to our f/s to avoid confusing user-level utils */
 	buf->f_type = UNIONFS_SUPER_MAGIC;
+
+	/*
+	 * Our maximum file name can is shorter by a few bytes because every
+	 * file name could potentially be whited-out.
+	 */
 	buf->f_namelen -= UNIONFS_WHLEN;
 
 	memset(&buf->f_fsid, 0, sizeof(__kernel_fsid_t));
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 06/16] MAINTAINERS: Add Erez Zadok as a maintainer of Unionfs
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (4 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 05/16] Unionfs: call statfs on lower file system properly Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 07/16] Unionfs: Add standard copyright comment to include/linux/union_fs.h Josef 'Jeff' Sipek
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, 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>
---
 MAINTAINERS |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 905dab1..e09ad36 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3594,6 +3594,8 @@ W:	http://www.kernel.dk
 S:	Maintained
 
 UNIONFS
+P:	Erez Zadok
+M:	ezk@cs.sunysb.edu
 P:	Josef "Jeff" Sipek
 M:	jsipek@cs.sunysb.edu
 L:	unionfs@filesystems.org
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 07/16] Unionfs: Add standard copyright comment to include/linux/union_fs.h
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (5 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 06/16] MAINTAINERS: Add Erez Zadok as a maintainer of Unionfs Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 08/16] Unionfs: Remove unnecessary #define Josef 'Jeff' Sipek
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, 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>
---
 include/linux/union_fs.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/union_fs.h b/include/linux/union_fs.h
index b724031..223ccab 100644
--- a/include/linux/union_fs.h
+++ b/include/linux/union_fs.h
@@ -1,3 +1,14 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
 #ifndef _LINUX_UNION_FS_H
 #define _LINUX_UNION_FS_H
 
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 08/16] Unionfs: Remove unnecessary #define
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (6 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 07/16] Unionfs: Add standard copyright comment to include/linux/union_fs.h Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 09/16] Unionfs: mmap implementation Josef 'Jeff' Sipek
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, Josef 'Jeff' Sipek

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

UNIONFS_TMPNAM_LEN is used in only one place, and we have calculate the
length of the string to begin with.

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

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 28635d8..db8c334 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -64,7 +64,7 @@ retry:
 		       dentry->d_name.name, name);
 
 		tmp_dentry = lookup_one_len(name, hidden_dentry->d_parent,
-					    UNIONFS_TMPNAM_LEN);
+					    nlen);
 		if (IS_ERR(tmp_dentry)) {
 			err = PTR_ERR(tmp_dentry);
 			goto out;
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 335d579..01e29f3 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -54,9 +54,6 @@
 /* unionfs root inode number */
 #define UNIONFS_ROOT_INO     1
 
-/* number of characters while generating unique temporary file names */
-#define	UNIONFS_TMPNAM_LEN	12
-
 /* number of times we try to get a unique temporary file name */
 #define GET_TMPNAM_MAX_RETRY	5
 
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 09/16] Unionfs: mmap implementation
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (7 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 08/16] Unionfs: Remove unnecessary #define Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 10/16] Unionfs: merge find_new_branch_index and branch_id_to_idx into one function Josef 'Jeff' Sipek
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel
  Cc: akpm, Yiannis Pericleous, Shaya Potter, Erez Zadok,
	Josef 'Jeff' Sipek

From: Yiannis Pericleous <yiannos@fsl.cs.sunysb.edu>

Signed-off-by: Shaya Potter <spotter@cs.columbia.edu>
Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Yiannis Pericleous <yiannos@fsl.cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/Makefile     |    2 +-
 fs/unionfs/commonfops.c |   19 ++-
 fs/unionfs/copyup.c     |    5 +
 fs/unionfs/file.c       |  214 +++++++----------------------
 fs/unionfs/inode.c      |    9 ++
 fs/unionfs/main.c       |    6 -
 fs/unionfs/mmap.c       |  348 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/unionfs/super.c      |    8 +-
 fs/unionfs/union.h      |    1 +
 9 files changed, 438 insertions(+), 174 deletions(-)
 create mode 100644 fs/unionfs/mmap.c

diff --git a/fs/unionfs/Makefile b/fs/unionfs/Makefile
index 6986d79..78be3e7 100644
--- a/fs/unionfs/Makefile
+++ b/fs/unionfs/Makefile
@@ -2,6 +2,6 @@ obj-$(CONFIG_UNION_FS) += unionfs.o
 
 unionfs-y := subr.o dentry.o file.o inode.o main.o super.o \
 	rdstate.o copyup.o dirhelper.o rename.o unlink.o \
-	lookup.o commonfops.o dirfops.o sioq.o
+	lookup.o commonfops.o dirfops.o sioq.o mmap.o
 
 unionfs-$(CONFIG_UNION_FS_XATTR) += xattr.o
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index db8c334..0222393 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -571,12 +571,25 @@ out_nofree:
 int unionfs_file_release(struct inode *inode, struct file *file)
 {
 	struct file *hidden_file = NULL;
-	struct unionfs_file_info *fileinfo = UNIONFS_F(file);
-	struct unionfs_inode_info *inodeinfo = UNIONFS_I(inode);
+	struct unionfs_file_info *fileinfo;
+	struct unionfs_inode_info *inodeinfo;
+	struct super_block *sb = inode->i_sb;
 	int bindex, bstart, bend;
 	int fgen;
+	int err;
+
+	unionfs_read_lock(sb);
+	/*
+	 * Yes, we have to revalidate this file even if it's being released.
+	 * This is important for open-but-unlinked files, as well as mmap
+	 * support.
+	 */
+	if ((err = unionfs_file_revalidate(file, 1)))
+		return err;
+	fileinfo = UNIONFS_F(file);
+	BUG_ON(file->f_dentry->d_inode != inode);
+	inodeinfo = UNIONFS_I(inode);
 
-	unionfs_read_lock(inode->i_sb);
 	/* fput all the hidden files */
 	fgen = atomic_read(&fileinfo->generation);
 	bstart = fbstart(file);
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index a80ece6..dff4f1c 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -291,8 +291,13 @@ static int __copyup_reg_data(struct dentry *dentry,
 
 	kfree(buf);
 
+	if (!err)
+		err = output_file->f_op->fsync(output_file,
+					       new_hidden_dentry, 0);
+
 	if (err)
 		goto out_close_out;
+
 	if (copyup_file) {
 		*copyup_file = output_file;
 		goto out_close_in;
diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index 2e5ec42..afffe59 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -22,219 +22,110 @@
  * File Operations *
  *******************/
 
-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;
-
-	hidden_file = unionfs_lower_file(file);
-	/* always set hidden position to this one */
-	hidden_file->f_pos = file->f_pos;
-
-	memcpy(&hidden_file->f_ra, &file->f_ra, sizeof(struct file_ra_state));
-
-	if (hidden_file->f_op && hidden_file->f_op->llseek)
-		err = hidden_file->f_op->llseek(hidden_file, offset, origin);
-	else
-		err = generic_file_llseek(hidden_file, offset, origin);
-
-	if (err < 0)
-		goto out;
-	if (err != file->f_pos) {
-		file->f_pos = err;
-		file->f_version++;
-	}
-out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
-	return err;
-}
-
 static ssize_t unionfs_read(struct file *file, char __user *buf,
 			    size_t count, loff_t *ppos)
 {
-	struct file *hidden_file;
-	loff_t pos = *ppos;
 	int err;
 
 	unionfs_read_lock(file->f_dentry->d_sb);
+
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
 
-	err = -EINVAL;
-	hidden_file = unionfs_lower_file(file);
-	if (!hidden_file->f_op || !hidden_file->f_op->read)
-		goto out;
+	err = do_sync_read(file, buf, count, ppos);
 
-	err = hidden_file->f_op->read(hidden_file, buf, count, &pos);
-	*ppos = pos;
+	if (err >= 0)
+		touch_atime(unionfs_lower_mnt(file->f_path.dentry),
+			    unionfs_lower_dentry(file->f_path.dentry));
 
 out:
 	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
 
-static ssize_t unionfs_write(struct file *file, const char __user *buf,
-			     size_t count, loff_t *ppos)
+static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
+				unsigned long nr_segs, loff_t pos)
 {
-	int err;
-	struct file *hidden_file = NULL;
-	struct inode *inode;
-	struct inode *hidden_inode;
-	loff_t pos = *ppos;
-	int bstart, bend;
+	int err = 0;
+	struct file *file = iocb->ki_filp;
 
 	unionfs_read_lock(file->f_dentry->d_sb);
-	if ((err = unionfs_file_revalidate(file, 1)))
-		goto out;
-
-	inode = file->f_dentry->d_inode;
-
-	bstart = fbstart(file);
-	bend = fbend(file);
 
-	BUG_ON(bstart == -1);
-
-	hidden_file = unionfs_lower_file(file);
-	hidden_inode = hidden_file->f_dentry->d_inode;
-
-	if (!hidden_file->f_op || !hidden_file->f_op->write) {
-		err = -EINVAL;
+	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
-	}
 
-	/* adjust for append -- seek to the end of the file */
-	if (file->f_flags & O_APPEND)
-		pos = inode->i_size;
+	err = generic_file_aio_read(iocb, iov, nr_segs, pos);
 
-	err = hidden_file->f_op->write(hidden_file, buf, count, &pos);
+	if (err == -EIOCBQUEUED)
+		err = wait_on_sync_kiocb(iocb);
 
-	/*
-	 * copy ctime and mtime from lower layer attributes
-	 * atime is unchanged for both layers
-	 */
 	if (err >= 0)
-		fsstack_copy_attr_times(inode, hidden_inode);
-
-	*ppos = pos;
+		touch_atime(unionfs_lower_mnt(file->f_path.dentry),
+			    unionfs_lower_dentry(file->f_path.dentry));
 
-	/* update this inode's size */
-	if (pos > inode->i_size)
-		inode->i_size = pos;
 out:
 	unionfs_read_unlock(file->f_dentry->d_sb);
 	return err;
 }
-
-static int unionfs_file_readdir(struct file *file, void *dirent,
-				filldir_t filldir)
-{
-	return -ENOTDIR;
-}
-
-static unsigned int unionfs_poll(struct file *file, poll_table *wait)
+static ssize_t unionfs_write(struct file * file, const char __user * buf,
+			     size_t count, loff_t *ppos)
 {
-	unsigned int mask = DEFAULT_POLLMASK;
-	struct file *hidden_file = NULL;
+	int err = 0;
 
 	unionfs_read_lock(file->f_dentry->d_sb);
-	if (unionfs_file_revalidate(file, 0)) {
-		/* We should pretend an error happened. */
-		mask = POLLERR | POLLIN | POLLOUT;
-		goto out;
-	}
-
-	hidden_file = unionfs_lower_file(file);
 
-	if (!hidden_file->f_op || !hidden_file->f_op->poll)
+	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
 
-	mask = hidden_file->f_op->poll(hidden_file, wait);
+	err = do_sync_write(file, buf, count, ppos);
 
 out:
 	unionfs_read_unlock(file->f_dentry->d_sb);
-	return mask;
+	return err;
 }
 
-static int __do_mmap(struct file *file, struct vm_area_struct *vma)
+static int unionfs_file_readdir(struct file *file, void *dirent,
+				filldir_t filldir)
 {
-	int err;
-	struct file *hidden_file;
-
-	hidden_file = unionfs_lower_file(file);
-
-	err = -ENODEV;
-	if (!hidden_file->f_op || !hidden_file->f_op->mmap)
-		goto out;
-
-	vma->vm_file = hidden_file;
-	err = hidden_file->f_op->mmap(hidden_file, vma);
-	get_file(hidden_file);	/* make sure it doesn't get freed on us */
-	fput(file);		/* no need to keep extra ref on ours */
-out:
-	return err;
+	return -ENOTDIR;
 }
 
 static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	int err = 0;
 	int willwrite;
+	struct file *lower_file;
 
 	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)))
-		goto out;
-
-	err = __do_mmap(file, vma);
-
-out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
-	return err;
-}
-
-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;
 
-	hidden_file = unionfs_lower_file(file);
-
-	err = -EINVAL;
-	if (!hidden_file->f_op || !hidden_file->f_op->fsync)
-		goto out;
-
-	mutex_lock(&hidden_file->f_dentry->d_inode->i_mutex);
-	err = hidden_file->f_op->fsync(hidden_file, hidden_file->f_dentry,
-				       datasync);
-	mutex_unlock(&hidden_file->f_dentry->d_inode->i_mutex);
-
-out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
-	return err;
-}
-
-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)))
+	/* This might be deferred to mmap's writepage */
+	willwrite = ((vma->vm_flags | VM_SHARED | VM_WRITE) == vma->vm_flags);
+	if ((err = unionfs_file_revalidate(file, willwrite)))
 		goto out;
 
-	hidden_file = unionfs_lower_file(file);
-
-	if (hidden_file->f_op && hidden_file->f_op->fasync)
-		err = hidden_file->f_op->fasync(fd, hidden_file, flag);
+	/*
+	 * File systems which do not implement ->writepage may use
+	 * generic_file_readonly_mmap as their ->mmap op.  If you call
+	 * generic_file_readonly_mmap with VM_WRITE, you'd get an -EINVAL.
+	 * But we cannot call the lower ->mmap op, so we can't tell that
+	 * writeable mappings won't work.  Therefore, our only choice is to
+	 * check if the lower file system supports the ->writepage, and if
+	 * not, return EINVAL (the same error that
+	 * generic_file_readonly_mmap returns in that case).
+	 */
+	lower_file = unionfs_lower_file(file);
+	if (willwrite && !lower_file->f_mapping->a_ops->writepage) {
+		err = -EINVAL;
+		printk("unionfs: branch %d file system does not support "
+		       "writeable mmap\n", fbstart(file));
+	} else {
+		err = generic_file_mmap(file, vma);
+		if (err)
+			printk("unionfs: generic_file_mmap failed %d\n", err);
+	}
 
 out:
 	unionfs_read_unlock(file->f_dentry->d_sb);
@@ -242,16 +133,17 @@ out:
 }
 
 struct file_operations unionfs_main_fops = {
-	.llseek		= unionfs_llseek,
+	.llseek		= generic_file_llseek,
 	.read		= unionfs_read,
+	.aio_read       = unionfs_aio_read,
 	.write		= unionfs_write,
+	.aio_write      = generic_file_aio_write,
 	.readdir	= unionfs_file_readdir,
-	.poll		= unionfs_poll,
 	.unlocked_ioctl	= unionfs_ioctl,
 	.mmap		= unionfs_mmap,
 	.open		= unionfs_open,
 	.flush		= unionfs_flush,
 	.release	= unionfs_file_release,
-	.fsync		= unionfs_fsync,
-	.fasync		= unionfs_fasync,
+	.fsync		= file_fsync,
+	.sendfile	= generic_file_sendfile,
 };
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 627c2a7..9f1acc4 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -1018,6 +1018,15 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
 		break;
 	}
 
+	/* for mmap */
+	if (ia->ia_valid & ATTR_SIZE) {
+		if (ia->ia_size != i_size_read(inode)) {
+			err = vmtruncate(inode, ia->ia_size);
+			if (err)
+				printk("unionfs_setattr: vmtruncate failed\n");
+		}
+	}
+
 	/* get the size from the first hidden inode */
 	hidden_inode = unionfs_lower_inode(dentry->d_inode);
 	fsstack_copy_attr_all(inode, hidden_inode, unionfs_get_nlinks);
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index a9ad445..2bcc84c 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -121,12 +121,6 @@ int unionfs_interpose(struct dentry *dentry, struct super_block *sb, int flag)
 	    S_ISFIFO(hidden_inode->i_mode) || S_ISSOCK(hidden_inode->i_mode))
 		init_special_inode(inode, hidden_inode->i_mode,
 				   hidden_inode->i_rdev);
-	/*
-	 * Fix our inode's address operations to that of the lower inode
-	 * (Unionfs is FiST-Lite)
-	 */
-	if (inode->i_mapping->a_ops != hidden_inode->i_mapping->a_ops)
-		inode->i_mapping->a_ops = hidden_inode->i_mapping->a_ops;
 
 	/* all well, copy inode attributes */
 	fsstack_copy_attr_all(inode, hidden_inode, unionfs_get_nlinks);
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
new file mode 100644
index 0000000..997b619
--- /dev/null
+++ b/fs/unionfs/mmap.c
@@ -0,0 +1,348 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2006      Shaya Potter
+ * Copyright (c) 2005      Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003      Puja Gupta
+ * Copyright (c) 2003      Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of State University of New York
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/*
+ * Unionfs doesn't implement ->writepages, which is OK with the VFS and
+ * nkeeps our code simpler and smaller.  Nevertheless, somehow, our own
+ * ->writepage must be called so we can sync the upper pages with the lower
+ * pages: otherwise data changed at the upper layer won't get written to the
+ * lower layer.
+ *
+ * Some lower file systems (e.g., NFS) expect the VFS to call its writepages
+ * only, which in turn will call generic_writepages and invoke each of the
+ * lower file system's ->writepage.  NFS in particular uses the
+ * wbc->fs_private field in its nfs_writepage, which is set in its
+ * nfs_writepages.  So if we don't call the lower nfs_writepages first, then
+ * NFS's nfs_writepage will dereference a NULL wbc->fs_private and cause an
+ * OOPS.  If, however, we implement a unionfs_writepages and then we do call
+ * the lower nfs_writepages, then we "lose control" over the pages we're
+ * trying to write to the lower file system: we won't be writing our own
+ * new/modified data from the upper pages to the lower pages, and any
+ * mmap-based changes are lost.
+ *
+ * This is a fundamental cache-coherency problem in Linux.  The kernel isn't
+ * able to support such stacking abstractions cleanly.  One possible clean
+ * way would be that a lower file system's ->writepage method have some sort
+ * of a callback to validate if any upper pages for the same file+offset
+ * exist and have newer content in them.
+ *
+ * This whole NULL ptr dereference is triggered at the lower file system
+ * (NFS) because the wbc->for_writepages is set to 1.  Therefore, to avoid
+ * this NULL pointer dereference, we set this flag to 0 and restore it upon
+ * exit.  This probably means that we're slightly less efficient in writing
+ * pages out, doing them one at a time, but at least we avoid the oops until
+ * such day as Linux can better support address_space_ops in a stackable
+ * fashion.
+ */
+int unionfs_writepage(struct page *page, struct writeback_control *wbc)
+{
+	int err = -EIO;
+	struct inode *inode;
+	struct inode *lower_inode;
+	struct page *lower_page;
+	char *kaddr, *lower_kaddr;
+	int saved_for_writepages = wbc->for_writepages;
+
+	inode = page->mapping->host;
+	lower_inode = unionfs_lower_inode(inode);
+
+	/* find lower page (returns a locked page) */
+	lower_page = grab_cache_page(lower_inode->i_mapping, page->index);
+	if (!lower_page)
+		goto out;
+
+	/* get page address, and encode it */
+	kaddr = kmap(page);
+	lower_kaddr = kmap(lower_page);
+
+	memcpy(lower_kaddr, kaddr, PAGE_CACHE_SIZE);
+
+	kunmap(page);
+	kunmap(lower_page);
+
+	BUG_ON(!lower_inode->i_mapping->a_ops->writepage);
+
+	/* workaround for some lower file systems: see big comment on top */
+	if (wbc->for_writepages && !wbc->fs_private)
+		wbc->for_writepages = 0;
+
+	/* call lower writepage (expects locked page) */
+	err = lower_inode->i_mapping->a_ops->writepage(lower_page, wbc);
+	wbc->for_writepages = saved_for_writepages; /* restore value */
+
+	/*
+	 * update mtime and ctime of lower level file system
+	 * unionfs' mtime and ctime are updated by generic_file_write
+	 */
+	lower_inode->i_mtime = lower_inode->i_ctime = CURRENT_TIME;
+
+	page_cache_release(lower_page);	/* b/c grab_cache_page increased refcnt */
+
+	if (err)
+		ClearPageUptodate(page);
+	else
+		SetPageUptodate(page);
+
+out:
+	unlock_page(page);
+	return err;
+}
+
+/*
+ * readpage is called from generic_page_read and the fault handler.
+ * If your file system uses generic_page_read for the read op, it
+ * must implement readpage.
+ *
+ * Readpage expects a locked page, and must unlock it.
+ */
+static int unionfs_do_readpage(struct file *file, struct page *page)
+{
+	int err = -EIO;
+	struct dentry *dentry;
+	struct file *lower_file = NULL;
+	struct inode *inode, *lower_inode;
+	char *page_data;
+	struct page *lower_page;
+	char *lower_page_data;
+
+	dentry = file->f_dentry;
+	if (UNIONFS_F(file) == NULL) {
+		err = -ENOENT;
+		goto out_err;
+	}
+
+	lower_file = unionfs_lower_file(file);
+	inode = dentry->d_inode;
+	lower_inode = unionfs_lower_inode(inode);
+
+	lower_page = NULL;
+
+	/* find lower page (returns a locked page) */
+	lower_page = read_cache_page(lower_inode->i_mapping,
+				     page->index,
+				     (filler_t *) lower_inode->i_mapping->
+				     a_ops->readpage, (void *)lower_file);
+
+	if (IS_ERR(lower_page)) {
+		err = PTR_ERR(lower_page);
+		lower_page = NULL;
+		goto out_release;
+	}
+
+	/*
+	 * wait for the page data to show up
+	 * (signaled by readpage as unlocking the page)
+	 */
+	wait_on_page_locked(lower_page);
+	if (!PageUptodate(lower_page)) {
+		/*
+		 * call readpage() again if we returned from wait_on_page
+		 * with a page that's not up-to-date; that can happen when a
+		 * partial page has a few buffers which are ok, but not the
+		 * whole page.
+		 */
+		lock_page(lower_page);
+		err = lower_inode->i_mapping->a_ops->readpage(lower_file,
+							      lower_page);
+		if (err) {
+			lower_page = NULL;
+			goto out_release;
+		}
+
+		wait_on_page_locked(lower_page);
+		if (!PageUptodate(lower_page)) {
+			err = -EIO;
+			goto out_release;
+		}
+	}
+
+	/* map pages, get their addresses */
+	page_data = (char *)kmap(page);
+	lower_page_data = (char *)kmap(lower_page);
+
+	memcpy(page_data, lower_page_data, PAGE_CACHE_SIZE);
+
+	err = 0;
+
+	kunmap(lower_page);
+	kunmap(page);
+
+out_release:
+	if (lower_page)
+		page_cache_release(lower_page);	/* undo read_cache_page */
+
+	if (err == 0)
+		SetPageUptodate(page);
+	else
+		ClearPageUptodate(page);
+
+out_err:
+	return err;
+}
+
+int unionfs_readpage(struct file *file, struct page *page)
+{
+	int err;
+
+	unionfs_read_lock(file->f_dentry->d_sb);
+
+	if ((err = unionfs_file_revalidate(file, 0)))
+		goto out;
+
+	err = unionfs_do_readpage(file, page);
+
+	if (!err)
+		touch_atime(unionfs_lower_mnt(file->f_path.dentry),
+			    unionfs_lower_dentry(file->f_path.dentry));
+
+	/*
+	 * we have to unlock our page, b/c we _might_ have gotten a locked
+	 * page.  but we no longer have to wakeup on our page here, b/c
+	 * UnlockPage does it
+	 */
+out:
+	unlock_page(page);
+	unionfs_read_unlock(file->f_dentry->d_sb);
+
+	return err;
+}
+
+int unionfs_prepare_write(struct file *file, struct page *page, unsigned from,
+			  unsigned to)
+{
+	int err;
+
+	unionfs_read_lock(file->f_dentry->d_sb);
+
+	err = unionfs_file_revalidate(file, 1);
+
+	unionfs_read_unlock(file->f_dentry->d_sb);
+
+	return err;
+}
+
+int unionfs_commit_write(struct file *file, struct page *page, unsigned from,
+			 unsigned to)
+{
+	int err = -ENOMEM;
+	struct inode *inode, *lower_inode;
+	struct file *lower_file = NULL;
+	loff_t pos;
+	unsigned bytes = to - from;
+	char *page_data = NULL;
+	mm_segment_t old_fs;
+
+	BUG_ON(file == NULL);
+
+	unionfs_read_lock(file->f_dentry->d_sb);
+
+	if ((err = unionfs_file_revalidate(file, 1)))
+		goto out;
+
+	inode = page->mapping->host;
+	lower_inode = unionfs_lower_inode(inode);
+
+	if (UNIONFS_F(file) != NULL)
+		lower_file = unionfs_lower_file(file);
+
+	/* FIXME: is this assertion right here? */
+	BUG_ON(lower_file == NULL);
+
+	page_data = (char *)kmap(page);
+	lower_file->f_pos = (page->index << PAGE_CACHE_SHIFT) + from;
+
+	/* SP: I use vfs_write instead of copying page data and the
+	 * prepare_write/commit_write combo because file system's like
+	 * GFS/OCFS2 don't like things touching those directly,
+	 * calling the underlying write op, while a little bit slower, will
+	 * call all the FS specific code as well
+	 */
+	old_fs = get_fs();
+	set_fs(KERNEL_DS);
+	err = vfs_write(lower_file, page_data + from, bytes,
+			&lower_file->f_pos);
+	set_fs(old_fs);
+
+	kunmap(page);
+
+	if (err < 0)
+		goto out;
+
+	inode->i_blocks = lower_inode->i_blocks;
+	/* we may have to update i_size */
+	pos = ((loff_t) page->index << PAGE_CACHE_SHIFT) + to;
+	if (pos > i_size_read(inode))
+		i_size_write(inode, pos);
+
+	/*
+	 * update mtime and ctime of lower level file system
+	 * unionfs' mtime and ctime are updated by generic_file_write
+	 */
+	lower_inode->i_mtime = lower_inode->i_ctime = CURRENT_TIME;
+
+	mark_inode_dirty_sync(inode);
+
+out:
+	if (err < 0)
+		ClearPageUptodate(page);
+
+	unionfs_read_unlock(file->f_dentry->d_sb);
+	return err;		/* assume all is ok */
+}
+
+void unionfs_sync_page(struct page *page)
+{
+	struct inode *inode;
+	struct inode *lower_inode;
+	struct page *lower_page;
+	struct address_space *mapping;
+
+	inode = page->mapping->host;
+	lower_inode = unionfs_lower_inode(inode);
+
+	/* find lower page (returns a locked page) */
+	lower_page = grab_cache_page(lower_inode->i_mapping, page->index);
+	if (!lower_page)
+		goto out;
+
+	/* do the actual sync */
+	mapping = lower_page->mapping;
+	/*
+	 * XXX: can we optimize ala RAIF and set the lower page to be
+	 * discarded after a successful sync_page?
+	 */
+	if (mapping && mapping->a_ops && mapping->a_ops->sync_page)
+		mapping->a_ops->sync_page(lower_page);
+
+	unlock_page(lower_page);	/* b/c grab_cache_page locked it */
+	page_cache_release(lower_page);	/* b/c grab_cache_page increased refcnt */
+
+out:
+	return;
+}
+
+struct address_space_operations unionfs_aops = {
+	.writepage	= unionfs_writepage,
+	.readpage	= unionfs_readpage,
+	.prepare_write	= unionfs_prepare_write,
+	.commit_write	= unionfs_commit_write,
+	.sync_page	= unionfs_sync_page,
+};
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index a7ff06c..196ff12 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -26,7 +26,7 @@ static struct kmem_cache *unionfs_inode_cachep;
 
 static void unionfs_read_inode(struct inode *inode)
 {
-	static struct address_space_operations unionfs_empty_aops;
+	extern struct address_space_operations unionfs_aops;
 	int size;
 	struct unionfs_inode_info *info = UNIONFS_I(inode);
 
@@ -58,8 +58,7 @@ static void unionfs_read_inode(struct inode *inode)
 	inode->i_op = &unionfs_main_iops;
 	inode->i_fop = &unionfs_main_fops;
 
-	/* I don't think ->a_ops is ever allowed to be NULL */
-	inode->i_mapping->a_ops = &unionfs_empty_aops;
+	inode->i_mapping->a_ops = &unionfs_aops;
 }
 
 /*
@@ -73,6 +72,9 @@ static void unionfs_delete_inode(struct inode *inode)
 {
 	inode->i_size = 0;	/* every f/s seems to do that */
 
+	if (inode->i_data.nrpages)
+		truncate_inode_pages(&inode->i_data, 0);
+
 	clear_inode(inode);
 }
 
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 01e29f3..480b8ee 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -38,6 +38,7 @@
 #include <linux/string.h>
 #include <linux/vmalloc.h>
 #include <linux/writeback.h>
+#include <linux/buffer_head.h>
 #include <linux/xattr.h>
 #include <linux/fs_stack.h>
 #include <linux/magic.h>
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 10/16] Unionfs: merge find_new_branch_index and branch_id_to_idx into one function
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (8 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 09/16] Unionfs: mmap implementation Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 11/16] Unionfs: Revalidate dentries passed to all inode/super operations Josef 'Jeff' Sipek
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, Josef 'Jeff' Sipek

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

Useful code cleanup and consolidation between the ODF code and non-ODF code.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |   35 +++++++++--------------------------
 fs/unionfs/fanout.h     |   24 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 0222393..731e3a9 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -90,31 +90,6 @@ out:
 }
 
 /*
- * 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
  */
@@ -130,8 +105,16 @@ static void cleanup_file(struct file *file)
 
 	for (bindex = bstart; bindex <= bend; bindex++) {
 		if (unionfs_lower_file_idx(file, bindex)) {
+			/*
+			 * Find new index of matching branch with an open
+			 * file, since branches could have been added or
+			 * deleted causing the one with open files to shift.
+			 */
 			int i;	/* holds (possibly) updated branch index */
-			i = find_new_branch_index(file, bindex, sb);
+			int old_bid;
+
+			old_bid = UNIONFS_F(file)->saved_branch_ids[bindex];
+			i = branch_id_to_idx(sb, old_bid);
 			if (i < 0)
 				printk(KERN_ERR "unionfs: no superblock for "
 				       "file %p\n", file);
diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 71052a3..0319835 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -55,6 +55,30 @@ static inline void new_branch_id(struct super_block *sb, int index)
 	set_branch_id(sb, index, ++UNIONFS_SB(sb)->high_branch_id);
 }
 
+/*
+ * Find new index of matching branch with an existing superblock a a known
+ * (possibly old) id.  This is needed because branches could have been
+ * added/deleted causing the branchs of any open files to shift.
+ *
+ * @sb: the new superblock which may have new/different branch IDs
+ * @id: the old/existing id we're looking for
+ * Returns index of newly found branch (0 or greater), -1 otherwise.
+ */
+static inline int branch_id_to_idx(struct super_block *sb, int id)
+{
+	int i;
+	for (i = 0; i < sbmax(sb); i++) {
+		if (branch_id(sb, i) == id)
+			return i;
+	}
+	/*
+	 * XXX: maybe we should BUG_ON if not found new branch index?
+	 * (really that should never happen).
+	 */
+	printk(KERN_WARNING "unionfs: cannot find branch with id %d\n", id);
+	return -1;
+}
+
 /* File to lower file. */
 static inline struct file *unionfs_lower_file(const struct file *f)
 {
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 11/16] Unionfs: Revalidate dentries passed to all inode/super operations
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (9 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 10/16] Unionfs: merge find_new_branch_index and branch_id_to_idx into one function Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 12/16] Unionfs: Cleanup new_dentry_private_data Josef 'Jeff' Sipek
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Erez Zadok, Josef 'Jeff' Sipek

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

Be sure to properly revalidate all dentry chains passed to all inode and
super_block operations.  Remove the older BUG_ON test is_valid_dentry().
This should help improve cache-coherency.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/inode.c  |   71 ++++++++++++++++++++++++++++++++++++++++-----------
 fs/unionfs/rename.c |   13 +++++++--
 fs/unionfs/super.c  |    9 ++++++-
 fs/unionfs/union.h  |   13 ---------
 fs/unionfs/unlink.c |   15 ++++++++---
 fs/unionfs/xattr.c  |   34 ++++++++++++++++++------
 6 files changed, 111 insertions(+), 44 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 9f1acc4..aaabfcf 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -47,7 +47,7 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
 	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 */
+		err = -ESTALE;	/* same as what real_lookup does */
 		goto out;
 	}
 	valid = __unionfs_d_revalidate_chain(dentry, nd);
@@ -234,6 +234,12 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 	struct path path_save;
 	struct dentry *ret;
 
+	/*
+	 * lookup is the only special function which takes a dentry, yet we
+	 * do NOT want to call __unionfs_d_revalidate_chain because by
+	 * definition, we don't have a valid dentry here yet.
+	 */
+
 	/* save the dentry & vfsmnt from namei */
 	if (nd) {
 		path_save.dentry = nd->dentry;
@@ -262,11 +268,18 @@ 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));
-
 	unionfs_double_lock_dentry(new_dentry, old_dentry);
 
+	if (!__unionfs_d_revalidate_chain(old_dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+	if (new_dentry->d_inode &&
+	    !__unionfs_d_revalidate_chain(new_dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	hidden_new_dentry = unionfs_lower_dentry(new_dentry);
 
 	/*
@@ -392,10 +405,14 @@ 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);
 
+	if (dentry->d_inode &&
+	    !__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	/* We start out in the leftmost branch. */
 	bstart = dbstart(dentry);
 
@@ -534,9 +551,14 @@ 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);
+
+	if (dentry->d_inode &&
+	    !__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	bstart = dbstart(dentry);
 
 	hidden_dentry = unionfs_lower_dentry(dentry);
@@ -667,9 +689,14 @@ 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);
+
+	if (dentry->d_inode &&
+	    !__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	bstart = dbstart(dentry);
 
 	hidden_dentry = unionfs_lower_dentry(dentry);
@@ -774,9 +801,13 @@ 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);
+
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
 	if (!hidden_dentry->d_inode->i_op ||
@@ -803,7 +834,13 @@ 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));
+	unionfs_lock_dentry(dentry);
+
+	if (dentry->d_inode &&
+	    !__unionfs_d_revalidate_chain(dentry, nd)) {
+		err = -ESTALE;
+		goto out;
+	}
 
 	/* This is freed by the put_link method assuming a successful call. */
 	buf = kmalloc(len, GFP_KERNEL);
@@ -969,9 +1006,13 @@ 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);
+
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	bstart = dbstart(dentry);
 	bend = dbend(dentry);
 	inode = dentry->d_inode;
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index edc5a5c..4ceb815 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -398,11 +398,18 @@ 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));
-
 	unionfs_double_lock_dentry(old_dentry, new_dentry);
 
+	if (!__unionfs_d_revalidate_chain(old_dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+	if (!d_deleted(new_dentry) && new_dentry->d_inode &&
+	    !__unionfs_d_revalidate_chain(new_dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	if (!S_ISDIR(old_dentry->d_inode->i_mode))
 		err = unionfs_partial_lookup(old_dentry);
 	else
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 196ff12..3f334f3 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -117,7 +117,12 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct super_block *sb;
 	struct dentry *lower_dentry;
 
-	BUG_ON(!is_valid_dentry(dentry));
+	unionfs_lock_dentry(dentry);
+
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
 
 	sb = dentry->d_sb;
 
@@ -136,6 +141,8 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	memset(&buf->f_fsid, 0, sizeof(__kernel_fsid_t));
 	memset(&buf->f_spare, 0, sizeof(buf->f_spare));
 
+out:
+	unionfs_unlock_dentry(dentry);
 	return err;
 }
 
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 480b8ee..28eb622 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -401,19 +401,6 @@ 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 27b4542..c785ff0 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -74,15 +74,19 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
 {
 	int err = 0;
 
-	BUG_ON(!is_valid_dentry(dentry));
-
 	unionfs_lock_dentry(dentry);
 
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	err = unionfs_unlink_whiteout(dir, dentry);
 	/* call d_drop so the system "forgets" about us */
 	if (!err)
 		d_drop(dentry);
 
+out:
 	unionfs_unlock_dentry(dentry);
 	return err;
 }
@@ -124,10 +128,13 @@ 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);
 
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	/* check if this unionfs directory is empty or not */
 	err = check_empty(dentry, &namelist);
 	if (err)
diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index 12d618b..1beb373 100644
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -57,14 +57,18 @@ 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);
 
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
 	err = vfs_getxattr(hidden_dentry, (char*) name, value, size);
 
+out:
 	unionfs_unlock_dentry(dentry);
 	return err;
 }
@@ -79,14 +83,19 @@ int unionfs_setxattr(struct dentry *dentry, const char *name,
 	struct dentry *hidden_dentry = NULL;
 	int err = -EOPNOTSUPP;
 
-	BUG_ON(!is_valid_dentry(dentry));
-
 	unionfs_lock_dentry(dentry);
+
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
 	err = vfs_setxattr(hidden_dentry, (char*) name, (void*) value,
 			   size, flags);
 
+out:
 	unionfs_unlock_dentry(dentry);
 	return err;
 }
@@ -100,13 +109,18 @@ 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);
+
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
 	err = vfs_removexattr(hidden_dentry, (char*) name);
 
+out:
 	unionfs_unlock_dentry(dentry);
 	return err;
 }
@@ -121,15 +135,19 @@ 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);
 
+	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+		err = -ESTALE;
+		goto out;
+	}
+
 	hidden_dentry = unionfs_lower_dentry(dentry);
 
 	encoded_list = list;
 	err = vfs_listxattr(hidden_dentry, encoded_list, size);
 
+out:
 	unionfs_unlock_dentry(dentry);
 	return err;
 }
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 12/16] Unionfs: Cleanup new_dentry_private_data
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (10 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 11/16] Unionfs: Revalidate dentries passed to all inode/super operations Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 13/16] Unionfs: Change free_dentry_private_info to take a struct dentry Josef 'Jeff' Sipek
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Josef 'Jeff' Sipek

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

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 758c813..246a67a 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -106,11 +106,22 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
 		BUG_ON(UNIONFS_D(dentry) != NULL);
 		locked_child = 1;
 	}
-	if (lookupmode != INTERPOSE_PARTIAL) {
-		if ((err = new_dentry_private_data(dentry)))
-			goto out;
-		allocated_new_info = 1;
+
+	switch(lookupmode) {
+		case INTERPOSE_PARTIAL:
+			break;
+		case INTERPOSE_LOOKUP:
+			if ((err = new_dentry_private_data(dentry)))
+				goto out;
+			allocated_new_info = 1;
+			break;
+		default:
+			if ((err = realloc_dentry_private_data(dentry)))
+				goto out;
+			allocated_new_info = 1;
+			break;
 	}
+
 	/* must initialize dentry operations */
 	dentry->d_op = &unionfs_dops;
 
@@ -448,58 +459,69 @@ void free_dentry_private_data(struct unionfs_dentry_info *udi)
 	kmem_cache_free(unionfs_dentry_cachep, udi);
 }
 
-/* allocate new dentry private data, free old one if necessary */
-int new_dentry_private_data(struct dentry *dentry)
+static inline int __realloc_dentry_private_data(struct dentry *dentry)
 {
-	int size;
 	struct unionfs_dentry_info *info = UNIONFS_D(dentry);
 	void *p;
-	int unlock_on_err = 0;
-
-	spin_lock(&dentry->d_lock);
-	if (!info) {
-		dentry->d_fsdata = kmem_cache_alloc(unionfs_dentry_cachep,
-						    GFP_ATOMIC);
-		
-		info = UNIONFS_D(dentry);
-		if (!info)
-			goto out;
+	int size;
 
-		mutex_init(&info->lock);
-		mutex_lock(&info->lock);
-		unlock_on_err = 1;
+	BUG_ON(!info);
 
-		info->lower_paths = NULL;
-	}
+	size = sizeof(struct path) * sbmax(dentry->d_sb);
+	p = krealloc(info->lower_paths, size, GFP_ATOMIC);
+	if (!p)
+		return -ENOMEM;
+
+	info->lower_paths = p;
 
 	info->bstart = -1;
 	info->bend = -1;
 	info->bopaque = -1;
 	info->bcount = sbmax(dentry->d_sb);
 	atomic_set(&info->generation,
-		   atomic_read(&UNIONFS_SB(dentry->d_sb)->generation));
-
-	size = sizeof(struct path) * sbmax(dentry->d_sb);
-
-	p = krealloc(info->lower_paths, size, GFP_ATOMIC);
-	if (!p)
-		goto out_free;
+			atomic_read(&UNIONFS_SB(dentry->d_sb)->generation));
 
-	info->lower_paths = p;
 	memset(info->lower_paths, 0, size);
 
-	spin_unlock(&dentry->d_lock);
 	return 0;
+}
 
-out_free:
-	kfree(info->lower_paths);
-	if (unlock_on_err)
-		mutex_unlock(&info->lock);
+/* UNIONFS_D(dentry)->lock must be locked */
+int realloc_dentry_private_data(struct dentry *dentry)
+{
+	if (!__realloc_dentry_private_data(dentry))
+		return 0;
 
-out:
+	kfree(UNIONFS_D(dentry)->lower_paths);
+	free_dentry_private_data(UNIONFS_D(dentry));
+	dentry->d_fsdata = NULL;
+	return -ENOMEM;
+}
+
+/* allocate new dentry private data */
+int new_dentry_private_data(struct dentry *dentry)
+{
+	struct unionfs_dentry_info *info = UNIONFS_D(dentry);
+
+	BUG_ON(info);
+
+	info = kmem_cache_alloc(unionfs_dentry_cachep, GFP_ATOMIC);
+	if (!info)
+		return -ENOMEM;
+
+	mutex_init(&info->lock);
+	mutex_lock(&info->lock);
+
+	info->lower_paths = NULL;
+
+	dentry->d_fsdata = info;
+
+	if (!__realloc_dentry_private_data(dentry))
+		return 0;
+
+	mutex_unlock(&info->lock);
 	free_dentry_private_data(info);
 	dentry->d_fsdata = NULL;
-	spin_unlock(&dentry->d_lock);
 	return -ENOMEM;
 }
 
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 28eb622..7e0c318 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -233,6 +233,7 @@ static inline void unionfs_double_lock_dentry(struct dentry *d1,
 	unionfs_lock_dentry(d2);
 }
 
+extern int realloc_dentry_private_data(struct dentry *dentry);
 extern int new_dentry_private_data(struct dentry *dentry);
 extern void free_dentry_private_data(struct unionfs_dentry_info *udi);
 extern void update_bstart(struct dentry *dentry);
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 13/16] Unionfs: Change free_dentry_private_info to take a struct dentry
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (11 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 12/16] Unionfs: Cleanup new_dentry_private_data Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 14/16] Unionfs: Add BUG_ONs to unionfs_lower_* Josef 'Jeff' Sipek
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Josef 'Jeff' Sipek

This makes it more symmetric with new_dentry_private_info.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/dentry.c |    3 +--
 fs/unionfs/lookup.c |   13 ++++++-------
 fs/unionfs/main.c   |    2 +-
 fs/unionfs/union.h  |    2 +-
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index db0ef43..9bd521b 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -334,8 +334,7 @@ static void unionfs_d_release(struct dentry *dentry)
 
 out_free:
 	/* No need to unlock it, because it is disappeared. */
-	free_dentry_private_data(UNIONFS_D(dentry));
-	dentry->d_fsdata = NULL;	/* just to be safe */
+	free_dentry_private_data(dentry);
 
 out:
 	return;
diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 246a67a..8a09540 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -452,11 +452,12 @@ void unionfs_destroy_dentry_cache(void)
 		kmem_cache_destroy(unionfs_dentry_cachep);
 }
 
-void free_dentry_private_data(struct unionfs_dentry_info *udi)
+void free_dentry_private_data(struct dentry *dentry)
 {
-	if (!udi)
+	if (!dentry || !dentry->d_fsdata)
 		return;
-	kmem_cache_free(unionfs_dentry_cachep, udi);
+	kmem_cache_free(unionfs_dentry_cachep, dentry->d_fsdata);
+	dentry->d_fsdata = NULL;
 }
 
 static inline int __realloc_dentry_private_data(struct dentry *dentry)
@@ -493,8 +494,7 @@ int realloc_dentry_private_data(struct dentry *dentry)
 		return 0;
 
 	kfree(UNIONFS_D(dentry)->lower_paths);
-	free_dentry_private_data(UNIONFS_D(dentry));
-	dentry->d_fsdata = NULL;
+	free_dentry_private_data(dentry);
 	return -ENOMEM;
 }
 
@@ -520,8 +520,7 @@ int new_dentry_private_data(struct dentry *dentry)
 		return 0;
 
 	mutex_unlock(&info->lock);
-	free_dentry_private_data(info);
-	dentry->d_fsdata = NULL;
+	free_dentry_private_data(dentry);
 	return -ENOMEM;
 }
 
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 2bcc84c..b9fe431 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -632,7 +632,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
 out_freedpd:
 	if (UNIONFS_D(sb->s_root)) {
 		kfree(UNIONFS_D(sb->s_root)->lower_paths);
-		free_dentry_private_data(UNIONFS_D(sb->s_root));
+		free_dentry_private_data(sb->s_root);
 	}
 	dput(sb->s_root);
 
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 7e0c318..6eb151e 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -235,7 +235,7 @@ static inline void unionfs_double_lock_dentry(struct dentry *d1,
 
 extern int realloc_dentry_private_data(struct dentry *dentry);
 extern int new_dentry_private_data(struct dentry *dentry);
-extern void free_dentry_private_data(struct unionfs_dentry_info *udi);
+extern void free_dentry_private_data(struct dentry *dentry);
 extern void update_bstart(struct dentry *dentry);
 
 /*
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 14/16] Unionfs: Add BUG_ONs to unionfs_lower_*
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (12 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 13/16] Unionfs: Change free_dentry_private_info to take a struct dentry Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 15/16] Unionfs: Change the semantics of sb info's rwsem Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 16/16] Unionfs: Remove superfluous check for NULL pointer Josef 'Jeff' Sipek
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Josef 'Jeff' Sipek

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

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 0319835..d4933ce 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -42,16 +42,19 @@ static inline struct unionfs_inode_info *UNIONFS_I(const struct inode *inode)
 /* macros to manipulate branch IDs in stored in our superblock */
 static inline int branch_id(struct super_block *sb, int index)
 {
+	BUG_ON(!sb || index < 0);
 	return UNIONFS_SB(sb)->data[index].branch_id;
 }
 
 static inline void set_branch_id(struct super_block *sb, int index, int val)
 {
+	BUG_ON(!sb || index < 0);
 	UNIONFS_SB(sb)->data[index].branch_id = val;
 }
 
 static inline void new_branch_id(struct super_block *sb, int index)
 {
+	BUG_ON(!sb || index < 0);
 	set_branch_id(sb, index, ++UNIONFS_SB(sb)->high_branch_id);
 }
 
@@ -82,18 +85,21 @@ static inline int branch_id_to_idx(struct super_block *sb, int id)
 /* File to lower file. */
 static inline struct file *unionfs_lower_file(const struct file *f)
 {
+	BUG_ON(!f);
 	return UNIONFS_F(f)->lower_files[fbstart(f)];
 }
 
 static inline struct file *unionfs_lower_file_idx(const struct file *f,
 						  int index)
 {
+	BUG_ON(!f || index < 0);
 	return UNIONFS_F(f)->lower_files[index];
 }
 
 static inline void unionfs_set_lower_file_idx(struct file *f, int index,
 					      struct file *val)
 {
+	BUG_ON(!f || index < 0);
 	UNIONFS_F(f)->lower_files[index] = val;
 	/* save branch ID (may be redundant?) */
 	UNIONFS_F(f)->saved_branch_ids[index] =
@@ -102,29 +108,34 @@ static inline void unionfs_set_lower_file_idx(struct file *f, int index,
 
 static inline void unionfs_set_lower_file(struct file *f, struct file *val)
 {
+	BUG_ON(!f);
 	unionfs_set_lower_file_idx((f), fbstart(f), (val));
 }
 
 /* Inode to lower inode. */
 static inline struct inode *unionfs_lower_inode(const struct inode *i)
 {
+	BUG_ON(!i);
 	return UNIONFS_I(i)->lower_inodes[ibstart(i)];
 }
 
 static inline struct inode *unionfs_lower_inode_idx(const struct inode *i,
 						    int index)
 {
+	BUG_ON(!i || index < 0);
 	return UNIONFS_I(i)->lower_inodes[index];
 }
 
 static inline void unionfs_set_lower_inode_idx(struct inode *i, int index,
 					       struct inode *val)
 {
+	BUG_ON(!i || index < 0);
 	UNIONFS_I(i)->lower_inodes[index] = val;
 }
 
 static inline void unionfs_set_lower_inode(struct inode *i, struct inode *val)
 {
+	BUG_ON(!i);
 	UNIONFS_I(i)->lower_inodes[ibstart(i)] = val;
 }
 
@@ -132,6 +143,7 @@ static inline void unionfs_set_lower_inode(struct inode *i, struct inode *val)
 static inline struct super_block *unionfs_lower_super(
 					const struct super_block *sb)
 {
+	BUG_ON(!sb);
 	return UNIONFS_SB(sb)->data[sbstart(sb)].sb;
 }
 
@@ -139,6 +151,7 @@ static inline struct super_block *unionfs_lower_super_idx(
 					const struct super_block *sb,
 					int index)
 {
+	BUG_ON(!sb || index < 0);
 	return UNIONFS_SB(sb)->data[index].sb;
 }
 
@@ -146,75 +159,89 @@ static inline void unionfs_set_lower_super_idx(struct super_block *sb,
 					       int index,
 					       struct super_block *val)
 {
+	BUG_ON(!sb || index < 0);
 	UNIONFS_SB(sb)->data[index].sb = val;
 }
 
 static inline void unionfs_set_lower_super(struct super_block *sb,
 					   struct super_block *val)
 {
+	BUG_ON(!sb);
 	UNIONFS_SB(sb)->data[sbstart(sb)].sb = val;
 }
 
 /* Branch count macros. */
 static inline int branch_count(const struct super_block *sb, int index)
 {
+	BUG_ON(!sb || index < 0);
 	return atomic_read(&UNIONFS_SB(sb)->data[index].open_files);
 }
 
 static inline void set_branch_count(struct super_block *sb, int index, int val)
 {
+	BUG_ON(!sb || index < 0);
 	atomic_set(&UNIONFS_SB(sb)->data[index].open_files, val);
 }
 
 static inline void branchget(struct super_block *sb, int index)
 {
+	BUG_ON(!sb || index < 0);
 	atomic_inc(&UNIONFS_SB(sb)->data[index].open_files);
 }
 
 static inline void branchput(struct super_block *sb, int index)
 {
+	BUG_ON(!sb || index < 0);
 	atomic_dec(&UNIONFS_SB(sb)->data[index].open_files);
 }
 
 /* Dentry macros */
 static inline struct unionfs_dentry_info *UNIONFS_D(const struct dentry *dent)
 {
+	BUG_ON(!dent);
 	return dent->d_fsdata;
 }
 
 static inline int dbstart(const struct dentry *dent)
 {
+	BUG_ON(!dent);
 	return UNIONFS_D(dent)->bstart;
 }
 
 static inline void set_dbstart(struct dentry *dent, int val)
 {
+	BUG_ON(!dent);
 	UNIONFS_D(dent)->bstart = val;
 }
 
 static inline int dbend(const struct dentry *dent)
 {
+	BUG_ON(!dent);
 	return UNIONFS_D(dent)->bend;
 }
 
 static inline void set_dbend(struct dentry *dent, int val)
 {
+	BUG_ON(!dent);
 	UNIONFS_D(dent)->bend = val;
 }
 
 static inline int dbopaque(const struct dentry *dent)
 {
+	BUG_ON(!dent);
 	return UNIONFS_D(dent)->bopaque;
 }
 
 static inline void set_dbopaque(struct dentry *dent, int val)
 {
+	BUG_ON(!dent);
 	UNIONFS_D(dent)->bopaque = val;
 }
 
 static inline void unionfs_set_lower_dentry_idx(struct dentry *dent, int index,
 						struct dentry *val)
 {
+	BUG_ON(!dent || index < 0);
 	UNIONFS_D(dent)->lower_paths[index].dentry = val;
 }
 
@@ -222,17 +249,20 @@ static inline struct dentry *unionfs_lower_dentry_idx(
 				const struct dentry *dent,
 				int index)
 {
+	BUG_ON(!dent || index < 0);
 	return UNIONFS_D(dent)->lower_paths[index].dentry;
 }
 
 static inline struct dentry *unionfs_lower_dentry(const struct dentry *dent)
 {
+	BUG_ON(!dent);
 	return unionfs_lower_dentry_idx(dent, dbstart(dent));
 }
 
 static inline void unionfs_set_lower_mnt_idx(struct dentry *dent, int index,
 					     struct vfsmount *mnt)
 {
+	BUG_ON(!dent || index < 0);
 	UNIONFS_D(dent)->lower_paths[index].mnt = mnt;
 }
 
@@ -240,27 +270,32 @@ static inline struct vfsmount *unionfs_lower_mnt_idx(
 					const struct dentry *dent,
 					int index)
 {
+	BUG_ON(!dent || index < 0);
 	return UNIONFS_D(dent)->lower_paths[index].mnt;
 }
 
 static inline struct vfsmount *unionfs_lower_mnt(const struct dentry *dent)
 {
+	BUG_ON(!dent);
 	return unionfs_lower_mnt_idx(dent, dbstart(dent));
 }
 
 /* Macros for locking a dentry. */
 static inline void unionfs_lock_dentry(struct dentry *d)
 {
+	BUG_ON(!d);
 	mutex_lock(&UNIONFS_D(d)->lock);
 }
 
 static inline void unionfs_unlock_dentry(struct dentry *d)
 {
+	BUG_ON(!d);
 	mutex_unlock(&UNIONFS_D(d)->lock);
 }
 
 static inline void verify_locked(struct dentry *d)
 {
+	BUG_ON(!d);
 	BUG_ON(!mutex_is_locked(&UNIONFS_D(d)->lock));
 }
 
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 15/16] Unionfs: Change the semantics of sb info's rwsem
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (13 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 14/16] Unionfs: Add BUG_ONs to unionfs_lower_* Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  2007-06-17 19:09 ` [PATCH 16/16] Unionfs: Remove superfluous check for NULL pointer Josef 'Jeff' Sipek
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Josef 'Jeff' Sipek

This rw semaphore is used to make sure that a branch management operation...

1) will not begin before all currently in-flight operations complete

2) any new operations do not execute until the currently running branch
management operation completes

TODO: rename the functions unionfs_{read,write}_{,un}lock() to something
more descriptive.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |   33 ++++++++---------------
 fs/unionfs/copyup.c     |   10 -------
 fs/unionfs/dentry.c     |    7 +++++
 fs/unionfs/dirfops.c    |   10 ++++---
 fs/unionfs/dirhelper.c  |   10 -------
 fs/unionfs/file.c       |   16 +++++-----
 fs/unionfs/inode.c      |   66 ++++++++++++++++++++++++++++++----------------
 fs/unionfs/main.c       |   23 ++++++++--------
 fs/unionfs/rename.c     |    2 +
 fs/unionfs/super.c      |   34 +++++++++++++++++++-----
 fs/unionfs/union.h      |   15 +++++++---
 fs/unionfs/unlink.c     |    4 +++
 fs/unionfs/xattr.c      |    8 +++++
 13 files changed, 138 insertions(+), 100 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 731e3a9..a6917fe 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -119,10 +119,8 @@ static void cleanup_file(struct file *file)
 				printk(KERN_ERR "unionfs: no superblock for "
 				       "file %p\n", file);
 			else {
-				unionfs_read_lock(sb);
 				/* decrement count of open files */
 				branchput(sb, i);
-				unionfs_read_unlock(sb);
 				/*
 				 * fput will perform an mntput for us on the
 				 * correct branch.  Although we're using the
@@ -164,9 +162,7 @@ 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,
@@ -213,9 +209,7 @@ 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);
@@ -259,9 +253,7 @@ 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);
 			}
@@ -400,9 +392,7 @@ 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;
@@ -463,9 +453,7 @@ 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;
 }
@@ -479,6 +467,7 @@ int unionfs_open(struct inode *inode, struct file *file)
 	int size;
 
 	unionfs_read_lock(inode->i_sb);
+
 	file->private_data =
 		kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
 	if (!UNIONFS_F(file)) {
@@ -529,9 +518,7 @@ 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);
 		}
@@ -550,7 +537,11 @@ out_nofree:
 	return err;
 }
 
-/* release all lower object references & free the file info structure */
+/*
+ * release all lower object references & free the file info structure
+ *
+ * No need to grab sb info's rwsem.
+ */
 int unionfs_file_release(struct inode *inode, struct file *file)
 {
 	struct file *hidden_file = NULL;
@@ -583,9 +574,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 
 		if (hidden_file) {
 			fput(hidden_file);
-			unionfs_read_lock(inode->i_sb);
 			branchput(inode->i_sb, bindex);
-			unionfs_read_unlock(inode->i_sb);
 		}
 	}
 	kfree(fileinfo->lower_files);
@@ -607,7 +596,6 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 		fileinfo->rdstate = NULL;
 	}
 	kfree(fileinfo);
-	unionfs_read_unlock(inode->i_sb);
 	return 0;
 }
 
@@ -684,7 +672,8 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	long err;
 
-	unionfs_read_lock(file->f_dentry->d_sb);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
+
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
 
@@ -709,7 +698,7 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	}
 
 out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
 
@@ -720,7 +709,7 @@ 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);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
 
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
@@ -754,6 +743,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);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index dff4f1c..8f13670 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -210,9 +210,7 @@ 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);
@@ -229,9 +227,7 @@ 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);
@@ -307,17 +303,13 @@ 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;
 }
@@ -461,9 +453,7 @@ out_unlink:
 		/* need to close the file */
 
 		fput(*copyup_file);
-		unionfs_read_lock(sb);
 		branchput(sb, new_bindex);
-		unionfs_read_unlock(sb);
 	}
 
 	/*
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 9bd521b..306e171 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -290,10 +290,14 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
 	int err;
 
+	unionfs_read_lock(dentry->d_sb);
+
 	unionfs_lock_dentry(dentry);
 	err = __unionfs_d_revalidate_chain(dentry, nd);
 	unionfs_unlock_dentry(dentry);
 
+	unionfs_read_unlock(dentry->d_sb);
+
 	return err;
 }
 
@@ -305,6 +309,8 @@ static void unionfs_d_release(struct dentry *dentry)
 {
 	int bindex, bstart, bend;
 
+	unionfs_read_lock(dentry->d_sb);
+
 	/* this could be a negative dentry, so check first */
 	if (!UNIONFS_D(dentry)) {
 		printk(KERN_DEBUG "unionfs: dentry without private data: %.*s",
@@ -337,6 +343,7 @@ out_free:
 	free_dentry_private_data(dentry);
 
 out:
+	unionfs_read_unlock(dentry->d_sb);
 	return;
 }
 
diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c
index 7306b3f..95b0946 100644
--- a/fs/unionfs/dirfops.c
+++ b/fs/unionfs/dirfops.c
@@ -97,7 +97,8 @@ 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);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
+
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
 
@@ -179,7 +180,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);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
 
@@ -198,7 +199,8 @@ 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);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
+
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
 
@@ -255,7 +257,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
 	}
 
 out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/dirhelper.c b/fs/unionfs/dirhelper.c
index 975d6fe..82e7896 100644
--- a/fs/unionfs/dirhelper.c
+++ b/fs/unionfs/dirhelper.c
@@ -99,7 +99,6 @@ int delete_whiteouts(struct dentry *dentry, int bindex,
 	struct sioq_args args;
 
 	sb = dentry->d_sb;
-	unionfs_read_lock(sb);
 
 	BUG_ON(!S_ISDIR(dentry->d_inode->i_mode));
 	BUG_ON(bindex < dbstart(dentry));
@@ -126,7 +125,6 @@ int delete_whiteouts(struct dentry *dentry, int bindex,
 	mutex_unlock(&hidden_dir->i_mutex);
 
 out:
-	unionfs_read_unlock(sb);
 	return err;
 }
 
@@ -195,7 +193,6 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
 
 	sb = dentry->d_sb;
 
-	unionfs_read_lock(sb);
 
 	BUG_ON(!S_ISDIR(dentry->d_inode->i_mode));
 
@@ -233,9 +230,7 @@ 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),
@@ -243,9 +238,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
 		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;
 		}
 
@@ -260,9 +253,7 @@ 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;
@@ -277,7 +268,6 @@ out:
 		kfree(buf);
 	}
 
-	unionfs_read_unlock(sb);
 
 	return err;
 }
diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index afffe59..aea378d 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -27,7 +27,7 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
 {
 	int err;
 
-	unionfs_read_lock(file->f_dentry->d_sb);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
 
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
@@ -39,7 +39,7 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
 			    unionfs_lower_dentry(file->f_path.dentry));
 
 out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
 
@@ -49,7 +49,7 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
 	int err = 0;
 	struct file *file = iocb->ki_filp;
 
-	unionfs_read_lock(file->f_dentry->d_sb);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
 
 	if ((err = unionfs_file_revalidate(file, 0)))
 		goto out;
@@ -64,7 +64,7 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
 			    unionfs_lower_dentry(file->f_path.dentry));
 
 out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
 static ssize_t unionfs_write(struct file * file, const char __user * buf,
@@ -72,7 +72,7 @@ static ssize_t unionfs_write(struct file * file, const char __user * buf,
 {
 	int err = 0;
 
-	unionfs_read_lock(file->f_dentry->d_sb);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
 
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
@@ -80,7 +80,7 @@ static ssize_t unionfs_write(struct file * file, const char __user * buf,
 	err = do_sync_write(file, buf, count, ppos);
 
 out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
 
@@ -96,7 +96,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 	int willwrite;
 	struct file *lower_file;
 
-	unionfs_read_lock(file->f_dentry->d_sb);
+	unionfs_read_lock(file->f_path.dentry->d_sb);
 
 	if ((err = unionfs_file_revalidate(file, 1)))
 		goto out;
@@ -128,7 +128,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 	}
 
 out:
-	unionfs_read_unlock(file->f_dentry->d_sb);
+	unionfs_read_unlock(file->f_path.dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index aaabfcf..cdd1a7c 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -30,16 +30,6 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
 	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);
 
@@ -234,11 +224,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 	struct path path_save;
 	struct dentry *ret;
 
-	/*
-	 * lookup is the only special function which takes a dentry, yet we
-	 * do NOT want to call __unionfs_d_revalidate_chain because by
-	 * definition, we don't have a valid dentry here yet.
-	 */
+	unionfs_read_lock(dentry->d_sb);
 
 	/* save the dentry & vfsmnt from namei */
 	if (nd) {
@@ -255,6 +241,8 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 		nd->mnt = path_save.mnt;
 	}
 
+	unionfs_read_unlock(dentry->d_sb);
+
 	return ret;
 }
 
@@ -268,6 +256,7 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
 	struct dentry *whiteout_dentry;
 	char *name = NULL;
 
+	unionfs_read_lock(old_dentry->d_sb);
 	unionfs_double_lock_dentry(new_dentry, old_dentry);
 
 	if (!__unionfs_d_revalidate_chain(old_dentry, NULL)) {
@@ -391,6 +380,8 @@ out:
 	unionfs_unlock_dentry(new_dentry);
 	unionfs_unlock_dentry(old_dentry);
 
+	unionfs_read_unlock(old_dentry->d_sb);
+
 	return err;
 }
 
@@ -405,6 +396,7 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
 	int bindex = 0, bstart;
 	char *name = NULL;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (dentry->d_inode &&
@@ -538,6 +530,7 @@ out:
 
 	kfree(name);
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -551,6 +544,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
 	int whiteout_unlinked = 0;
 	struct sioq_args args;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (dentry->d_inode &&
@@ -676,6 +670,7 @@ out:
 	kfree(name);
 
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -689,6 +684,7 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
 	char *name = NULL;
 	int whiteout_unlinked = 0;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (dentry->d_inode &&
@@ -792,6 +788,7 @@ out:
 	kfree(name);
 
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -801,6 +798,7 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf,
 	int err;
 	struct dentry *hidden_dentry;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -824,9 +822,23 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf,
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
+/*
+ * 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));
+}
+
 /* We don't lock the dentry here, because readlink does the heavy lifting. */
 static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
@@ -834,13 +846,17 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 	int len = PAGE_SIZE, err;
 	mm_segment_t old_fs;
 
-	unionfs_lock_dentry(dentry);
+	/*
+	 * FIXME: Really nasty...we can get called from two distinct places:
+	 * 1) read_link - locks the dentry
+	 * 2) VFS lookup code - does NOT lock the dentry
+	 *
+	 * The proper thing would be to call dentry revalidate. It however
+	 * expects a locked dentry, and we can't cleanly guarantee that.
+	 */
+	BUG_ON(!is_valid_dentry(dentry));
 
-	if (dentry->d_inode &&
-	    !__unionfs_d_revalidate_chain(dentry, nd)) {
-		err = -ESTALE;
-		goto out;
-	}
+	unionfs_read_lock(dentry->d_sb);
 
 	/* This is freed by the put_link method assuming a successful call. */
 	buf = kmalloc(len, GFP_KERNEL);
@@ -864,13 +880,17 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 	err = 0;
 
 out:
+	unionfs_read_unlock(dentry->d_sb);
 	return ERR_PTR(err);
 }
 
+/* FIXME: We may not have to lock here */
 static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
 			     void *cookie)
 {
+	unionfs_read_lock(dentry->d_sb);
 	kfree(nd_get_link(nd));
+	unionfs_read_unlock(dentry->d_sb);
 }
 
 /*
@@ -912,9 +932,7 @@ static int inode_permission(struct inode *inode, int mask,
 		    (!strcmp("nfs", (inode)->i_sb->s_type->name)) &&
 		    (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);
@@ -1006,6 +1024,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
 	int i;
 	int copyup = 0;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -1075,6 +1094,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index b9fe431..c6bf729 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -242,7 +242,13 @@ int parse_branch_mode(const char *name)
 	return perms;
 }
 
-/* parse the dirs= mount argument */
+/* 
+ * parse the dirs= mount argument
+ *
+ * We don't need to lock the superblock private data's rwsem, as we get
+ * called only by unionfs_read_super - it is still a long time before anyone
+ * can even get a reference to us.
+ */
 static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
 			     *hidden_root_info, char *options)
 {
@@ -325,11 +331,9 @@ 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;
@@ -530,6 +534,10 @@ static struct dentry *unionfs_d_alloc_root(struct super_block *sb)
 	return ret;
 }
 
+/*
+ * There is no need to lock the unionfs_super_info's rwsem as there is no
+ * way anyone can have a reference to the superblock at this point in time.
+ */
 static int unionfs_read_super(struct super_block *sb, void *raw_data,
 			      int silent)
 {
@@ -577,19 +585,12 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
 	BUG_ON(bstart != 0);
 	sbend(sb) = bend = hidden_root_info->bend;
 	for (bindex = bstart; bindex <= bend; bindex++) {
-		struct dentry *d;
-
-		d = hidden_root_info->lower_paths[bindex].dentry;
-
-		unionfs_write_lock(sb);
+		struct dentry *d = hidden_root_info->lower_paths[bindex].dentry;
 		unionfs_set_lower_super_idx(sb, bindex, d->d_sb);
-		unionfs_write_unlock(sb);
 	}
 
 	/* 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/rename.c b/fs/unionfs/rename.c
index 4ceb815..861c8db 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -398,6 +398,7 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	int err = 0;
 	struct dentry *wh_dentry;
 
+	unionfs_read_lock(old_dentry->d_sb);
 	unionfs_double_lock_dentry(old_dentry, new_dentry);
 
 	if (!__unionfs_d_revalidate_chain(old_dentry, NULL)) {
@@ -471,5 +472,6 @@ out:
 
 	unionfs_unlock_dentry(new_dentry);
 	unionfs_unlock_dentry(old_dentry);
+	unionfs_read_unlock(old_dentry->d_sb);
 	return err;
 }
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 3f334f3..9f248b9 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -30,6 +30,8 @@ static void unionfs_read_inode(struct inode *inode)
 	int size;
 	struct unionfs_inode_info *info = UNIONFS_I(inode);
 
+	unionfs_read_lock(inode->i_sb);
+
 	if (!info) {
 		printk(KERN_ERR "unionfs: no kernel memory when allocating "
 		       "inode private data!\n");
@@ -59,6 +61,8 @@ static void unionfs_read_inode(struct inode *inode)
 	inode->i_fop = &unionfs_main_fops;
 
 	inode->i_mapping->a_ops = &unionfs_aops;
+
+	unionfs_read_unlock(inode->i_sb);
 }
 
 /*
@@ -67,6 +71,8 @@ static void unionfs_read_inode(struct inode *inode)
  * else that's needed, and the other is fine.  This way we truncate the inode
  * size (and its pages) and then clear our own inode, which will do an iput
  * on our and the lower inode.
+ *
+ * No need to lock sb info's rwsem.
  */
 static void unionfs_delete_inode(struct inode *inode)
 {
@@ -78,7 +84,11 @@ static void unionfs_delete_inode(struct inode *inode)
 	clear_inode(inode);
 }
 
-/* final actions when unmounting a file system */
+/*
+ * final actions when unmounting a file system
+ *
+ * No need to lock rwsem.
+ */
 static void unionfs_put_super(struct super_block *sb)
 {
 	int bindex, bstart, bend;
@@ -117,6 +127,9 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct super_block *sb;
 	struct dentry *lower_dentry;
 
+	sb = dentry->d_sb;
+
+	unionfs_read_lock(sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -124,8 +137,6 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 		goto out;
 	}
 
-	sb = dentry->d_sb;
-
 	lower_dentry = unionfs_lower_dentry(sb->s_root);
 	err = vfs_statfs(lower_dentry, buf);
 
@@ -143,6 +154,7 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(sb);
 	return err;
 }
 
@@ -787,6 +799,8 @@ out_error:
  * and the inode is not hashed anywhere.  Used to clear anything
  * that needs to be, before the inode is completely destroyed and put
  * on the inode free list.
+ *
+ * No need to lock sb info's rwsem.
  */
 static void unionfs_clear_inode(struct inode *inode)
 {
@@ -871,6 +885,8 @@ void unionfs_destroy_inode_cache(void)
 /*
  * Called when we have a dirty inode, right here we only throw out
  * parts of our readdir list that are too old.
+ *
+ * No need to grab sb info's rwsem.
  */
 static int unionfs_write_inode(struct inode *inode, int sync)
 {
@@ -911,18 +927,20 @@ static void unionfs_umount_begin(struct vfsmount *mnt, int flags)
 
 	sb = mnt->mnt_sb;
 
+	unionfs_read_lock(sb);
+
 	bstart = sbstart(sb);
 	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)
 			hidden_sb->s_op->umount_begin(hidden_mnt, flags);
 	}
+
+	unionfs_read_unlock(sb);
 }
 
 static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
@@ -934,6 +952,8 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
 	int bindex, bstart, bend;
 	int perms;
 
+	unionfs_read_lock(sb);
+
 	unionfs_lock_dentry(sb->s_root);
 
 	tmp_page = (char*) __get_free_page(GFP_KERNEL);
@@ -955,9 +975,7 @@ 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");
@@ -970,6 +988,8 @@ out:
 
 	unionfs_unlock_dentry(sb->s_root);
 
+	unionfs_read_unlock(sb);
+
 	return ret;
 }
 
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 6eb151e..3fb4c14 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -134,7 +134,16 @@ struct unionfs_sb_info {
 	int bend;
 
 	atomic_t generation;
-	struct rw_semaphore rwsem; /* protects access to data+id fields */
+
+	/*
+	 * This rwsem is used to make sure that a branch management
+	 * operation...
+	 *   1) will not begin before all currently in-flight operations
+	 *      complete
+	 *   2) any new operations do not execute until the currently
+	 *      running branch management operation completes
+	 */
+	struct rw_semaphore rwsem;
 	int high_branch_id;	/* last unique branch ID given */
 	struct unionfs_data *data;
 };
@@ -371,9 +380,7 @@ static inline int is_robranch_super(const struct super_block *sb, int index)
 {
 	int ret;
 
-	unionfs_read_lock(sb);
   	ret = (!(branchperms(sb, index) & MAY_WRITE)) ? -EROFS : 0;
-	unionfs_read_unlock(sb);
 	return ret;
 }
 
@@ -384,11 +391,9 @@ 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;
 }
 
diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index c785ff0..a6b8bab 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -74,6 +74,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
 {
 	int err = 0;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -88,6 +89,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -128,6 +130,7 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)
 	int err = 0;
 	struct unionfs_dir_state *namelist = NULL;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -168,5 +171,6 @@ out:
 		free_rdstate(namelist);
 
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index 1beb373..5bb8054 100644
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -57,6 +57,7 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value,
 	struct dentry *hidden_dentry = NULL;
 	int err = -EOPNOTSUPP;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -70,6 +71,7 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value,
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -83,6 +85,7 @@ int unionfs_setxattr(struct dentry *dentry, const char *name,
 	struct dentry *hidden_dentry = NULL;
 	int err = -EOPNOTSUPP;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -97,6 +100,7 @@ int unionfs_setxattr(struct dentry *dentry, const char *name,
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -109,6 +113,7 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
 	struct dentry *hidden_dentry = NULL;
 	int err = -EOPNOTSUPP;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -122,6 +127,7 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -135,6 +141,7 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size)
 	int err = -EOPNOTSUPP;
 	char *encoded_list = NULL;
 
+	unionfs_read_lock(dentry->d_sb);
 	unionfs_lock_dentry(dentry);
 
 	if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
@@ -149,5 +156,6 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size)
 
 out:
 	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 16/16] Unionfs: Remove superfluous check for NULL pointer
  2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
                   ` (14 preceding siblings ...)
  2007-06-17 19:09 ` [PATCH 15/16] Unionfs: Change the semantics of sb info's rwsem Josef 'Jeff' Sipek
@ 2007-06-17 19:09 ` Josef 'Jeff' Sipek
  15 siblings, 0 replies; 17+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-06-17 19:09 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel; +Cc: akpm, Josef 'Jeff' Sipek

Since we use containers and the struct inode is _inside_ the
unionfs_inode_info structure, UNIONFS_I will always (given a non-NULL inode
pointer), return a valid non-NULL pointer.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/fanout.h |    8 +++++++-
 fs/unionfs/super.c  |    6 ------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index d4933ce..4da34c6 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -18,7 +18,13 @@
 #ifndef _FANOUT_H_
 #define _FANOUT_H_
 
-/* Inode to private data */
+/*
+ * Inode to private data
+ *
+ * Since we use containers and the struct inode is _inside_ the
+ * unionfs_inode_info structure, UNIONFS_I will always (given a non-NULL
+ * inode pointer), return a valid non-NULL pointer.
+ */
 static inline struct unionfs_inode_info *UNIONFS_I(const struct inode *inode)
 {
 	return container_of(inode, struct unionfs_inode_info, vfs_inode);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 9f248b9..0e6dad1 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -32,12 +32,6 @@ static void unionfs_read_inode(struct inode *inode)
 
 	unionfs_read_lock(inode->i_sb);
 
-	if (!info) {
-		printk(KERN_ERR "unionfs: no kernel memory when allocating "
-		       "inode private data!\n");
-		BUG();
-	}
-
 	memset(info, 0, offsetof(struct unionfs_inode_info, vfs_inode));
 	info->bstart = -1;
 	info->bend = -1;
-- 
1.5.2.rc1.165.gaf9b


^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2007-06-17 19:15 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-06-17 19:09 [GIT PULL -mm] Unionfs cleanups, fixes, and mmap Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 01/16] [PATCH] unionfs section mismatch Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 02/16] Unionfs: Don't revalidate dropped dentries Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 03/16] Unionfs: Retry lookup for different silly-renamed files Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 04/16] Unionfs: Set lower inodes correctly after branch management succeeds Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 05/16] Unionfs: call statfs on lower file system properly Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 06/16] MAINTAINERS: Add Erez Zadok as a maintainer of Unionfs Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 07/16] Unionfs: Add standard copyright comment to include/linux/union_fs.h Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 08/16] Unionfs: Remove unnecessary #define Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 09/16] Unionfs: mmap implementation Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 10/16] Unionfs: merge find_new_branch_index and branch_id_to_idx into one function Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 11/16] Unionfs: Revalidate dentries passed to all inode/super operations Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 12/16] Unionfs: Cleanup new_dentry_private_data Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 13/16] Unionfs: Change free_dentry_private_info to take a struct dentry Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 14/16] Unionfs: Add BUG_ONs to unionfs_lower_* Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 15/16] Unionfs: Change the semantics of sb info's rwsem Josef 'Jeff' Sipek
2007-06-17 19:09 ` [PATCH 16/16] Unionfs: Remove superfluous check for NULL pointer Josef 'Jeff' Sipek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).