2007-06-17 19:12:32

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [GIT PULL -mm] Unionfs cleanups, fixes, and mmap

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 <[email protected]>


2007-06-17 19:09:46

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 13/16] Unionfs: Change free_dentry_private_info to take a struct dentry

This makes it more symmetric with new_dentry_private_info.

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:10:13

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 12/16] Unionfs: Cleanup new_dentry_private_data

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:10:44

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 16/16] Unionfs: Remove superfluous check for NULL pointer

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 <[email protected]>
---
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

2007-06-17 19:11:10

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 14/16] Unionfs: Add BUG_ONs to unionfs_lower_*

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:11:31

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 11/16] Unionfs: Revalidate dentries passed to all inode/super operations

From: Erez Zadok <[email protected]>

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 <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:11:47

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 15/16] Unionfs: Change the semantics of sb info's rwsem

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 <[email protected]>
---
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

2007-06-17 19:12:09

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 02/16] Unionfs: Don't revalidate dropped dentries

From: Erez Zadok <[email protected]>

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 <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:12:46

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 06/16] MAINTAINERS: Add Erez Zadok as a maintainer of Unionfs

From: Erez Zadok <[email protected]>

Signed-off-by: Erez Zadok <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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: [email protected]
P: Josef "Jeff" Sipek
M: [email protected]
L: [email protected]
--
1.5.2.rc1.165.gaf9b

2007-06-17 19:13:06

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 04/16] Unionfs: Set lower inodes correctly after branch management succeeds

From: Erez Zadok <[email protected]>

Signed-off-by: Erez Zadok <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:13:25

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 07/16] Unionfs: Add standard copyright comment to include/linux/union_fs.h

From: Erez Zadok <[email protected]>

Signed-off-by: Erez Zadok <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:13:44

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 03/16] Unionfs: Retry lookup for different silly-renamed files

From: Erez Zadok <[email protected]>

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 <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:14:08

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 10/16] Unionfs: merge find_new_branch_index and branch_id_to_idx into one function

From: Erez Zadok <[email protected]>

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

Signed-off-by: Erez Zadok <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:14:29

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 01/16] [PATCH] unionfs section mismatch

From: Randy Dunlap <[email protected]>

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 <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:14:47

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 08/16] Unionfs: Remove unnecessary #define

From: Erez Zadok <[email protected]>

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 <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:15:12

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 05/16] Unionfs: call statfs on lower file system properly

From: Erez Zadok <[email protected]>

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

Signed-off-by: Erez Zadok <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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

2007-06-17 19:15:58

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 09/16] Unionfs: mmap implementation

From: Yiannis Pericleous <[email protected]>

Signed-off-by: Shaya Potter <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
Signed-off-by: Yiannis Pericleous <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
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