2007-01-29 20:37:50

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [GIT PULL -mm] Unionfs updates/cleanups

The following patches (also available though the git tree) address a number
of code cleanliness issues with Unionfs.

You can pull from 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jsipek/unionfs.git

to receive the following:

Adrian Bunk (1):
fs/unionfs/: possible cleanups

Josef 'Jeff' Sipek (3):
fs/unionfs/: Remove stale_inode.c
fs/unionfs/: Andrew Morton's comments
fs/unionfs/: Don't duplicate the struct nameidata

fs/unionfs/branchman.c | 4 +-
fs/unionfs/commonfops.c | 54 +++++++++++-----------
fs/unionfs/copyup.c | 67 +++++++++++++++------------
fs/unionfs/dentry.c | 19 +++-----
fs/unionfs/fanout.h | 51 +++++++++++++++++----
fs/unionfs/file.c | 17 +++-----
fs/unionfs/inode.c | 69 +++++++++++++++-------------
fs/unionfs/lookup.c | 113 +++++++++++++++++++++++-----------------------
fs/unionfs/main.c | 32 +++++++-------
fs/unionfs/rdstate.c | 2 +-
fs/unionfs/rename.c | 8 ++--
fs/unionfs/sioq.c | 19 ++++----
fs/unionfs/sioq.h | 1 -
fs/unionfs/stale_inode.c | 112 ---------------------------------------------
fs/unionfs/subr.c | 65 +++++++++++++++++++++++++-
fs/unionfs/super.c | 7 +--
fs/unionfs/union.h | 84 +++-------------------------------
fs/unionfs/unlink.c | 8 ++--
fs/unionfs/xattr.c | 16 +++---
19 files changed, 330 insertions(+), 418 deletions(-)

Josef 'Jeff' Sipek.

[email protected]


2007-01-29 20:37:52

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 4/4] fs/unionfs/: Don't duplicate the struct nameidata

Josef 'Jeff' Sipek (3):
fs/unionfs/: Remove stale_inode.c
fs/unionfs/: Andrew Morton's comments
fs/unionfs/: Don't duplicate the struct nameidata

fs/unionfs/branchman.c | 4 +-
fs/unionfs/commonfops.c | 54 +++++++++++-----------
fs/unionfs/copyup.c | 67 +++++++++++++++------------
fs/unionfs/dentry.c | 19 +++-----
fs/unionfs/fanout.h | 51 +++++++++++++++++----
fs/unionfs/file.c | 17 +++-----
fs/unionfs/inode.c | 69 +++++++++++++++-------------
fs/unionfs/lookup.c | 113 +++++++++++++++++++++++-----------------------
fs/unionfs/main.c | 32 +++++++-------
fs/unionfs/rdstate.c | 2 +-
fs/unionfs/rename.c | 8 ++--
fs/unionfs/sioq.c | 19 ++++----
fs/unionfs/sioq.h | 1 -
fs/unionfs/stale_inode.c | 112 ---------------------------------------------
fs/unionfs/subr.c | 65 +++++++++++++++++++++++++-
fs/unionfs/super.c | 7 +--
fs/unionfs/union.h | 84 +++-------------------------------
fs/unionfs/unlink.c | 8 ++--
fs/unionfs/xattr.c | 16 +++---
19 files changed, 330 insertions(+), 418 deletions(-)

2007-01-29 20:38:25

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 3/4] fs/unionfs/: Andrew Morton's comments

- rename {,un}lock_dentry to unionfs_{,un}lock_dentry
- few minor coding style fixes
- removed prototypes from .c files
- replaced dbstart macros etc with static inlines
- replaced UNIONFS_D(d)->sem semaphore with a mutex
- renamed sioq struct workqueue to superio_workqueue
- made unionfs_get_nlinks and alloc_whname not inlined

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/branchman.c | 4 +-
fs/unionfs/commonfops.c | 48 ++++++++++----------
fs/unionfs/copyup.c | 8 +++-
fs/unionfs/dentry.c | 6 +-
fs/unionfs/fanout.h | 51 +++++++++++++++++----
fs/unionfs/inode.c | 28 ++++++------
fs/unionfs/lookup.c | 113 +++++++++++++++++++++++------------------------
fs/unionfs/main.c | 28 ++++++------
fs/unionfs/rename.c | 8 ++--
fs/unionfs/sioq.c | 19 ++++----
fs/unionfs/subr.c | 65 ++++++++++++++++++++++++++-
fs/unionfs/super.c | 7 +--
fs/unionfs/union.h | 70 +++--------------------------
fs/unionfs/unlink.c | 8 ++--
fs/unionfs/xattr.c | 16 +++---
15 files changed, 259 insertions(+), 220 deletions(-)

diff --git a/fs/unionfs/branchman.c b/fs/unionfs/branchman.c
index 9180de1..83d443a 100644
--- a/fs/unionfs/branchman.c
+++ b/fs/unionfs/branchman.c
@@ -54,7 +54,7 @@ int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
struct dentry *dentry, *hidden_dentry;

dentry = file->f_dentry;
- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
if ((err = unionfs_partial_lookup(dentry)))
goto out;
bstart = dbstart(dentry);
@@ -75,7 +75,7 @@ int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
err = -EFAULT;

out:
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err < 0 ? err : bend;
}

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 1806acf..379c525 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -33,7 +33,7 @@ static int copyup_deleted_file(struct file *file, struct dentry *dentry,

int err;
struct dentry *tmp_dentry = NULL;
- struct dentry *hidden_dentry = NULL;
+ struct dentry *hidden_dentry;
struct dentry *hidden_dir_dentry = NULL;

hidden_dentry = unionfs_lower_dentry_idx(dentry, bstart);
@@ -225,7 +225,7 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
int err = 0;

dentry = file->f_dentry;
- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
sb = dentry->d_sb;
unionfs_read_lock(sb);
if (!unionfs_d_revalidate(dentry, NULL) && !d_deleted(dentry)) {
@@ -286,7 +286,7 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
}

out:
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
}
@@ -406,7 +406,7 @@ int unionfs_open(struct inode *inode, struct file *file)
}

dentry = file->f_dentry;
- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
unionfs_read_lock(inode->i_sb);

bstart = fbstart(file) = dbstart(dentry);
@@ -436,7 +436,7 @@ int unionfs_open(struct inode *inode, struct file *file)
}
}

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(inode->i_sb);

out:
@@ -529,23 +529,23 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)

/* check if asked for local commands */
switch (cmd) {
- case UNIONFS_IOCTL_INCGEN:
- /* Increment the superblock generation count */
- err = -EACCES;
- if (!capable(CAP_SYS_ADMIN))
- goto out;
- err = unionfs_ioctl_incgen(file, cmd, arg);
- break;
-
- case UNIONFS_IOCTL_QUERYFILE:
- /* Return list of branches containing the given file */
- err = unionfs_ioctl_queryfile(file, cmd, arg);
- break;
-
- default:
- /* pass the ioctl down */
- err = do_ioctl(file, cmd, arg);
- break;
+ case UNIONFS_IOCTL_INCGEN:
+ /* Increment the superblock generation count */
+ err = -EACCES;
+ if (!capable(CAP_SYS_ADMIN))
+ goto out;
+ err = unionfs_ioctl_incgen(file, cmd, arg);
+ break;
+
+ case UNIONFS_IOCTL_QUERYFILE:
+ /* Return list of branches containing the given file */
+ err = unionfs_ioctl_queryfile(file, cmd, arg);
+ break;
+
+ default:
+ /* pass the ioctl down */
+ err = do_ioctl(file, cmd, arg);
+ break;
}

out:
@@ -564,7 +564,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
if (!atomic_dec_and_test(&UNIONFS_I(dentry->d_inode)->totalopens))
goto out;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

bstart = fbstart(file);
bend = fbend(file);
@@ -586,7 +586,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
}

out_lock:
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
out:
return err;
}
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 1ef8baf..998cc69 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -26,6 +26,10 @@ static struct dentry *create_parents_named(struct inode *dir,
struct dentry *dentry,
const char *name, int bindex);

+/* For detailed explanation of copyup see:
+ * Documentation/filesystems/unionfs/concepts.txt
+ */
+
#ifdef CONFIG_UNION_FS_XATTR
/* copyup all extended attrs for a given dentry */
static int copyup_xattrs(struct dentry *old_hidden_dentry,
@@ -646,7 +650,7 @@ static struct dentry *create_parents_named(struct inode *dir,

/* find the parent directory dentry in unionfs */
parent_dentry = child_dentry->d_parent;
- lock_dentry(parent_dentry);
+ unionfs_lock_dentry(parent_dentry);

/* find out the hidden_parent_dentry in the given branch */
hidden_parent_dentry = unionfs_lower_dentry_idx(parent_dentry, bindex);
@@ -682,7 +686,7 @@ static struct dentry *create_parents_named(struct inode *dir,
while (1) {
/* get hidden parent dir in the current branch */
hidden_parent_dentry = unionfs_lower_dentry_idx(parent_dentry, bindex);
- unlock_dentry(parent_dentry);
+ unionfs_unlock_dentry(parent_dentry);

/* init the values to lookup */
childname = child_dentry->d_name.name;
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index d7193cc..3721409 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -179,9 +179,9 @@ static int unionfs_d_revalidate_wrap(struct dentry *dentry,
{
int err;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
err = unionfs_d_revalidate(dentry, nd);
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);

return err;
}
@@ -194,7 +194,7 @@ static void unionfs_d_release(struct dentry *dentry)
* reference, but the printing functions verify that we have a lock
* on the dentry before calling dbstart, etc.
*/
- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

/* this could be a negative dentry, so check first */
if (!UNIONFS_D(dentry)) {
diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 6fe5d3f..e2acb75 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -130,12 +130,35 @@ static inline struct unionfs_dentry_info *UNIONFS_D(const struct dentry *dent)
return dent->d_fsdata;
}

-#define dbstart(dent) (UNIONFS_D(dent)->bstart)
-#define set_dbstart(dent, val) do { UNIONFS_D(dent)->bstart = val; } while(0)
-#define dbend(dent) (UNIONFS_D(dent)->bend)
-#define set_dbend(dent, val) do { UNIONFS_D(dent)->bend = val; } while(0)
-#define dbopaque(dent) (UNIONFS_D(dent)->bopaque)
-#define set_dbopaque(dent, val) do { UNIONFS_D(dent)->bopaque = val; } while (0)
+static inline int dbstart(const struct dentry *dent)
+{
+ return UNIONFS_D(dent)->bstart;
+}
+
+static inline void set_dbstart(struct dentry *dent, int val)
+{
+ UNIONFS_D(dent)->bstart = val;
+}
+
+static inline int dbend(const struct dentry *dent)
+{
+ return UNIONFS_D(dent)->bend;
+}
+
+static inline void set_dbend(struct dentry *dent, int val)
+{
+ UNIONFS_D(dent)->bend = val;
+}
+
+static inline int dbopaque(const struct dentry *dent)
+{
+ return UNIONFS_D(dent)->bopaque;
+}
+
+static inline void set_dbopaque(struct dentry *dent, int val)
+{
+ UNIONFS_D(dent)->bopaque = val;
+}

static inline void unionfs_set_lower_dentry_idx(struct dentry *dent, int index,
struct dentry *val)
@@ -170,8 +193,18 @@ static inline struct vfsmount *unionfs_lower_mnt(const struct dentry *dent)
}

/* Macros for locking a dentry. */
-#define lock_dentry(d) down(&UNIONFS_D(d)->sem)
-#define unlock_dentry(d) up(&UNIONFS_D(d)->sem)
-#define verify_locked(d)
+static inline void unionfs_lock_dentry(struct dentry *d)
+{
+ mutex_lock(&UNIONFS_D(d)->lock);
+}
+
+static inline void unionfs_unlock_dentry(struct dentry *d)
+{
+ mutex_unlock(&UNIONFS_D(d)->lock);
+}
+
+static inline void verify_locked(struct dentry *d)
+{
+}

#endif /* _FANOUT_H */
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 1adb83c..3b4a388 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -29,7 +29,7 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
int bindex = 0, bstart;
char *name = NULL;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

/* We start out in the leftmost branch. */
bstart = dbstart(dentry);
@@ -183,7 +183,7 @@ out:
dput(wh_dentry);
kfree(name);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -319,8 +319,8 @@ out:

kfree(name);

- unlock_dentry(new_dentry);
- unlock_dentry(old_dentry);
+ unionfs_unlock_dentry(new_dentry);
+ unionfs_unlock_dentry(old_dentry);

return err;
}
@@ -336,7 +336,7 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
int bindex = 0, bstart;
char *name = NULL;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

/* We start out in the leftmost branch. */
bstart = dbstart(dentry);
@@ -444,7 +444,7 @@ out:
d_drop(dentry);

kfree(name);
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -458,7 +458,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
int whiteout_unlinked = 0;
struct sioq_args args;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
bstart = dbstart(dentry);

hidden_dentry = unionfs_lower_dentry(dentry);
@@ -571,7 +571,7 @@ out:

kfree(name);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -585,7 +585,7 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
char *name = NULL;
int whiteout_unlinked = 0;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
bstart = dbstart(dentry);

hidden_dentry = unionfs_lower_dentry(dentry);
@@ -677,7 +677,7 @@ out:

kfree(name);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -687,7 +687,7 @@ static int unionfs_readlink(struct dentry *dentry, char __user * buf,
int err;
struct dentry *hidden_dentry;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
hidden_dentry = unionfs_lower_dentry(dentry);

if (!hidden_dentry->d_inode->i_op ||
@@ -701,7 +701,7 @@ static int unionfs_readlink(struct dentry *dentry, char __user * buf,
fsstack_copy_attr_atime(dentry->d_inode, hidden_dentry->d_inode);

out:
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -853,7 +853,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
int i;
int copyup = 0;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
bstart = dbstart(dentry);
bend = dbend(dentry);
inode = dentry->d_inode;
@@ -901,7 +901,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
fsstack_copy_inode_size(inode, hidden_inode);

out:
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 63b5719..df929e9 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -18,8 +18,56 @@

#include "union.h"

-static int is_opaque_dir(struct dentry *dentry, int bindex);
-static int is_validname(const char *name);
+/* is the filename valid == !(whiteout for a file or opaque dir marker) */
+static int is_validname(const char *name)
+{
+ if (!strncmp(name, UNIONFS_WHPFX, UNIONFS_WHLEN))
+ return 0;
+ if (!strncmp(name, UNIONFS_DIR_OPAQUE_NAME,
+ sizeof(UNIONFS_DIR_OPAQUE_NAME) - 1))
+ return 0;
+ return 1;
+}
+
+/* The rest of these are utility functions for lookup. */
+static int is_opaque_dir(struct dentry *dentry, int bindex)
+{
+ int err = 0;
+ struct dentry *hidden_dentry;
+ struct dentry *wh_hidden_dentry;
+ struct inode *hidden_inode;
+ struct sioq_args args;
+
+ hidden_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+ hidden_inode = hidden_dentry->d_inode;
+
+ BUG_ON(!S_ISDIR(hidden_inode->i_mode));
+
+ mutex_lock(&hidden_inode->i_mutex);
+
+ if (!permission(hidden_inode, MAY_EXEC, NULL))
+ wh_hidden_dentry = lookup_one_len(UNIONFS_DIR_OPAQUE, hidden_dentry,
+ sizeof(UNIONFS_DIR_OPAQUE) - 1);
+ else {
+ args.is_opaque.dentry = hidden_dentry;
+ run_sioq(__is_opaque_dir, &args);
+ wh_hidden_dentry = args.ret;
+ }
+
+ mutex_unlock(&hidden_inode->i_mutex);
+
+ if (IS_ERR(wh_hidden_dentry)) {
+ err = PTR_ERR(wh_hidden_dentry);
+ goto out;
+ }
+
+ /* This is an opaque dir iff wh_hidden_dentry is positive */
+ err = !!wh_hidden_dentry->d_inode;
+
+ dput(wh_hidden_dentry);
+out:
+ return err;
+}

struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *nd,
int lookupmode)
@@ -62,7 +110,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
parent_dentry = dget_parent(dentry);
/* We never partial lookup the root directory. */
if (parent_dentry != dentry) {
- lock_dentry(parent_dentry);
+ unionfs_lock_dentry(parent_dentry);
locked_parent = 1;
} else {
dput(parent_dentry);
@@ -330,10 +378,10 @@ out:
}
kfree(whname);
if (locked_parent)
- unlock_dentry(parent_dentry);
+ unionfs_unlock_dentry(parent_dentry);
dput(parent_dentry);
if (locked_child)
- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return ERR_PTR(err);
}

@@ -353,57 +401,6 @@ int unionfs_partial_lookup(struct dentry *dentry)
return -ENOSYS;
}

-/* The rest of these are utility functions for lookup. */
-static int is_opaque_dir(struct dentry *dentry, int bindex)
-{
- int err = 0;
- struct dentry *hidden_dentry;
- struct dentry *wh_hidden_dentry;
- struct inode *hidden_inode;
- struct sioq_args args;
-
- hidden_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- hidden_inode = hidden_dentry->d_inode;
-
- BUG_ON(!S_ISDIR(hidden_inode->i_mode));
-
- mutex_lock(&hidden_inode->i_mutex);
-
- if (!permission(hidden_inode, MAY_EXEC, NULL))
- wh_hidden_dentry = lookup_one_len(UNIONFS_DIR_OPAQUE, hidden_dentry,
- sizeof(UNIONFS_DIR_OPAQUE) - 1);
- else {
- args.is_opaque.dentry = hidden_dentry;
- run_sioq(__is_opaque_dir, &args);
- wh_hidden_dentry = args.ret;
- }
-
- mutex_unlock(&hidden_inode->i_mutex);
-
- if (IS_ERR(wh_hidden_dentry)) {
- err = PTR_ERR(wh_hidden_dentry);
- goto out;
- }
-
- /* This is an opaque dir iff wh_hidden_dentry is positive */
- err = !!wh_hidden_dentry->d_inode;
-
- dput(wh_hidden_dentry);
-out:
- return err;
-}
-
-/* is the filename valid == !(whiteout for a file or opaque dir marker) */
-static int is_validname(const char *name)
-{
- if (!strncmp(name, UNIONFS_WHPFX, UNIONFS_WHLEN))
- return 0;
- if (!strncmp(name, UNIONFS_DIR_OPAQUE_NAME,
- sizeof(UNIONFS_DIR_OPAQUE_NAME) - 1))
- return 0;
- return 1;
-}
-
/* The dentry cache is just so we have properly sized dentries. */
static struct kmem_cache *unionfs_dentry_cachep;
int unionfs_init_dentry_cache(void)
@@ -443,7 +440,9 @@ int new_dentry_private_data(struct dentry *dentry)

if (!info)
goto out;
- init_MUTEX_LOCKED(&info->sem);
+
+ mutex_init(&info->lock);
+ mutex_lock(&info->lock);

info->lower_paths = NULL;
} else
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 033eb7c..36d30bc 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -132,19 +132,19 @@ int unionfs_interpose(struct dentry *dentry, struct super_block *sb, int flag)
skip:
/* only (our) lookup wants to do a d_add */
switch (flag) {
- case INTERPOSE_DEFAULT:
- case INTERPOSE_REVAL_NEG:
- d_instantiate(dentry, inode);
- break;
- case INTERPOSE_LOOKUP:
- err = PTR_ERR(d_splice_alias(inode, dentry));
- break;
- case INTERPOSE_REVAL:
- /* Do nothing. */
- break;
- default:
- printk(KERN_ERR "Invalid interpose flag passed!");
- BUG();
+ case INTERPOSE_DEFAULT:
+ case INTERPOSE_REVAL_NEG:
+ d_instantiate(dentry, inode);
+ break;
+ case INTERPOSE_LOOKUP:
+ err = PTR_ERR(d_splice_alias(inode, dentry));
+ break;
+ case INTERPOSE_REVAL:
+ /* Do nothing. */
+ break;
+ default:
+ printk(KERN_ERR "Invalid interpose flag passed!");
+ BUG();
}

mutex_unlock(&inode->i_mutex);
@@ -586,7 +586,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
/* call interpose to create the upper level inode */
if ((err = unionfs_interpose(sb->s_root, sb, 0)))
goto out_freedpd;
- unlock_dentry(sb->s_root);
+ unionfs_unlock_dentry(sb->s_root);
goto out;

out_freedpd:
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index c561f3d..c9aa040 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -322,7 +322,7 @@ static struct dentry *lookup_whiteout(struct dentry *dentry)
return (void *)whname;

parent = dget_parent(dentry);
- lock_dentry(parent);
+ unionfs_lock_dentry(parent);
bstart = dbstart(parent);
bend = dbend(parent);
wh_dentry = ERR_PTR(-ENOENT);
@@ -339,7 +339,7 @@ static struct dentry *lookup_whiteout(struct dentry *dentry)
dput(wh_dentry);
wh_dentry = ERR_PTR(-ENOENT);
}
- unlock_dentry(parent);
+ unionfs_unlock_dentry(parent);
dput(parent);
kfree(whname);
return wh_dentry;
@@ -438,8 +438,8 @@ out:
if (S_ISDIR(old_dentry->d_inode->i_mode))
atomic_dec(&UNIONFS_D(old_dentry)->generation);

- unlock_dentry(new_dentry);
- unlock_dentry(old_dentry);
+ unionfs_unlock_dentry(new_dentry);
+ unionfs_unlock_dentry(old_dentry);
return err;
}

diff --git a/fs/unionfs/sioq.c b/fs/unionfs/sioq.c
index 3225f5b..7830b89 100644
--- a/fs/unionfs/sioq.c
+++ b/fs/unionfs/sioq.c
@@ -24,26 +24,26 @@
* whiteouts).
*/

-static struct workqueue_struct *sioq;
+static struct workqueue_struct *superio_workqueue;

int __init init_sioq(void)
{
int err;

- sioq = create_workqueue("unionfs_siod");
- if (!IS_ERR(sioq))
+ superio_workqueue = create_workqueue("unionfs_siod");
+ if (!IS_ERR(superio_workqueue))
return 0;

- err = PTR_ERR(sioq);
+ err = PTR_ERR(superio_workqueue);
printk(KERN_ERR "create_workqueue failed %d\n", err);
- sioq = NULL;
+ superio_workqueue = NULL;
return err;
}

void __exit stop_sioq(void)
{
- if (sioq)
- destroy_workqueue(sioq);
+ if (superio_workqueue)
+ destroy_workqueue(superio_workqueue);
}

void run_sioq(work_func_t func, struct sioq_args *args)
@@ -51,7 +51,7 @@ void run_sioq(work_func_t func, struct sioq_args *args)
INIT_WORK(&args->work, func);

init_completion(&args->comp);
- while (!queue_work(sioq, &args->work)) {
+ while (!queue_work(superio_workqueue, &args->work)) {
/* TODO: do accounting if needed */
schedule();
}
@@ -103,7 +103,8 @@ void __unionfs_unlink(struct work_struct *work)
complete(&args->comp);
}

-void __delete_whiteouts(struct work_struct *work) {
+void __delete_whiteouts(struct work_struct *work)
+{
struct sioq_args *args = container_of(work, struct sioq_args, work);
struct deletewh_args *d = &args->deletewh;

diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 15116e3..d274752 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -109,9 +109,9 @@ int unionfs_refresh_hidden_dentry(struct dentry *dentry, int bindex)

verify_locked(dentry);

- lock_dentry(dentry->d_parent);
+ unionfs_lock_dentry(dentry->d_parent);
hidden_parent = unionfs_lower_dentry_idx(dentry->d_parent, bindex);
- unlock_dentry(dentry->d_parent);
+ unionfs_unlock_dentry(dentry->d_parent);

BUG_ON(!S_ISDIR(hidden_parent->d_inode->i_mode));

@@ -170,3 +170,64 @@ out:
return err;
}

+/* returns the sum of the n_link values of all the underlying inodes of the
+ * passed inode
+ */
+int unionfs_get_nlinks(struct inode *inode)
+{
+ int sum_nlinks = 0;
+ int dirs = 0;
+ int bindex;
+ struct inode *hidden_inode;
+
+ /* don't bother to do all the work since we're unlinked */
+ if (inode->i_nlink == 0)
+ return 0;
+
+ if (!S_ISDIR(inode->i_mode))
+ return unionfs_lower_inode(inode)->i_nlink;
+
+ for (bindex = ibstart(inode); bindex <= ibend(inode); bindex++) {
+ hidden_inode = unionfs_lower_inode_idx(inode, bindex);
+
+ /* ignore files */
+ if (!hidden_inode || !S_ISDIR(hidden_inode->i_mode))
+ continue;
+
+ BUG_ON(hidden_inode->i_nlink < 0);
+
+ /* A deleted directory. */
+ if (hidden_inode->i_nlink == 0)
+ continue;
+ dirs++;
+
+ /*
+ * A broken directory...
+ *
+ * Some filesystems don't properly set the number of links
+ * on empty directories
+ */
+ if (hidden_inode->i_nlink == 1)
+ sum_nlinks += 2;
+ else
+ sum_nlinks += (hidden_inode->i_nlink - 2);
+ }
+
+ return (!dirs ? 0 : sum_nlinks + 2);
+}
+
+/* construct whiteout filename */
+char *alloc_whname(const char *name, int len)
+{
+ char *buf;
+
+ buf = kmalloc(len + UNIONFS_WHLEN + 1, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ strcpy(buf, UNIONFS_WHPFX);
+ strlcat(buf, name, len + UNIONFS_WHLEN + 1);
+
+ return buf;
+}
+
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 823cdfc..38443c7 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -295,7 +295,7 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
int bindex, bstart, bend;
int perms;

- lock_dentry(sb->s_root);
+ unionfs_lock_dentry(sb->s_root);

tmp_page = (char*) __get_free_page(GFP_KERNEL);
if (!tmp_page) {
@@ -321,10 +321,9 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
}

out:
- if (tmp_page)
- free_page((unsigned long) tmp_page);
+ free_page((unsigned long) tmp_page);

- unlock_dentry(sb->s_root);
+ unionfs_unlock_dentry(sb->s_root);

return ret;
}
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 0c61f80..8e9a1cc 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -111,7 +111,7 @@ struct unionfs_dentry_info {
* unionfs function from the VFS. Our lock ordering is that children
* go before their parents.
*/
- struct semaphore sem;
+ struct mutex lock;
int bstart;
int bend;
int bopaque;
@@ -226,8 +226,8 @@ static inline void double_lock_dentry(struct dentry *d1, struct dentry *d2)
d1 = d2;
d2 = tmp;
}
- lock_dentry(d1);
- lock_dentry(d2);
+ unionfs_lock_dentry(d1);
+ unionfs_lock_dentry(d2);
}

extern int new_dentry_private_data(struct dentry *dentry);
@@ -267,6 +267,8 @@ extern int remove_whiteouts(struct dentry *dentry, struct dentry *hidden_dentry,
extern int do_delete_whiteouts(struct dentry *dentry, int bindex,
struct unionfs_dir_state *namelist);

+extern int unionfs_get_nlinks(struct inode *inode);
+
/* Is this directory empty: 0 if it is empty, -ENOTEMPTY if not. */
extern int check_empty(struct dentry *dentry,
struct unionfs_dir_state **namelist);
@@ -336,52 +338,6 @@ static inline int d_deleted(struct dentry *d)
return d_unhashed(d) && (d != d->d_sb->s_root);
}

-/* returns the sum of the n_link values of all the underlying inodes of the
- * passed inode
- */
-static inline int unionfs_get_nlinks(struct inode *inode)
-{
- int sum_nlinks = 0;
- int dirs = 0;
- int bindex;
- struct inode *hidden_inode;
-
- /* don't bother to do all the work since we're unlinked */
- if (inode->i_nlink == 0)
- return 0;
-
- if (!S_ISDIR(inode->i_mode))
- return unionfs_lower_inode(inode)->i_nlink;
-
- for (bindex = ibstart(inode); bindex <= ibend(inode); bindex++) {
- hidden_inode = unionfs_lower_inode_idx(inode, bindex);
-
- /* ignore files */
- if (!hidden_inode || !S_ISDIR(hidden_inode->i_mode))
- continue;
-
- BUG_ON(hidden_inode->i_nlink < 0);
-
- /* A deleted directory. */
- if (hidden_inode->i_nlink == 0)
- continue;
- dirs++;
-
- /*
- * A broken directory...
- *
- * Some filesystems don't properly set the number of links
- * on empty directories
- */
- if (hidden_inode->i_nlink == 1)
- sum_nlinks += 2;
- else
- sum_nlinks += (hidden_inode->i_nlink - 2);
- }
-
- return (!dirs ? 0 : sum_nlinks + 2);
-}
-
struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *nd, int lookupmode);

#define IS_SET(sb, check_flag) ((check_flag) & MOUNT_FLAG(sb))
@@ -448,21 +404,6 @@ static inline int is_robranch(const struct dentry *dentry)
#define UNIONFS_DIR_OPAQUE_NAME "__dir_opaque"
#define UNIONFS_DIR_OPAQUE UNIONFS_WHPFX UNIONFS_DIR_OPAQUE_NAME

-/* construct whiteout filename */
-static inline char *alloc_whname(const char *name, int len)
-{
- char *buf;
-
- buf = kmalloc(len + UNIONFS_WHLEN + 1, GFP_KERNEL);
- if (!buf)
- return ERR_PTR(-ENOMEM);
-
- strcpy(buf, UNIONFS_WHPFX);
- strlcat(buf, name, len + UNIONFS_WHLEN + 1);
-
- return buf;
-}
-
#define VALID_MOUNT_FLAGS (0)

#ifndef DEFAULT_POLLMASK
@@ -472,6 +413,7 @@ static inline char *alloc_whname(const char *name, int len)
/*
* EXTERNALS:
*/
+extern char *alloc_whname(const char *name, int len);

/* These two functions are here because it is kind of daft to copy and paste the
* contents of the two functions to 32+ places in unionfs
diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index 9e690d2..f5b250a 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -74,14 +74,14 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
{
int err = 0;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

err = unionfs_unlink_whiteout(dir, dentry);
/* call d_drop so the system "forgets" about us */
if (!err)
d_drop(dentry);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -122,7 +122,7 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)
int err = 0;
struct unionfs_dir_state *namelist = NULL;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

/* check if this unionfs directory is empty or not */
err = check_empty(dentry, &namelist);
@@ -156,7 +156,7 @@ out:
if (namelist)
free_rdstate(namelist);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index 4705f46..d57f079 100644
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -56,13 +56,13 @@ ssize_t unionfs_getxattr(struct dentry * dentry, const char *name, void *value,
struct dentry *hidden_dentry = NULL;
int err = -EOPNOTSUPP;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

hidden_dentry = unionfs_lower_dentry(dentry);

err = vfs_getxattr(hidden_dentry, (char*) name, value, size);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -75,12 +75,12 @@ int unionfs_setxattr(struct dentry *dentry, const char *name, const void *value,
struct dentry *hidden_dentry = NULL;
int err = -EOPNOTSUPP;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
hidden_dentry = unionfs_lower_dentry(dentry);

err = vfs_setxattr(hidden_dentry, (char*) name, (void*) value, size, flags);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -92,12 +92,12 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
struct dentry *hidden_dentry = NULL;
int err = -EOPNOTSUPP;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);
hidden_dentry = unionfs_lower_dentry(dentry);

err = vfs_removexattr(hidden_dentry, (char*) name);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

@@ -110,14 +110,14 @@ ssize_t unionfs_listxattr(struct dentry * dentry, char *list, size_t size)
int err = -EOPNOTSUPP;
char *encoded_list = NULL;

- lock_dentry(dentry);
+ unionfs_lock_dentry(dentry);

hidden_dentry = unionfs_lower_dentry(dentry);

encoded_list = list;
err = vfs_listxattr(hidden_dentry, encoded_list, size);

- unlock_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
return err;
}

--
1.5.0.rc1.g5355

2007-01-29 20:38:33

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 4/4] fs/unionfs/: Don't duplicate the struct nameidata

The only fields that we have to watch out for are the dentry and vfsmount.
Additionally, this makes Unionfs gentler on the stack as nameidata is rather
large.

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/inode.c | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 3b4a388..1b2e8a8 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -191,15 +191,25 @@ static struct dentry *unionfs_lookup(struct inode *parent,
struct dentry *dentry,
struct nameidata *nd)
{
- struct nameidata lowernd; /* TODO: be gentler to the stack */
+ struct path path_save;
+ struct dentry *ret;

- if (nd)
- memcpy(&lowernd, nd, sizeof(struct nameidata));
- else
- memset(&lowernd, 0, sizeof(struct nameidata));
+ /* save the dentry & vfsmnt from namei */
+ if (nd) {
+ path_save.dentry = nd->dentry;
+ path_save.mnt = nd->mnt;
+ }

/* The locking is done by unionfs_lookup_backend. */
- return unionfs_lookup_backend(dentry, &lowernd, INTERPOSE_LOOKUP);
+ ret = unionfs_lookup_backend(dentry, nd, INTERPOSE_LOOKUP);
+
+ /* restore the dentry & vfsmnt in namei */
+ if (nd) {
+ nd->dentry = path_save.dentry;
+ nd->mnt = path_save.mnt;
+ }
+
+ return ret;
}

static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
--
1.5.0.rc1.g5355

2007-01-29 20:38:52

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 1/4] fs/unionfs/: Remove stale_inode.c

The stale inode operations were heavily based on bad inode operations. This
patch removes stale_inode.c and converts all users of stale_inode_ops to
bad_inode_ops as there seems to be no reason to return ESTALE instead of
EIO.

This is the more appropriate than porting the bad_inode.c fix (commit
be6aab0e9fa6d3c6d75aa1e38ac972d8b4ee82b8) to stale_inode.c.

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/dentry.c | 2 +-
fs/unionfs/stale_inode.c | 112 ----------------------------------------------
fs/unionfs/union.h | 1 -
3 files changed, 1 insertions(+), 114 deletions(-)
delete mode 100644 fs/unionfs/stale_inode.c

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 78de9c9..0b002d1 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -141,7 +141,7 @@ restart:
}

if (positive && UNIONFS_I(dentry->d_inode)->stale) {
- make_stale_inode(dentry->d_inode);
+ make_bad_inode(dentry->d_inode);
d_drop(dentry);
valid = 0;
goto out;
diff --git a/fs/unionfs/stale_inode.c b/fs/unionfs/stale_inode.c
deleted file mode 100644
index bce938d..0000000
--- a/fs/unionfs/stale_inode.c
+++ /dev/null
@@ -1,112 +0,0 @@
-/*
- * Adpated from linux/fs/bad_inode.c
- *
- * Copyright (C) 1997, Stephen Tweedie
- *
- * Provide stub functions for "stale" inodes, a bit friendlier than the
- * -EIO that bad_inode.c does.
- */
-
-#include <linux/version.h>
-
-#include <linux/fs.h>
-#include <linux/stat.h>
-#include <linux/sched.h>
-
-static struct address_space_operations unionfs_stale_aops;
-
-/* declarations for "sparse */
-extern struct inode_operations stale_inode_ops;
-
-/*
- * The follow_link operation is special: it must behave as a no-op
- * so that a stale root inode can at least be unmounted. To do this
- * we must dput() the base and return the dentry with a dget().
- */
-static void *stale_follow_link(struct dentry *dent, struct nameidata *nd)
-{
- return ERR_PTR(vfs_follow_link(nd, ERR_PTR(-ESTALE)));
-}
-
-static int return_ESTALE(void)
-{
- return -ESTALE;
-}
-
-#define ESTALE_ERROR ((void *) (return_ESTALE))
-
-static struct file_operations stale_file_ops = {
- .llseek = ESTALE_ERROR,
- .read = ESTALE_ERROR,
- .write = ESTALE_ERROR,
- .readdir = ESTALE_ERROR,
- .poll = ESTALE_ERROR,
- .ioctl = ESTALE_ERROR,
- .mmap = ESTALE_ERROR,
- .open = ESTALE_ERROR,
- .flush = ESTALE_ERROR,
- .release = ESTALE_ERROR,
- .fsync = ESTALE_ERROR,
- .fasync = ESTALE_ERROR,
- .lock = ESTALE_ERROR,
-};
-
-struct inode_operations stale_inode_ops = {
- .create = ESTALE_ERROR,
- .lookup = ESTALE_ERROR,
- .link = ESTALE_ERROR,
- .unlink = ESTALE_ERROR,
- .symlink = ESTALE_ERROR,
- .mkdir = ESTALE_ERROR,
- .rmdir = ESTALE_ERROR,
- .mknod = ESTALE_ERROR,
- .rename = ESTALE_ERROR,
- .readlink = ESTALE_ERROR,
- .follow_link = stale_follow_link,
- .truncate = ESTALE_ERROR,
- .permission = ESTALE_ERROR,
-};
-
-/*
- * When a filesystem is unable to read an inode due to an I/O error in
- * its read_inode() function, it can call make_stale_inode() to return a
- * set of stubs which will return ESTALE errors as required.
- *
- * We only need to do limited initialisation: all other fields are
- * preinitialised to zero automatically.
- */
-
-/**
- * make_stale_inode - mark an inode stale due to an I/O error
- * @inode: Inode to mark stale
- *
- * When an inode cannot be read due to a media or remote network
- * failure this function makes the inode "stale" and causes I/O operations
- * on it to fail from this point on.
- */
-void make_stale_inode(struct inode *inode)
-{
- inode->i_mode = S_IFREG;
- inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
- inode->i_op = &stale_inode_ops;
- inode->i_fop = &stale_file_ops;
- inode->i_mapping->a_ops = &unionfs_stale_aops;
-}
-
-/*
- * This tests whether an inode has been flagged as stale. The test uses
- * &stale_inode_ops to cover the case of invalidated inodes as well as
- * those created by make_stale_inode() above.
- */
-
-/**
- * is_stale_inode - is an inode errored
- * @inode: inode to test
- *
- * Returns true if the inode in question has been marked as stale.
- */
-int is_stale_inode(struct inode *inode)
-{
- return (inode->i_op == &stale_inode_ops);
-}
-
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index b6e03ff..637f9f0 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -396,7 +396,6 @@ static inline int unionfs_get_nlinks(struct inode *inode)

struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *nd, int lookupmode);
int is_stale_inode(struct inode *inode);
-void make_stale_inode(struct inode *inode);

#define IS_SET(sb, check_flag) ((check_flag) & MOUNT_FLAG(sb))

--
1.5.0.rc1.g5355

2007-01-29 20:39:18

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 2/4] fs/unionfs/: possible cleanups

From: Adrian Bunk <[email protected]> - unquoted

This patch contains the following possible cleanups:
- every function should #include the headers containing the prototypes
of it's global functions
- static functions in C files shouldn't be marked "inline", gcc should
know best when to inline them
- make needlessly global code static
- #if 0 the following unused global function:
- stale_inode.c: is_stale_inode()

Signed-off-by: Adrian Bunk <[email protected]>
[removed stale inode related fixes as stale_inode.c is gone]
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/commonfops.c | 6 ++--
fs/unionfs/copyup.c | 59 +++++++++++++++++++++++++---------------------
fs/unionfs/dentry.c | 11 ++------
fs/unionfs/file.c | 17 ++++--------
fs/unionfs/inode.c | 19 +++++---------
fs/unionfs/main.c | 4 +-
fs/unionfs/rdstate.c | 2 +-
fs/unionfs/sioq.c | 2 +-
fs/unionfs/sioq.h | 1 -
fs/unionfs/union.h | 13 ----------
10 files changed, 55 insertions(+), 79 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 6226a34..1806acf 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -292,7 +292,7 @@ out:
}

/* unionfs_open helper function: open a directory */
-static inline int __open_dir(struct inode *inode, struct file *file)
+static int __open_dir(struct inode *inode, struct file *file)
{
struct dentry *hidden_dentry;
struct file *hidden_file;
@@ -326,7 +326,7 @@ static inline int __open_dir(struct inode *inode, struct file *file)
}

/* unionfs_open helper function: open a file */
-static inline int __open_file(struct inode *inode, struct file *file)
+static int __open_file(struct inode *inode, struct file *file)
{
struct dentry *hidden_dentry;
struct file *hidden_file;
@@ -493,7 +493,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
}

/* pass the ioctl to the lower fs */
-static inline long do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
struct file *hidden_file;
int err;
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 97ab571..1ef8baf 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -18,6 +18,14 @@

#include "union.h"

+static int copyup_named_dentry(struct inode *dir, struct dentry *dentry,
+ int bstart, int new_bindex, const char *name,
+ int namelen, struct file **copyup_file,
+ loff_t len);
+static struct dentry *create_parents_named(struct inode *dir,
+ struct dentry *dentry,
+ const char *name, int bindex);
+
#ifdef CONFIG_UNION_FS_XATTR
/* copyup all extended attrs for a given dentry */
static int copyup_xattrs(struct dentry *old_hidden_dentry,
@@ -129,10 +137,10 @@ int copyup_dentry(struct inode *dir, struct dentry *dentry,
* if the object being copied up is a regular file, the file is only created,
* the contents have to be copied up separately
*/
-static inline int __copyup_ndentry(struct dentry *old_hidden_dentry,
- struct dentry *new_hidden_dentry,
- struct dentry *new_hidden_parent_dentry,
- char *symbuf)
+static int __copyup_ndentry(struct dentry *old_hidden_dentry,
+ struct dentry *new_hidden_dentry,
+ struct dentry *new_hidden_parent_dentry,
+ char *symbuf)
{
int err = 0;
umode_t old_mode = old_hidden_dentry->d_inode->i_mode;
@@ -179,13 +187,10 @@ static inline int __copyup_ndentry(struct dentry *old_hidden_dentry,
return err;
}

-static inline int __copyup_reg_data(struct dentry *dentry,
- struct dentry *new_hidden_dentry,
- int new_bindex,
- struct dentry *old_hidden_dentry,
- int old_bindex,
- struct file **copyup_file,
- loff_t len)
+static int __copyup_reg_data(struct dentry *dentry,
+ struct dentry *new_hidden_dentry, int new_bindex,
+ struct dentry *old_hidden_dentry, int old_bindex,
+ struct file **copyup_file, loff_t len)
{
struct super_block *sb = dentry->d_sb;
struct file *input_file;
@@ -300,11 +305,9 @@ out:
/* dput the lower references for old and new dentry & clear a lower dentry
* pointer
*/
-static inline void __clear(struct dentry *dentry,
- struct dentry *old_hidden_dentry,
- int old_bstart, int old_bend,
- struct dentry *new_hidden_dentry,
- int new_bindex)
+static void __clear(struct dentry *dentry, struct dentry *old_hidden_dentry,
+ int old_bstart, int old_bend,
+ struct dentry *new_hidden_dentry, int new_bindex)
{
/* get rid of the hidden dentry and all its traces */
unionfs_set_lower_dentry_idx(dentry, new_bindex, NULL);
@@ -316,9 +319,10 @@ static inline void __clear(struct dentry *dentry,
}

/* copy up a dentry to a file of specified name */
-int copyup_named_dentry(struct inode *dir, struct dentry *dentry,
- int bstart, int new_bindex, const char *name,
- int namelen, struct file **copyup_file, loff_t len)
+static int copyup_named_dentry(struct inode *dir, struct dentry *dentry,
+ int bstart, int new_bindex, const char *name,
+ int namelen, struct file **copyup_file,
+ loff_t len)
{
struct dentry *new_hidden_dentry;
struct dentry *old_hidden_dentry = NULL;
@@ -510,8 +514,8 @@ struct dentry *create_parents(struct inode *dir, struct dentry *dentry,
return create_parents_named(dir, dentry, dentry->d_name.name, bindex);
}

-static inline void __cleanup_dentry(struct dentry * dentry, int bindex,
- int old_bstart, int old_bend)
+static void __cleanup_dentry(struct dentry * dentry, int bindex,
+ int old_bstart, int old_bend)
{
int loop_start;
int loop_end;
@@ -557,8 +561,8 @@ static inline void __cleanup_dentry(struct dentry * dentry, int bindex,
}

/* set lower inode ptr and update bstart & bend if necessary */
-static inline void __set_inode(struct dentry * upper, struct dentry * lower,
- int bindex)
+static void __set_inode(struct dentry * upper, struct dentry * lower,
+ int bindex)
{
unionfs_set_lower_inode_idx(upper->d_inode, bindex,
igrab(lower->d_inode));
@@ -570,8 +574,8 @@ static inline void __set_inode(struct dentry * upper, struct dentry * lower,
}

/* set lower dentry ptr and update bstart & bend if necessary */
-static inline void __set_dentry(struct dentry * upper, struct dentry * lower,
- int bindex)
+static void __set_dentry(struct dentry * upper, struct dentry * lower,
+ int bindex)
{
unionfs_set_lower_dentry_idx(upper, bindex, lower);
if (likely(dbstart(upper) > bindex))
@@ -583,8 +587,9 @@ static inline void __set_dentry(struct dentry * upper, struct dentry * lower,
/* This function replicates the directory structure upto given dentry
* in the bindex branch.
*/
-struct dentry *create_parents_named(struct inode *dir, struct dentry *dentry,
- const char *name, int bindex)
+static struct dentry *create_parents_named(struct inode *dir,
+ struct dentry *dentry,
+ const char *name, int bindex)
{
int err;
struct dentry *child_dentry;
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 0b002d1..d7193cc 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -18,12 +18,6 @@

#include "union.h"

-/* declarations added for "sparse" */
-extern int unionfs_d_revalidate_wrap(struct dentry *dentry,
- struct nameidata *nd);
-extern void unionfs_d_release(struct dentry *dentry);
-extern void unionfs_d_iput(struct dentry *dentry, struct inode *inode);
-
/*
* returns 1 if valid, 0 otherwise.
*/
@@ -180,7 +174,8 @@ out:
return valid;
}

-int unionfs_d_revalidate_wrap(struct dentry *dentry, struct nameidata *nd)
+static int unionfs_d_revalidate_wrap(struct dentry *dentry,
+ struct nameidata *nd)
{
int err;

@@ -191,7 +186,7 @@ int unionfs_d_revalidate_wrap(struct dentry *dentry, struct nameidata *nd)
return err;
}

-void unionfs_d_release(struct dentry *dentry)
+static void unionfs_d_release(struct dentry *dentry)
{
int bindex, bstart, bend;

diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index d056e4f..9ce092d 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -18,11 +18,6 @@

#include "union.h"

-/* declarations for sparse */
-extern ssize_t unionfs_read(struct file *, char __user *, size_t, loff_t *);
-extern ssize_t unionfs_write(struct file *, const char __user *, size_t,
- loff_t *);
-
/*******************
* File Operations *
*******************/
@@ -56,8 +51,8 @@ out:
return err;
}

-ssize_t unionfs_read(struct file * file, char __user * buf, size_t count,
- loff_t * ppos)
+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;
@@ -78,8 +73,8 @@ out:
return err;
}

-ssize_t __unionfs_write(struct file * file, const char __user * buf,
- size_t count, loff_t * ppos)
+static ssize_t __unionfs_write(struct file * file, const char __user * buf,
+ size_t count, loff_t * ppos)
{
int err = -EINVAL;
struct file *hidden_file = NULL;
@@ -123,8 +118,8 @@ out:
return err;
}

-ssize_t unionfs_write(struct file * file, const char __user * buf, size_t count,
- loff_t * ppos)
+static ssize_t unionfs_write(struct file * file, const char __user * buf,
+ size_t count, loff_t * ppos)
{
int err = 0;

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 8246d15..1adb83c 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -18,14 +18,6 @@

#include "union.h"

-/* declarations added for "sparse" */
-extern struct dentry *unionfs_lookup(struct inode *, struct dentry *,
- struct nameidata *);
-extern int unionfs_readlink(struct dentry *dentry, char __user * buf,
- int bufsiz);
-extern void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
- void *cookie);
-
static int unionfs_create(struct inode *parent, struct dentry *dentry,
int mode, struct nameidata *nd)
{
@@ -195,8 +187,9 @@ out:
return err;
}

-struct dentry *unionfs_lookup(struct inode *parent, struct dentry *dentry,
- struct nameidata *nd)
+static struct dentry *unionfs_lookup(struct inode *parent,
+ struct dentry *dentry,
+ struct nameidata *nd)
{
struct nameidata lowernd; /* TODO: be gentler to the stack */

@@ -688,7 +681,8 @@ out:
return err;
}

-int unionfs_readlink(struct dentry *dentry, char __user * buf, int bufsiz)
+static int unionfs_readlink(struct dentry *dentry, char __user * buf,
+ int bufsiz)
{
int err;
struct dentry *hidden_dentry;
@@ -743,7 +737,8 @@ out:
return ERR_PTR(err);
}

-void unionfs_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie)
+static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
+ void *cookie)
{
kfree(nd_get_link(nd));
}
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index ae0a148..033eb7c 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -188,7 +188,7 @@ void unionfs_reinterpose(struct dentry *dentry)
* 2) it exists
* 3) is a directory
*/
-int check_branch(struct nameidata *nd)
+static int check_branch(struct nameidata *nd)
{
if (!strcmp(nd->dentry->d_sb->s_type->name, "unionfs"))
return -EINVAL;
@@ -200,7 +200,7 @@ int check_branch(struct nameidata *nd)
}

/* checks if two hidden_dentries have overlapping branches */
-int is_branch_overlap(struct dentry *dent1, struct dentry *dent2)
+static int is_branch_overlap(struct dentry *dent1, struct dentry *dent2)
{
struct dentry *dent = NULL;

diff --git a/fs/unionfs/rdstate.c b/fs/unionfs/rdstate.c
index 7f11ae2..16ce1bf 100644
--- a/fs/unionfs/rdstate.c
+++ b/fs/unionfs/rdstate.c
@@ -232,7 +232,7 @@ struct filldir_node *find_filldir_node(struct unionfs_dir_state *rdstate,
return cursor;
}

-inline struct filldir_node *alloc_filldir_node(const char *name, int namelen,
+static struct filldir_node *alloc_filldir_node(const char *name, int namelen,
unsigned int hash, int bindex)
{
return kmem_cache_alloc(unionfs_filldir_cachep, GFP_KERNEL);
diff --git a/fs/unionfs/sioq.c b/fs/unionfs/sioq.c
index a8bc493..3225f5b 100644
--- a/fs/unionfs/sioq.c
+++ b/fs/unionfs/sioq.c
@@ -24,7 +24,7 @@
* whiteouts).
*/

-struct workqueue_struct *sioq;
+static struct workqueue_struct *sioq;

int __init init_sioq(void)
{
diff --git a/fs/unionfs/sioq.h b/fs/unionfs/sioq.h
index 5a93414..20e3b0c 100644
--- a/fs/unionfs/sioq.h
+++ b/fs/unionfs/sioq.h
@@ -61,7 +61,6 @@ struct sioq_args {
};
};

-extern struct workqueue_struct *sioq;
extern int __init init_sioq(void);
extern __exit void stop_sioq(void);
extern void run_sioq(work_func_t func, struct sioq_args *args);
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 637f9f0..0c61f80 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -241,11 +241,6 @@ void update_bstart(struct dentry *dentry);
/* replicates the directory structure upto given dentry in given branch */
extern struct dentry *create_parents(struct inode *dir, struct dentry *dentry,
int bindex);
-struct dentry *create_parents_named(struct inode *dir, struct dentry *dentry,
- const char *name, int bindex);
-
-/* check if two branches overlap */
-extern int is_branch_overlap(struct dentry *dent1, struct dentry *dent2);

/* partial lookup */
extern int unionfs_partial_lookup(struct dentry *dentry);
@@ -265,10 +260,6 @@ extern int copyup_named_file(struct inode *dir, struct file *file,
/* copies a dentry from dbstart to newbindex branch */
extern int copyup_dentry(struct inode *dir, struct dentry *dentry, int bstart,
int new_bindex, struct file **copyup_file, loff_t len);
-extern int copyup_named_dentry(struct inode *dir, struct dentry *dentry,
- int bstart, int new_bindex, const char *name,
- int namelen, struct file **copyup_file,
- loff_t len);

extern int remove_whiteouts(struct dentry *dentry, struct dentry *hidden_dentry,
int bindex);
@@ -325,9 +316,6 @@ int unionfs_ioctl_incgen(struct file *file, unsigned int cmd,
int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
unsigned long arg);

-/* Verify that a branch is valid. */
-int check_branch(struct nameidata *nd);
-
#ifdef CONFIG_UNION_FS_XATTR
/* Extended attribute functions. */
extern void *unionfs_xattr_alloc(size_t size, size_t limit);
@@ -395,7 +383,6 @@ static inline int unionfs_get_nlinks(struct inode *inode)
}

struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *nd, int lookupmode);
-int is_stale_inode(struct inode *inode);

#define IS_SET(sb, check_flag) ((check_flag) & MOUNT_FLAG(sb))

--
1.5.0.rc1.g5355

2007-01-29 20:40:42

by Josef Sipek

[permalink] [raw]
Subject: Re: [PATCH 4/4] fs/unionfs/: Don't duplicate the struct nameidata

On Mon, Jan 29, 2007 at 03:37:43PM -0500, Josef 'Jeff' Sipek wrote:
> Josef 'Jeff' Sipek (3):
> fs/unionfs/: Remove stale_inode.c
> fs/unionfs/: Andrew Morton's comments
> fs/unionfs/: Don't duplicate the struct nameidata
>
> fs/unionfs/branchman.c | 4 +-
> fs/unionfs/commonfops.c | 54 +++++++++++-----------
> fs/unionfs/copyup.c | 67 +++++++++++++++------------
> fs/unionfs/dentry.c | 19 +++-----
> fs/unionfs/fanout.h | 51 +++++++++++++++++----
> fs/unionfs/file.c | 17 +++-----
> fs/unionfs/inode.c | 69 +++++++++++++++-------------
> fs/unionfs/lookup.c | 113 +++++++++++++++++++++++-----------------------
> fs/unionfs/main.c | 32 +++++++-------
> fs/unionfs/rdstate.c | 2 +-
> fs/unionfs/rename.c | 8 ++--
> fs/unionfs/sioq.c | 19 ++++----
> fs/unionfs/sioq.h | 1 -
> fs/unionfs/stale_inode.c | 112 ---------------------------------------------
> fs/unionfs/subr.c | 65 +++++++++++++++++++++++++-
> fs/unionfs/super.c | 7 +--
> fs/unionfs/union.h | 84 +++-------------------------------
> fs/unionfs/unlink.c | 8 ++--
> fs/unionfs/xattr.c | 16 +++---
> 19 files changed, 330 insertions(+), 418 deletions(-)

Not sure why this was sent. The other 4/4 email is the correct one.

Josef "Jeff" Sipek.

--
Bad pun of the week: The formula 1 control computer suffered from a race
condition

2007-01-30 09:42:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 4/4] fs/unionfs/: Don't duplicate the struct nameidata

On Mon, Jan 29, 2007 at 03:37:42PM -0500, Josef 'Jeff' Sipek wrote:
> The only fields that we have to watch out for are the dentry and vfsmount.
> Additionally, this makes Unionfs gentler on the stack as nameidata is rather
> large.

That's onviously not true at all. To handle any filesystems using intents
(e.g. NFSv4) you need to do much more. Then again doing things correctly
doesn't seem to be interesting to the stackable filesystems crowd an this
problem has been constantly ignored over the last year, including merging
ecryptfs which has been broken in the same way.

Folks, if you want your stackable filesystem toys taken seriously you
need to fix these kind of issues instead of talking them away. And yes,
this will involve some surgery to the VFS.

2007-01-30 17:52:37

by Josef Sipek

[permalink] [raw]
Subject: Re: [PATCH 4/4] fs/unionfs/: Don't duplicate the struct nameidata

On Tue, Jan 30, 2007 at 09:42:33AM +0000, Christoph Hellwig wrote:
> On Mon, Jan 29, 2007 at 03:37:42PM -0500, Josef 'Jeff' Sipek wrote:
> > The only fields that we have to watch out for are the dentry and vfsmount.
> > Additionally, this makes Unionfs gentler on the stack as nameidata is rather
> > large.
>
> That's onviously not true at all. To handle any filesystems using intents
> (e.g. NFSv4) you need to do much more. Then again doing things correctly
> doesn't seem to be interesting to the stackable filesystems crowd an this
> problem has been constantly ignored over the last year, including merging
> ecryptfs which has been broken in the same way.
>
> Folks, if you want your stackable filesystem toys taken seriously you
> need to fix these kind of issues instead of talking them away. And yes,
> this will involve some surgery to the VFS.

Indeed. I asked around (#linuxfs) and it seemed that restoring the
dentry/vfsmount was sufficient for the purpose of passing intents down. If
this is not the case, I'll revert the patch to do the full namei allocation.

Josef "Jeff" Sipek.

--
Computer Science is no more about computers than astronomy is about
telescopes.
- Edsger Dijkstra