The following is a series of patchsets related to Unionfs. The most
significant changes are several fixes to races/locking. This release also
supports newer APIs in 2.6.25-rc: not using iget/read_inode, using the
changed d_path, using the revised nameidata which embeds a struct path, and
using path_get/put.
These patches were tested (where appropriate) on 2.6.25, MM, as well as the
backports to 2.6.{24,23,22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs,
nfs2/3/4, jffs2, ramfs, tmpfs, cramfs, and squashfs (where available). Also
tested with LTP-full and with a continuous parallel kernel compile (while
forcing cache flushing, manipulating lower branches, etc.). See
http://unionfs.filesystems.org/ to download back-ported unionfs code.
Please pull from the 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ezk/unionfs.git
to receive the following:
Andrew Morton (1):
Unionfs: embed a struct path into struct nameidata instead of nd dentrymnt
David Howells (1):
Unionfs: stop using iget() and read_inode()
Erez Zadok (14):
Unionfs: grab lower super_block references
Unionfs: ensure consistent lower inodes types
Unionfs: document behavior when the lower topology changes
Unionfs: uninline unionfs_copy_attr_times and unionfs_copy_attr_all
Unionfs: initialize path_save variable
Unionfs: extend dentry branch configuration lock in open
Unionfs: follow_link locking fixes
Unionfs: improve debugging in copy_attr_times
Unionfs: revalidation code cleanup and refactoring
Unionfs: factor out revalidation routine
Unionfs: lock parents' branch configuration fixes
Unionfs: branch management/configuration fixes
Unionfs: use dget_parent in revalidation code
VFS/Unionfs: use generic path_get/path_put functions
Jan Blunck (1):
Unionfs: use the new path_put
Documentation/filesystems/unionfs/concepts.txt | 13 +
fs/unionfs/commonfops.c | 54 ++++---
fs/unionfs/copyup.c | 3
fs/unionfs/dentry.c | 182 ++++++++++++++-----------
fs/unionfs/fanout.h | 50 ------
fs/unionfs/inode.c | 78 +++++++---
fs/unionfs/lookup.c | 13 +
fs/unionfs/main.c | 26 +--
fs/unionfs/mmap.c | 17 --
fs/unionfs/rename.c | 7
fs/unionfs/subr.c | 56 +++++++
fs/unionfs/super.c | 72 ++++++---
fs/unionfs/union.h | 13 +
fs/unionfs/unlink.c | 11 +
include/linux/namei.h | 12 -
15 files changed, 366 insertions(+), 241 deletions(-)
Thanks.
---
Erez Zadok
[email protected]
This prevents the lower super_block from being destroyed too early, when a
lower file system is being unmounted with MNT_FORCE or MNT_DETACH.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/main.c | 3 +++
fs/unionfs/super.c | 14 ++++++++++++++
fs/unionfs/union.h | 2 +-
3 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 23c18f7..ba3471d 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -636,6 +636,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
sbend(sb) = bend = lower_root_info->bend;
for (bindex = bstart; bindex <= bend; bindex++) {
struct dentry *d = lower_root_info->lower_paths[bindex].dentry;
+ atomic_inc(&d->d_sb->s_active);
unionfs_set_lower_super_idx(sb, bindex, d->d_sb);
}
@@ -711,6 +712,8 @@ out_dput:
dput(d);
/* initializing: can't use unionfs_mntput here */
mntput(m);
+ /* drop refs we took earlier */
+ atomic_dec(&d->d_sb->s_active);
}
kfree(lower_root_info->lower_paths);
kfree(lower_root_info);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 986c980..175840f 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -116,6 +116,14 @@ static void unionfs_put_super(struct super_block *sb)
}
BUG_ON(leaks != 0);
+ /* decrement lower super references */
+ for (bindex = bstart; bindex <= bend; bindex++) {
+ struct super_block *s;
+ s = unionfs_lower_super_idx(sb, bindex);
+ unionfs_set_lower_super_idx(sb, bindex, NULL);
+ atomic_dec(&s->s_active);
+ }
+
kfree(spd->data);
kfree(spd);
sb->s_fs_info = NULL;
@@ -729,6 +737,12 @@ out_no_change:
*/
purge_sb_data(sb);
+ /* grab new lower super references; release old ones */
+ for (i = 0; i < new_branches; i++)
+ atomic_inc(&new_data[i].sb->s_active);
+ for (i = 0; i < new_branches; i++)
+ atomic_dec(&UNIONFS_SB(sb)->data[i].sb->s_active);
+
/* copy new vectors into their correct place */
tmp_data = UNIONFS_SB(sb)->data;
UNIONFS_SB(sb)->data = new_data;
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 4b4d6c9..786c4be 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -127,7 +127,7 @@ struct unionfs_dentry_info {
/* These are the pointers to our various objects. */
struct unionfs_data {
- struct super_block *sb;
+ struct super_block *sb; /* lower super_block */
atomic_t open_files; /* number of open files on branch */
int branchperms;
int branch_id; /* unique branch ID at re/mount time */
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
---
Documentation/filesystems/unionfs/concepts.txt | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Documentation/filesystems/unionfs/concepts.txt b/Documentation/filesystems/unionfs/concepts.txt
index bed69bd..8d9a1c5 100644
--- a/Documentation/filesystems/unionfs/concepts.txt
+++ b/Documentation/filesystems/unionfs/concepts.txt
@@ -210,4 +210,17 @@ there's a lot of concurrent activity on both the upper and lower objects,
for the same file(s). Lastly, this delayed time attribute detection is
similar to how NFS clients operate (e.g., acregmin).
+Finally, there is no way currently in Linux to prevent lower directories
+from being moved around (i.e., topology changes); there's no way to prevent
+modifications to directory sub-trees of whole file systems which are mounted
+read-write. It is therefore possible for in-flight operations in unionfs to
+take place, while a lower directory is being moved around. Therefore, if
+you try to, say, create a new file in a directory through unionfs, while the
+directory is being moved around directly, then the new file may get created
+in the new location where that directory was moved to. This is a somewhat
+similar behaviour in NFS: an NFS client could be creating a new file while
+th NFS server is moving th directory around; the file will get successfully
+created in the new location. (The one exception in unionfs is that if the
+branch is marked read-only by unionfs, then a copyup will take place.)
+
For more information, see <http://unionfs.filesystems.org/>.
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/inode.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 8d939dc..6377533 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -820,7 +820,11 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
err = 0;
out:
- unionfs_check_dentry(dentry);
+ if (!err) {
+ unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+ unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
+ }
unionfs_check_nd(nd);
return ERR_PTR(err);
}
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/dentry.c | 55 ++++++++++++++++++++++++++++++++------------------
1 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index cd15243..afa2120 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -18,6 +18,39 @@
#include "union.h"
+
+static inline void __dput_lowers(struct dentry *dentry, int start, int end)
+{
+ struct dentry *lower_dentry;
+ int bindex;
+
+ if (start < 0)
+ return;
+ for (bindex = start; bindex <= end; bindex++) {
+ lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+ if (!lower_dentry)
+ continue;
+ unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
+ dput(lower_dentry);
+ }
+}
+
+static inline void __iput_lowers(struct inode *inode, int start, int end)
+{
+ struct inode *lower_inode;
+ int bindex;
+
+ if (start < 0)
+ return;
+ for (bindex = start; bindex <= end; bindex++) {
+ lower_inode = unionfs_lower_inode_idx(inode, bindex);
+ if (!lower_inode)
+ continue;
+ unionfs_set_lower_inode_idx(inode, bindex, NULL);
+ iput(lower_inode);
+ }
+}
+
/*
* Revalidate a single dentry.
* Assume that dentry's info node is locked.
@@ -72,15 +105,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
/* Free the pointers for our inodes and this dentry. */
bstart = dbstart(dentry);
bend = dbend(dentry);
- if (bstart >= 0) {
- struct dentry *lower_dentry;
- for (bindex = bstart; bindex <= bend; bindex++) {
- lower_dentry =
- unionfs_lower_dentry_idx(dentry,
- bindex);
- dput(lower_dentry);
- }
- }
+ __dput_lowers(dentry, bstart, bend);
set_dbstart(dentry, -1);
set_dbend(dentry, -1);
@@ -90,17 +115,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
bstart = ibstart(dentry->d_inode);
bend = ibend(dentry->d_inode);
- if (bstart >= 0) {
- struct inode *lower_inode;
- for (bindex = bstart; bindex <= bend;
- bindex++) {
- lower_inode =
- unionfs_lower_inode_idx(
- dentry->d_inode,
- bindex);
- iput(lower_inode);
- }
- }
+ __iput_lowers(dentry->d_inode, bstart, bend);
kfree(UNIONFS_I(dentry->d_inode)->lower_inodes);
UNIONFS_I(dentry->d_inode)->lower_inodes = NULL;
ibstart(dentry->d_inode) = -1;
--
1.5.2.2
When looking up a lower object in multiple branches, especially for
directories, ignore any existing entries whose type is different than the
type of the first found object (otherwise we'll be trying to, say, call
readdir on a non-dir inode).
Signed-off-by: Himanshu Kanda <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/lookup.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index b9ee072..755158e 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -256,6 +256,19 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
continue;
}
+ /*
+ * If we already found at least one positive dentry
+ * (dentry_count is non-zero), then we skip all remaining
+ * positive dentries if their type is a non-dir. This is
+ * because only directories are allowed to stack on multiple
+ * branches, but we have to skip non-dirs (to avoid, say,
+ * calling readdir on a regular file).
+ */
+ if (!S_ISDIR(lower_dentry->d_inode->i_mode) && dentry_count) {
+ dput(lower_dentry);
+ continue;
+ }
+
/* number of positive dentries */
dentry_count++;
--
1.5.2.2
This reduces text size by about 6k.
Cc: Hugh Dickins <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/fanout.h | 50 --------------------------------------------------
fs/unionfs/subr.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
fs/unionfs/union.h | 2 ++
3 files changed, 52 insertions(+), 50 deletions(-)
diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 4d9a45f..29d42fb 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -313,54 +313,4 @@ static inline void verify_locked(struct dentry *d)
BUG_ON(!mutex_is_locked(&UNIONFS_D(d)->lock));
}
-/* copy a/m/ctime from the lower branch with the newest times */
-static inline void unionfs_copy_attr_times(struct inode *upper)
-{
- int bindex;
- struct inode *lower;
-
- if (!upper || ibstart(upper) < 0)
- return;
- for (bindex = ibstart(upper); bindex <= ibend(upper); bindex++) {
- lower = unionfs_lower_inode_idx(upper, bindex);
- if (!lower)
- continue; /* not all lower dir objects may exist */
- if (unlikely(timespec_compare(&upper->i_mtime,
- &lower->i_mtime) < 0))
- upper->i_mtime = lower->i_mtime;
- if (unlikely(timespec_compare(&upper->i_ctime,
- &lower->i_ctime) < 0))
- upper->i_ctime = lower->i_ctime;
- if (unlikely(timespec_compare(&upper->i_atime,
- &lower->i_atime) < 0))
- upper->i_atime = lower->i_atime;
- }
-}
-
-/*
- * A unionfs/fanout version of fsstack_copy_attr_all. Uses a
- * unionfs_get_nlinks to properly calcluate the number of links to a file.
- * Also, copies the max() of all a/m/ctimes for all lower inodes (which is
- * important if the lower inode is a directory type)
- */
-static inline void unionfs_copy_attr_all(struct inode *dest,
- const struct inode *src)
-{
- dest->i_mode = src->i_mode;
- dest->i_uid = src->i_uid;
- dest->i_gid = src->i_gid;
- dest->i_rdev = src->i_rdev;
-
- unionfs_copy_attr_times(dest);
-
- dest->i_blkbits = src->i_blkbits;
- dest->i_flags = src->i_flags;
-
- /*
- * Update the nlinks AFTER updating the above fields, because the
- * get_links callback may depend on them.
- */
- dest->i_nlink = unionfs_get_nlinks(dest);
-}
-
#endif /* not _FANOUT_H */
diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 0a0fce9..68a6280 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -240,3 +240,53 @@ char *alloc_whname(const char *name, int len)
return buf;
}
+
+/* copy a/m/ctime from the lower branch with the newest times */
+void unionfs_copy_attr_times(struct inode *upper)
+{
+ int bindex;
+ struct inode *lower;
+
+ if (!upper || ibstart(upper) < 0)
+ return;
+ for (bindex = ibstart(upper); bindex <= ibend(upper); bindex++) {
+ lower = unionfs_lower_inode_idx(upper, bindex);
+ if (!lower)
+ continue; /* not all lower dir objects may exist */
+ if (unlikely(timespec_compare(&upper->i_mtime,
+ &lower->i_mtime) < 0))
+ upper->i_mtime = lower->i_mtime;
+ if (unlikely(timespec_compare(&upper->i_ctime,
+ &lower->i_ctime) < 0))
+ upper->i_ctime = lower->i_ctime;
+ if (unlikely(timespec_compare(&upper->i_atime,
+ &lower->i_atime) < 0))
+ upper->i_atime = lower->i_atime;
+ }
+}
+
+/*
+ * A unionfs/fanout version of fsstack_copy_attr_all. Uses a
+ * unionfs_get_nlinks to properly calcluate the number of links to a file.
+ * Also, copies the max() of all a/m/ctimes for all lower inodes (which is
+ * important if the lower inode is a directory type)
+ */
+void unionfs_copy_attr_all(struct inode *dest,
+ const struct inode *src)
+{
+ dest->i_mode = src->i_mode;
+ dest->i_uid = src->i_uid;
+ dest->i_gid = src->i_gid;
+ dest->i_rdev = src->i_rdev;
+
+ unionfs_copy_attr_times(dest);
+
+ dest->i_blkbits = src->i_blkbits;
+ dest->i_flags = src->i_flags;
+
+ /*
+ * Update the nlinks AFTER updating the above fields, because the
+ * get_links callback may depend on them.
+ */
+ dest->i_nlink = unionfs_get_nlinks(dest);
+}
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 786c4be..0e0ccde 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -199,6 +199,8 @@ struct unionfs_dir_state {
/* externs needed for fanout.h or sioq.h */
extern int unionfs_get_nlinks(const struct inode *inode);
+extern void unionfs_copy_attr_times(struct inode *upper);
+extern void unionfs_copy_attr_all(struct inode *dest, const struct inode *src);
/* include miscellaneous macros */
#include "fanout.h"
--
1.5.2.2
Remove unionfs's versions thereof.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/main.c | 10 +++++-----
fs/unionfs/super.c | 27 ++++++++++++++-------------
include/linux/namei.h | 12 ------------
3 files changed, 19 insertions(+), 30 deletions(-)
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 3585b29..8f59fb5 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -230,11 +230,11 @@ void unionfs_reinterpose(struct dentry *dentry)
int check_branch(struct nameidata *nd)
{
/* XXX: remove in ODF code -- stacking unions allowed there */
- if (!strcmp(nd->dentry->d_sb->s_type->name, UNIONFS_NAME))
+ if (!strcmp(nd->path.dentry->d_sb->s_type->name, UNIONFS_NAME))
return -EINVAL;
- if (!nd->dentry->d_inode)
+ if (!nd->path.dentry->d_inode)
return -ENOENT;
- if (!S_ISDIR(nd->dentry->d_inode->i_mode))
+ if (!S_ISDIR(nd->path.dentry->d_inode->i_mode))
return -ENOTDIR;
return 0;
}
@@ -375,8 +375,8 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
goto out;
}
- lower_root_info->lower_paths[bindex].dentry = nd.dentry;
- lower_root_info->lower_paths[bindex].mnt = nd.mnt;
+ lower_root_info->lower_paths[bindex].dentry = nd.path.dentry;
+ lower_root_info->lower_paths[bindex].mnt = nd.path.mnt;
set_branchperms(sb, bindex, perms);
set_branch_count(sb, bindex, 0);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 773623e..fba1598 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -231,8 +231,8 @@ static noinline int do_remount_mode_option(char *optarg, int cur_branches,
goto out;
}
for (idx = 0; idx < cur_branches; idx++)
- if (nd.mnt == new_lower_paths[idx].mnt &&
- nd.dentry == new_lower_paths[idx].dentry)
+ if (nd.path.mnt == new_lower_paths[idx].mnt &&
+ nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(&nd.path); /* no longer needed */
if (idx == cur_branches) {
@@ -274,8 +274,8 @@ static noinline int do_remount_del_option(char *optarg, int cur_branches,
goto out;
}
for (idx = 0; idx < cur_branches; idx++)
- if (nd.mnt == new_lower_paths[idx].mnt &&
- nd.dentry == new_lower_paths[idx].dentry)
+ if (nd.path.mnt == new_lower_paths[idx].mnt &&
+ nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(&nd.path); /* no longer needed */
if (idx == cur_branches) {
@@ -358,8 +358,8 @@ static noinline int do_remount_add_option(char *optarg, int cur_branches,
goto out;
}
for (idx = 0; idx < cur_branches; idx++)
- if (nd.mnt == new_lower_paths[idx].mnt &&
- nd.dentry == new_lower_paths[idx].dentry)
+ if (nd.path.mnt == new_lower_paths[idx].mnt &&
+ nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(&nd.path); /* no longer needed */
if (idx == cur_branches) {
@@ -425,10 +425,10 @@ found_insertion_point:
memmove(&new_lower_paths[idx+1], &new_lower_paths[idx],
(cur_branches - idx) * sizeof(struct path));
}
- new_lower_paths[idx].dentry = nd.dentry;
- new_lower_paths[idx].mnt = nd.mnt;
+ new_lower_paths[idx].dentry = nd.path.dentry;
+ new_lower_paths[idx].mnt = nd.path.mnt;
- new_data[idx].sb = nd.dentry->d_sb;
+ new_data[idx].sb = nd.path.dentry->d_sb;
atomic_set(&new_data[idx].open_files, 0);
new_data[idx].branchperms = perms;
new_data[idx].branch_id = ++*high_branch_id; /* assign new branch ID */
@@ -577,7 +577,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
memcpy(tmp_lower_paths, UNIONFS_D(sb->s_root)->lower_paths,
cur_branches * sizeof(struct path));
for (i = 0; i < cur_branches; i++)
- pathget(&tmp_lower_paths[i]); /* drop refs at end of fxn */
+ path_get(&tmp_lower_paths[i]); /* drop refs at end of fxn */
/*******************************************************************
* For each branch command, do path_lookup on the requested branch,
@@ -1008,9 +1008,10 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
seq_printf(m, ",dirs=");
for (bindex = bstart; bindex <= bend; bindex++) {
- path = d_path(unionfs_lower_dentry_idx(sb->s_root, bindex),
- unionfs_lower_mnt_idx(sb->s_root, bindex),
- tmp_page, PAGE_SIZE);
+ struct path p;
+ p.dentry = unionfs_lower_dentry_idx(sb->s_root, bindex);
+ p.mnt = unionfs_lower_mnt_idx(sb->s_root, bindex);
+ path = d_path(&p, tmp_page, PAGE_SIZE);
if (IS_ERR(path)) {
ret = PTR_ERR(path);
goto out;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 7ffeb21..24d88e9 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -93,16 +93,4 @@ static inline char *nd_get_link(struct nameidata *nd)
return nd->saved_names[nd->depth];
}
-static inline void pathget(struct path *path)
-{
- mntget(path->mnt);
- dget(path->dentry);
-}
-
-static inline void pathput(struct path *path)
-{
- dput(path->dentry);
- mntput(path->mnt);
-}
-
#endif /* _LINUX_NAMEI_H */
--
1.5.2.2
From: Jan Blunck <[email protected]>
* Add path_put() functions for releasing a reference to the dentry and
vfsmount of a struct path in the right order
* Switch from path_release(nd) to path_put(&nd->path)
* Rename dput_path() to path_put_conditional()
Signed-off-by: Jan Blunck <[email protected]>
Signed-off-by: Andreas Gruenbacher <[email protected]>
Acked-by: Christoph Hellwig <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/main.c | 2 +-
fs/unionfs/super.c | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 4bc2c66..3585b29 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -371,7 +371,7 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
if (err) {
printk(KERN_ERR "unionfs: lower directory "
"'%s' is not a valid branch\n", name);
- path_release(&nd);
+ path_put(&nd.path);
goto out;
}
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index b71fc2a..773623e 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -234,7 +234,7 @@ static noinline int do_remount_mode_option(char *optarg, int cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt &&
nd.dentry == new_lower_paths[idx].dentry)
break;
- path_release(&nd); /* no longer needed */
+ path_put(&nd.path); /* no longer needed */
if (idx == cur_branches) {
err = -ENOENT; /* err may have been reset above */
printk(KERN_ERR "unionfs: branch \"%s\" "
@@ -277,7 +277,7 @@ static noinline int do_remount_del_option(char *optarg, int cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt &&
nd.dentry == new_lower_paths[idx].dentry)
break;
- path_release(&nd); /* no longer needed */
+ path_put(&nd.path); /* no longer needed */
if (idx == cur_branches) {
printk(KERN_ERR "unionfs: branch \"%s\" "
"not found\n", optarg);
@@ -296,7 +296,7 @@ static noinline int do_remount_del_option(char *optarg, int cur_branches,
* new_data and new_lower_paths one to the left. Finally, adjust
* cur_branches.
*/
- pathput(&new_lower_paths[idx]);
+ path_put(&new_lower_paths[idx]);
if (idx < cur_branches - 1) {
/* if idx==cur_branches-1, we delete last branch: easy */
@@ -361,7 +361,7 @@ static noinline int do_remount_add_option(char *optarg, int cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt &&
nd.dentry == new_lower_paths[idx].dentry)
break;
- path_release(&nd); /* no longer needed */
+ path_put(&nd.path); /* no longer needed */
if (idx == cur_branches) {
printk(KERN_ERR "unionfs: branch \"%s\" "
"not found\n", optarg);
@@ -408,7 +408,7 @@ found_insertion_point:
if (err) {
printk(KERN_ERR "unionfs: lower directory "
"\"%s\" is not a valid branch\n", optarg);
- path_release(&nd);
+ path_put(&nd.path);
goto out;
}
@@ -818,7 +818,7 @@ out_release:
/* no need to cleanup/release anything in tmp_data */
if (tmp_lower_paths)
for (i = 0; i < new_branches; i++)
- pathput(&tmp_lower_paths[i]);
+ path_put(&tmp_lower_paths[i]);
out_free:
kfree(tmp_lower_paths);
kfree(tmp_data);
--
1.5.2.2
Remove unnecessary calls to update branch m/ctimes, and use them only when
needed. Update branch vfsmounts after operations that could cause a copyup.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/commonfops.c | 9 +++------
fs/unionfs/copyup.c | 3 ++-
fs/unionfs/dentry.c | 1 +
fs/unionfs/mmap.c | 17 ++++-------------
fs/unionfs/rename.c | 7 +++++--
5 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 491e2ff..2add167 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -604,6 +604,7 @@ out:
}
out_nofree:
if (!err) {
+ unionfs_postcopyup_setmnt(dentry);
unionfs_copy_attr_times(inode);
unionfs_check_file(file);
unionfs_check_inode(inode);
@@ -839,13 +840,9 @@ int unionfs_flush(struct file *file, fl_owner_t id)
}
- /* on success, update our times */
- unionfs_copy_attr_times(dentry->d_inode);
- /* parent time could have changed too (async) */
- unionfs_copy_attr_times(dentry->d_parent->d_inode);
-
out:
- unionfs_check_file(file);
+ if (!err)
+ unionfs_check_file(file);
unionfs_unlock_dentry(file->f_path.dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 9beac01..f71bddf 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -819,7 +819,8 @@ begin:
* update times of this dentry, but also the parent, because if
* we changed, the parent may have changed too.
*/
- unionfs_copy_attr_times(parent_dentry->d_inode);
+ fsstack_copy_attr_times(parent_dentry->d_inode,
+ lower_parent_dentry->d_inode);
unionfs_copy_attr_times(child_dentry->d_inode);
parent_dentry = child_dentry;
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 17b297d..a956b94 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -482,6 +482,7 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
err = __unionfs_d_revalidate_chain(dentry, nd, false);
if (likely(err > 0)) { /* true==1: dentry is valid */
+ unionfs_postcopyup_setmnt(dentry);
unionfs_check_dentry(dentry);
unionfs_check_nd(nd);
}
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index ad770ac..d6ac61e 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -227,20 +227,11 @@ static int unionfs_prepare_write(struct file *file, struct page *page,
int err;
unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
- /*
- * This is the only place where we unconditionally copy the lower
- * attribute times before calling unionfs_file_revalidate. The
- * reason is that our ->write calls do_sync_write which in turn will
- * call our ->prepare_write and then ->commit_write. Before our
- * ->write is called, the lower mtimes are in sync, but by the time
- * the VFS calls our ->commit_write, the lower mtimes have changed.
- * Therefore, the only reasonable time for us to sync up from the
- * changed lower mtimes, and avoid an invariant violation warning,
- * is here, in ->prepare_write.
- */
- unionfs_copy_attr_times(file->f_path.dentry->d_inode);
err = unionfs_file_revalidate(file, true);
- unionfs_check_file(file);
+ if (!err) {
+ unionfs_copy_attr_times(file->f_path.dentry->d_inode);
+ unionfs_check_file(file);
+ }
unionfs_read_unlock(file->f_path.dentry->d_sb);
return err;
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 5ab13f9..cc16eb2 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -138,6 +138,11 @@ static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
err = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry,
lower_new_dir_dentry->d_inode, lower_new_dentry);
out_err_unlock:
+ if (!err) {
+ /* update parent dir times */
+ fsstack_copy_attr_times(old_dir, lower_old_dir_dentry->d_inode);
+ fsstack_copy_attr_times(new_dir, lower_new_dir_dentry->d_inode);
+ }
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
lockdep_on();
@@ -526,8 +531,6 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
}
}
/* if all of this renaming succeeded, update our times */
- unionfs_copy_attr_times(old_dir);
- unionfs_copy_attr_times(new_dir);
unionfs_copy_attr_times(old_dentry->d_inode);
unionfs_copy_attr_times(new_dentry->d_inode);
unionfs_check_inode(old_dir);
--
1.5.2.2
To be used by rest of revalidation code, as well a callers who already
locked the child and parent dentry branch-configurations.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/dentry.c | 87 +++++++++++++++++++++++++++++++-------------------
fs/unionfs/union.h | 3 ++
2 files changed, 57 insertions(+), 33 deletions(-)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index afa2120..3bd2dfb 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -285,6 +285,59 @@ void purge_sb_data(struct super_block *sb)
}
/*
+ * Revalidate a single file/symlink/special dentry. Assume that info nodes
+ * of the dentry and its parent are locked. Assume that parent(s) are all
+ * valid already, but the child may not yet be valid. Returns true if
+ * valid, false otherwise.
+ */
+bool __unionfs_d_revalidate_one_locked(struct dentry *dentry,
+ struct nameidata *nd,
+ bool willwrite)
+{
+ bool valid = false; /* default is invalid */
+ int sbgen, dgen, bindex;
+
+ verify_locked(dentry);
+ verify_locked(dentry->d_parent);
+
+ sbgen = atomic_read(&UNIONFS_SB(dentry->d_sb)->generation);
+ dgen = atomic_read(&UNIONFS_D(dentry)->generation);
+
+ if (unlikely(is_newer_lower(dentry))) {
+ /* root dentry special case as aforementioned */
+ if (IS_ROOT(dentry)) {
+ unionfs_copy_attr_times(dentry->d_inode);
+ } else {
+ /*
+ * reset generation number to zero, guaranteed to be
+ * "old"
+ */
+ dgen = 0;
+ atomic_set(&UNIONFS_D(dentry)->generation, dgen);
+ }
+ if (!willwrite)
+ purge_inode_data(dentry->d_inode);
+ }
+ valid = __unionfs_d_revalidate_one(dentry, nd);
+
+ /*
+ * If __unionfs_d_revalidate_one() succeeded above, then it will
+ * have incremented the refcnt of the mnt's, but also the branch
+ * indices of the dentry will have been updated (to take into
+ * account any branch insertions/deletion. So the current
+ * dbstart/dbend match the current, and new, indices of the mnts
+ * which __unionfs_d_revalidate_one has incremented. Note: the "if"
+ * test below does not depend on whether chain_len was 0 or greater.
+ */
+ if (!valid || sbgen == dgen)
+ goto out;
+ for (bindex = dbstart(dentry); bindex <= dbend(dentry); bindex++)
+ unionfs_mntput(dentry, bindex);
+out:
+ return valid;
+}
+
+/*
* Revalidate a parent chain of dentries, then the actual node.
* Assumes that dentry is locked, but will lock all parents if/when needed.
*
@@ -404,42 +457,10 @@ out_this:
if (dentry != dentry->d_parent)
unionfs_lock_dentry(dentry->d_parent,
UNIONFS_DMUTEX_REVAL_PARENT);
- dgen = atomic_read(&UNIONFS_D(dentry)->generation);
-
- if (unlikely(is_newer_lower(dentry))) {
- /* root dentry special case as aforementioned */
- if (IS_ROOT(dentry)) {
- unionfs_copy_attr_times(dentry->d_inode);
- } else {
- /*
- * reset generation number to zero, guaranteed to be
- * "old"
- */
- dgen = 0;
- atomic_set(&UNIONFS_D(dentry)->generation, dgen);
- }
- if (!willwrite)
- purge_inode_data(dentry->d_inode);
- }
- valid = __unionfs_d_revalidate_one(dentry, nd);
+ valid = __unionfs_d_revalidate_one_locked(dentry, nd, willwrite);
if (dentry != dentry->d_parent)
unionfs_unlock_dentry(dentry->d_parent);
- /*
- * If __unionfs_d_revalidate_one() succeeded above, then it will
- * have incremented the refcnt of the mnt's, but also the branch
- * indices of the dentry will have been updated (to take into
- * account any branch insertions/deletion. So the current
- * dbstart/dbend match the current, and new, indices of the mnts
- * which __unionfs_d_revalidate_one has incremented. Note: the "if"
- * test below does not depend on whether chain_len was 0 or greater.
- */
- if (valid && sbgen != dgen)
- for (bindex = dbstart(dentry);
- bindex <= dbend(dentry);
- bindex++)
- unionfs_mntput(dentry, bindex);
-
out_free:
/* unlock/dput all dentries in chain and return status */
if (chain_len > 0) {
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 0e0ccde..5001b07 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -368,6 +368,9 @@ extern int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
extern int unionfs_unlink(struct inode *dir, struct dentry *dentry);
extern int unionfs_rmdir(struct inode *dir, struct dentry *dentry);
+extern bool __unionfs_d_revalidate_one_locked(struct dentry *dentry,
+ struct nameidata *nd,
+ bool willwrite);
extern bool __unionfs_d_revalidate_chain(struct dentry *dentry,
struct nameidata *nd, bool willwrite);
extern bool is_newer_lower(const struct dentry *dentry);
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/subr.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 68a6280..1a40f63 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -247,8 +247,14 @@ void unionfs_copy_attr_times(struct inode *upper)
int bindex;
struct inode *lower;
- if (!upper || ibstart(upper) < 0)
+ if (!upper)
return;
+ if (ibstart(upper) < 0) {
+#ifdef CONFIG_UNION_FS_DEBUG
+ WARN_ON(ibstart(upper) < 0);
+#endif /* CONFIG_UNION_FS_DEBUG */
+ return;
+ }
for (bindex = ibstart(upper); bindex <= ibend(upper); bindex++) {
lower = unionfs_lower_inode_idx(upper, bindex);
if (!lower)
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/dentry.c | 13 ++++---------
1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index a956b94..f8f65e1 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -410,15 +410,10 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
goto out;
}
- /*
- * lock all dentries in chain, in child to parent order.
- * if failed, then sleep for a little, then retry.
- */
- dtmp = dentry->d_parent;
- for (i = chain_len-1; i >= 0; i--) {
- chain[i] = dget(dtmp);
- dtmp = dtmp->d_parent;
- }
+ /* grab all dentries in chain, in child to parent order */
+ dtmp = dentry;
+ for (i = chain_len-1; i >= 0; i--)
+ dtmp = chain[i] = dget_parent(dtmp);
/*
* call __unionfs_d_revalidate_one() on each dentry, but in parent
--
1.5.2.2
This is not strictly necessary, but it helps quiet a gcc-4.2 warning (a good
optimizer may optimize this initialization away).
Signed-off-by: Erez Zadok <[email protected]>
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/inode.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 0b92da2..8d939dc 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -246,7 +246,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
struct dentry *dentry,
struct nameidata *nd)
{
- struct path path_save;
+ struct path path_save = {NULL, NULL};
struct dentry *ret;
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
--
1.5.2.2
Dentry branch configuration "info node" lock should extend to calls to
copy_attr_times.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/commonfops.c | 14 ++++----------
1 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index f37192f..96473c4 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -521,11 +521,12 @@ int unionfs_open(struct inode *inode, struct file *file)
{
int err = 0;
struct file *lower_file = NULL;
- struct dentry *dentry = NULL;
+ struct dentry *dentry = file->f_path.dentry;
int bindex = 0, bstart = 0, bend = 0;
int size;
unionfs_read_lock(inode->i_sb, UNIONFS_SMUTEX_PARENT);
+ unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
file->private_data =
kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
@@ -551,9 +552,6 @@ int unionfs_open(struct inode *inode, struct file *file)
goto out;
}
- dentry = file->f_path.dentry;
- unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
-
bstart = fbstart(file) = dbstart(dentry);
bend = fbend(file) = dbend(dentry);
@@ -573,15 +571,12 @@ int unionfs_open(struct inode *inode, struct file *file)
if (!lower_file)
continue;
- branchput(file->f_path.dentry->d_sb, bindex);
+ branchput(dentry->d_sb, bindex);
/* fput calls dput for lower_dentry */
fput(lower_file);
}
}
- /* XXX: should this unlock be moved to the function bottom? */
- unionfs_unlock_dentry(dentry);
-
out:
if (err) {
kfree(UNIONFS_F(file)->lower_files);
@@ -590,12 +585,11 @@ out:
}
out_nofree:
if (!err) {
- dentry = file->f_path.dentry;
- unionfs_copy_attr_times(dentry->d_parent->d_inode);
unionfs_copy_attr_times(inode);
unionfs_check_file(file);
unionfs_check_inode(inode);
}
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(inode->i_sb);
return err;
}
--
1.5.2.2
From: Andrew Morton <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/inode.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 2e791fd..640969d 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -254,8 +254,8 @@ static struct dentry *unionfs_lookup(struct inode *parent,
/* save the dentry & vfsmnt from namei */
if (nd) {
- path_save.dentry = nd->dentry;
- path_save.mnt = nd->mnt;
+ path_save.dentry = nd->path.dentry;
+ path_save.mnt = nd->path.mnt;
}
/*
@@ -266,8 +266,8 @@ static struct dentry *unionfs_lookup(struct inode *parent,
/* restore the dentry & vfsmnt in namei */
if (nd) {
- nd->dentry = path_save.dentry;
- nd->mnt = path_save.mnt;
+ nd->path.dentry = path_save.dentry;
+ nd->path.mnt = path_save.mnt;
}
if (!IS_ERR(ret)) {
if (ret)
@@ -885,7 +885,7 @@ static int unionfs_permission(struct inode *inode, int mask,
const int write_mask = (mask & MAY_WRITE) && !(mask & MAY_READ);
if (nd)
- unionfs_lock_dentry(nd->dentry, UNIONFS_DMUTEX_CHILD);
+ unionfs_lock_dentry(nd->path.dentry, UNIONFS_DMUTEX_CHILD);
if (!UNIONFS_I(inode)->lower_inodes) {
if (is_file) /* dirs can be unlinked but chdir'ed to */
@@ -960,7 +960,7 @@ out:
unionfs_check_inode(inode);
unionfs_check_nd(nd);
if (nd)
- unionfs_unlock_dentry(nd->dentry);
+ unionfs_unlock_dentry(nd->path.dentry);
return err;
}
--
1.5.2.2
Ensure that we lock the branch configuration of parent and child dentries in
operations which need it, and in the right order.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/commonfops.c | 31 +++++++++++++++++++++---
fs/unionfs/dentry.c | 26 +++++++++-----------
fs/unionfs/inode.c | 58 +++++++++++++++++++++++++++++++---------------
fs/unionfs/union.h | 5 ++-
fs/unionfs/unlink.c | 11 ++++++++-
5 files changed, 91 insertions(+), 40 deletions(-)
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 96473c4..491e2ff 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -300,8 +300,9 @@ out:
* Revalidate the struct file
* @file: file to revalidate
* @willwrite: true if caller may cause changes to the file; false otherwise.
+ * Caller must lock/unlock dentry's branch configuration.
*/
-int unionfs_file_revalidate(struct file *file, bool willwrite)
+int unionfs_file_revalidate_locked(struct file *file, bool willwrite)
{
struct super_block *sb;
struct dentry *dentry;
@@ -311,7 +312,6 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
int err = 0;
dentry = file->f_path.dentry;
- unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
sb = dentry->d_sb;
/*
@@ -416,7 +416,17 @@ out:
out_nofree:
if (!err)
unionfs_check_file(file);
- unionfs_unlock_dentry(dentry);
+ return err;
+}
+
+int unionfs_file_revalidate(struct file *file, bool willwrite)
+{
+ int err;
+
+ unionfs_lock_dentry(file->f_path.dentry, UNIONFS_DMUTEX_CHILD);
+ err = unionfs_file_revalidate_locked(file, willwrite);
+ unionfs_unlock_dentry(file->f_path.dentry);
+
return err;
}
@@ -524,9 +534,18 @@ int unionfs_open(struct inode *inode, struct file *file)
struct dentry *dentry = file->f_path.dentry;
int bindex = 0, bstart = 0, bend = 0;
int size;
+ int valid = 0;
unionfs_read_lock(inode->i_sb, UNIONFS_SMUTEX_PARENT);
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+ if (dentry != dentry->d_parent)
+ unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+
+ valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false);
+ if (unlikely(!valid)) {
+ err = -ESTALE;
+ goto out_nofree;
+ }
file->private_data =
kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
@@ -589,6 +608,8 @@ out_nofree:
unionfs_check_file(file);
unionfs_check_inode(inode);
}
+ if (dentry != dentry->d_parent)
+ unionfs_unlock_dentry(dentry->d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(inode->i_sb);
return err;
@@ -797,8 +818,9 @@ int unionfs_flush(struct file *file, fl_owner_t id)
int bindex, bstart, bend;
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+ unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
- err = unionfs_file_revalidate(file, true);
+ err = unionfs_file_revalidate_locked(file, true);
if (unlikely(err))
goto out;
unionfs_check_file(file);
@@ -824,6 +846,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
out:
unionfs_check_file(file);
+ unionfs_unlock_dentry(file->f_path.dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
}
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 3bd2dfb..17b297d 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -363,6 +363,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
chain_len = 0;
sbgen = atomic_read(&UNIONFS_SB(dentry->d_sb)->generation);
dtmp = dentry->d_parent;
+ verify_locked(dentry);
if (dentry != dtmp)
unionfs_lock_dentry(dtmp, UNIONFS_DMUTEX_REVAL_PARENT);
dgen = atomic_read(&UNIONFS_D(dtmp)->generation);
@@ -453,7 +454,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
out_this:
/* finally, lock this dentry and revalidate it */
- verify_locked(dentry);
+ verify_locked(dentry); /* verify child is locked */
if (dentry != dentry->d_parent)
unionfs_lock_dentry(dentry->d_parent,
UNIONFS_DMUTEX_REVAL_PARENT);
@@ -491,24 +492,20 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
return err;
}
-/*
- * At this point no one can reference this dentry, so we don't have to be
- * careful about concurrent access.
- */
static void unionfs_d_release(struct dentry *dentry)
{
int bindex, bstart, bend;
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+ /* must lock our branch configuration here */
+ unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
unionfs_check_dentry(dentry);
/* this could be a negative dentry, so check first */
- if (unlikely(!UNIONFS_D(dentry))) {
- printk(KERN_ERR "unionfs: dentry without private data: %.*s\n",
- dentry->d_name.len, dentry->d_name.name);
- goto out;
- } else if (dbstart(dentry) < 0)
- goto out_free; /* due to a (normal) failed lookup */
+ if (unlikely(!UNIONFS_D(dentry) || dbstart(dentry) < 0)) {
+ unionfs_unlock_dentry(dentry);
+ goto out; /* due to a (normal) failed lookup */
+ }
/* Release all the lower dentries */
bstart = dbstart(dentry);
@@ -526,11 +523,10 @@ static void unionfs_d_release(struct dentry *dentry)
kfree(UNIONFS_D(dentry)->lower_paths);
UNIONFS_D(dentry)->lower_paths = NULL;
-out_free:
- /* No need to unlock it, because it is disappeared. */
- free_dentry_private_data(dentry);
+ unionfs_unlock_dentry(dentry);
out:
+ free_dentry_private_data(dentry);
unionfs_read_unlock(dentry->d_sb);
return;
}
@@ -545,6 +541,7 @@ static void unionfs_d_iput(struct dentry *dentry, struct inode *inode)
BUG_ON(!dentry);
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+ unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
if (!UNIONFS_D(dentry) || dbstart(dentry) < 0)
goto drop_lower_inodes;
@@ -574,6 +571,7 @@ drop_lower_inodes:
iput(inode);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
}
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 6377533..2e791fd 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -175,16 +175,16 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
struct nameidata lower_nd;
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+ unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+
valid = __unionfs_d_revalidate_chain(dentry->d_parent, nd, false);
- unionfs_unlock_dentry(dentry->d_parent);
if (unlikely(!valid)) {
err = -ESTALE; /* same as what real_lookup does */
goto out;
}
- unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
- valid = __unionfs_d_revalidate_chain(dentry, nd, false);
+ valid = __unionfs_d_revalidate_one_locked(dentry, nd, false);
/*
* It's only a bug if this dentry was not negative and couldn't be
* revalidated (shouldn't happen).
@@ -224,14 +224,13 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
unlock_dir(lower_parent_dentry);
out:
- if (!err)
- unionfs_postcopyup_setmnt(dentry);
-
- unionfs_check_inode(parent);
if (!err) {
+ unionfs_postcopyup_setmnt(dentry);
+ unionfs_check_inode(parent);
unionfs_check_dentry(dentry);
unionfs_check_nd(nd);
}
+ unionfs_unlock_dentry(dentry->d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
@@ -251,7 +250,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
if (dentry != dentry->d_parent)
- unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+ unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_ROOT);
/* save the dentry & vfsmnt from namei */
if (nd) {
@@ -273,6 +272,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
if (!IS_ERR(ret)) {
if (ret)
dentry = ret;
+ unionfs_copy_attr_times(dentry->d_inode);
/* parent times may have changed */
unionfs_copy_attr_times(dentry->d_parent->d_inode);
}
@@ -469,9 +469,15 @@ static int unionfs_symlink(struct inode *parent, struct dentry *dentry,
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+ unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+ valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false);
+ if (unlikely(!valid)) {
+ err = -ESTALE;
+ goto out;
+ }
if (unlikely(dentry->d_inode &&
- !__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+ !__unionfs_d_revalidate_one_locked(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -514,12 +520,12 @@ out:
dput(wh_dentry);
kfree(name);
- if (!err)
+ if (!err) {
unionfs_postcopyup_setmnt(dentry);
-
- unionfs_check_inode(parent);
- if (!err)
+ unionfs_check_inode(parent);
unionfs_check_dentry(dentry);
+ }
+ unionfs_unlock_dentry(dentry->d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
@@ -534,12 +540,19 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
char *name = NULL;
int whiteout_unlinked = 0;
struct sioq_args args;
+ int valid;
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+ unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+ valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false);
+ if (unlikely(!valid)) {
+ err = -ESTALE; /* same as what real_lookup does */
+ goto out;
+ }
if (unlikely(dentry->d_inode &&
- !__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+ !__unionfs_d_revalidate_one_locked(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -673,6 +686,7 @@ out:
}
unionfs_check_inode(parent);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry->d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
@@ -691,9 +705,15 @@ static int unionfs_mknod(struct inode *parent, struct dentry *dentry, int mode,
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+ unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+ valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false);
+ if (unlikely(!valid)) {
+ err = -ESTALE;
+ goto out;
+ }
if (unlikely(dentry->d_inode &&
- !__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+ !__unionfs_d_revalidate_one_locked(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -734,12 +754,12 @@ out:
dput(wh_dentry);
kfree(name);
- if (!err)
+ if (!err) {
unionfs_postcopyup_setmnt(dentry);
-
- unionfs_check_inode(parent);
- if (!err)
+ unionfs_check_inode(parent);
unionfs_check_dentry(dentry);
+ }
+ unionfs_unlock_dentry(dentry->d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 5001b07..1bf0c09 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -281,11 +281,11 @@ static inline void unionfs_double_lock_dentry(struct dentry *d1,
{
BUG_ON(d1 == d2);
if (d1 < d2) {
- unionfs_lock_dentry(d1, UNIONFS_DMUTEX_PARENT);
unionfs_lock_dentry(d2, UNIONFS_DMUTEX_CHILD);
+ unionfs_lock_dentry(d1, UNIONFS_DMUTEX_PARENT);
} else {
- unionfs_lock_dentry(d2, UNIONFS_DMUTEX_PARENT);
unionfs_lock_dentry(d1, UNIONFS_DMUTEX_CHILD);
+ unionfs_lock_dentry(d2, UNIONFS_DMUTEX_PARENT);
}
}
@@ -352,6 +352,7 @@ extern int unionfs_setlk(struct file *file, int cmd, struct file_lock *fl);
extern int unionfs_getlk(struct file *file, struct file_lock *fl);
/* Common file operations. */
+extern int unionfs_file_revalidate_locked(struct file *file, bool willwrite);
extern int unionfs_file_revalidate(struct file *file, bool willwrite);
extern int unionfs_open(struct inode *inode, struct file *file);
extern int unionfs_file_release(struct inode *inode, struct file *file);
diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index 1e370a1..6e93da3 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -92,12 +92,20 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
{
int err = 0;
struct inode *inode = dentry->d_inode;
+ int valid;
BUG_ON(S_ISDIR(inode->i_mode));
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+ unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
- if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+ valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false);
+ if (unlikely(!valid)) {
+ err = -ESTALE;
+ goto out;
+ }
+ valid = __unionfs_d_revalidate_one_locked(dentry, NULL, false);
+ if (unlikely(!valid)) {
err = -ESTALE;
goto out;
}
@@ -126,6 +134,7 @@ out:
unionfs_check_dentry(dentry);
unionfs_check_inode(dir);
}
+ unionfs_unlock_dentry(dentry->d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
--
1.5.2.2
From: David Howells <[email protected]>
Replace unionfs_read_inode() with unionfs_iget(), and call that instead of
iget(). unionfs_iget() then uses iget_locked() directly and returns a
proper error code instead of an inode in the event of an error.
unionfs_fill_super() returns any error incurred when getting the root inode
instead of EINVAL.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/main.c | 11 +++++------
fs/unionfs/super.c | 19 ++++++++++++++-----
fs/unionfs/union.h | 1 +
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index ba3471d..4bc2c66 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -104,9 +104,8 @@ struct dentry *unionfs_interpose(struct dentry *dentry, struct super_block *sb,
BUG_ON(is_negative_dentry);
/*
- * We allocate our new inode below, by calling iget.
- * iget will call our read_inode which will initialize some
- * of the new inode's fields
+ * We allocate our new inode below by calling unionfs_iget,
+ * which will initialize some of the new inode's fields
*/
/*
@@ -128,9 +127,9 @@ struct dentry *unionfs_interpose(struct dentry *dentry, struct super_block *sb,
}
} else {
/* get unique inode number for unionfs */
- inode = iget(sb, iunique(sb, UNIONFS_ROOT_INO));
- if (!inode) {
- err = -EACCES;
+ inode = unionfs_iget(sb, iunique(sb, UNIONFS_ROOT_INO));
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
goto out;
}
if (atomic_read(&inode->i_count) > 1)
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 175840f..b71fc2a 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -24,11 +24,19 @@
*/
static struct kmem_cache *unionfs_inode_cachep;
-static void unionfs_read_inode(struct inode *inode)
+struct inode *unionfs_iget(struct super_block *sb, unsigned long ino)
{
int size;
- struct unionfs_inode_info *info = UNIONFS_I(inode);
+ struct unionfs_inode_info *info;
+ struct inode *inode;
+ inode = iget_locked(sb, ino);
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+ if (!(inode->i_state & I_NEW))
+ return inode;
+
+ info = UNIONFS_I(inode);
memset(info, 0, offsetof(struct unionfs_inode_info, vfs_inode));
info->bstart = -1;
info->bend = -1;
@@ -44,7 +52,8 @@ static void unionfs_read_inode(struct inode *inode)
if (unlikely(!info->lower_inodes)) {
printk(KERN_CRIT "unionfs: no kernel memory when allocating "
"lower-pointer array!\n");
- BUG();
+ iget_failed(inode);
+ return ERR_PTR(-ENOMEM);
}
inode->i_version++;
@@ -60,7 +69,8 @@ static void unionfs_read_inode(struct inode *inode)
inode->i_atime.tv_sec = inode->i_atime.tv_nsec = 0;
inode->i_mtime.tv_sec = inode->i_mtime.tv_nsec = 0;
inode->i_ctime.tv_sec = inode->i_ctime.tv_nsec = 0;
-
+ unlock_new_inode(inode);
+ return inode;
}
/*
@@ -1025,7 +1035,6 @@ out:
}
struct super_operations unionfs_sops = {
- .read_inode = unionfs_read_inode,
.delete_inode = unionfs_delete_inode,
.put_super = unionfs_put_super,
.statfs = unionfs_statfs,
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 1bf0c09..533806c 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -364,6 +364,7 @@ extern int unionfs_fsync(struct file *file, struct dentry *dentry,
extern int unionfs_fasync(int fd, struct file *file, int flag);
/* Inode operations */
+extern struct inode *unionfs_iget(struct super_block *sb, unsigned long ino);
extern int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry);
extern int unionfs_unlink(struct inode *dir, struct dentry *dentry);
--
1.5.2.2