The following is a series of patches related to Unionfs. They represent a
few code cleanups (suggested or inspired by comments on the previous set of
patches), and a few more bug fixes (esp. to cache coherency). These fixes
were tested on our 2.6.23-rc6 latest code, as well as the backports to
2.6.{22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs, nfs, jffs2, ramfs, tmpfs,
cramfs, and squashfs (where available). See http://unionfs.filesystems.org/
to download backported unionfs code.
Please pull from the 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ezk/unionfs.git
^^^
(Note: the git URL to pull from had changed from "jsipek" to "ezk")
to receive the following:
Erez_Zadok (6):
Unionfs: use bool type in dentry and file revalidation code
Unionfs: remove unnecessary comment
Unionfs: add missing newlines to printks
Unionfs: check integrity only if validated dentry successfully
Unionfs: unionfs_lookup locking consistency
Unionfs: cache coherency after lower objects are removed
commonfops.c | 12 ++++++------
dentry.c | 9 +++++----
dirfops.c | 4 ++--
fanout.h | 1 -
file.c | 12 ++++++------
inode.c | 6 +++++-
lookup.c | 52 +++++++++++++++++++++++++++++++---------------------
mmap.c | 6 +++---
union.h | 2 +-
9 files changed, 59 insertions(+), 45 deletions(-)
Erez Zadok
[email protected]
Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef Sipek <[email protected]>
---
fs/unionfs/fanout.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index c5bf454..afeb9f6 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -320,7 +320,6 @@ static inline void unionfs_copy_attr_times(struct inode *upper)
upper->i_ctime = lower->i_ctime;
if (timespec_compare(&upper->i_atime, &lower->i_atime) < 0)
upper->i_atime = lower->i_atime;
- /* XXX: should we notify_change on our upper inode? */
}
}
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef Sipek <[email protected]>
---
fs/unionfs/dentry.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index f32922e..91f9780 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -438,13 +438,13 @@ static void unionfs_d_release(struct dentry *dentry)
unionfs_check_dentry(dentry);
/* this could be a negative dentry, so check first */
if (!UNIONFS_D(dentry)) {
- printk(KERN_DEBUG "unionfs: dentry without private data: %.*s",
+ printk(KERN_DEBUG "unionfs: dentry without private data: %.*s\n",
dentry->d_name.len, dentry->d_name.name);
goto out;
} else if (dbstart(dentry) < 0) {
/* this is due to a failed lookup */
printk(KERN_DEBUG "unionfs: dentry without lower "
- "dentries: %.*s",
+ "dentries: %.*s\n",
dentry->d_name.len, dentry->d_name.name);
goto out_free;
}
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef Sipek <[email protected]>
---
fs/unionfs/commonfops.c | 12 ++++++------
fs/unionfs/dentry.c | 2 +-
fs/unionfs/dirfops.c | 4 ++--
fs/unionfs/file.c | 12 ++++++------
fs/unionfs/mmap.c | 6 +++---
fs/unionfs/union.h | 2 +-
6 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 724128d..87cbb09 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -190,7 +190,7 @@ out:
}
/* open the highest priority file for a given upper file */
-static int open_highest_file(struct file *file, int willwrite)
+static int open_highest_file(struct file *file, bool willwrite)
{
int bindex, bstart, bend, err = 0;
struct file *lower_file;
@@ -298,9 +298,9 @@ out:
/*
* Revalidate the struct file
* @file: file to revalidate
- * @willwrite: 1 if caller may cause changes to the file; 0 otherwise.
+ * @willwrite: true if caller may cause changes to the file; false otherwise.
*/
-int unionfs_file_revalidate(struct file *file, int willwrite)
+int unionfs_file_revalidate(struct file *file, bool willwrite)
{
struct super_block *sb;
struct dentry *dentry;
@@ -612,7 +612,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
* This is important for open-but-unlinked files, as well as mmap
* support.
*/
- if ((err = unionfs_file_revalidate(file, 1)))
+ if ((err = unionfs_file_revalidate(file, true)))
goto out;
unionfs_check_file(file);
fileinfo = UNIONFS_F(file);
@@ -753,7 +753,7 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 1)))
+ if ((err = unionfs_file_revalidate(file, true)))
goto out;
/* check if asked for local commands */
@@ -791,7 +791,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
unionfs_read_lock(dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 1)))
+ if ((err = unionfs_file_revalidate(file, true)))
goto out;
unionfs_check_file(file);
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index e46a53e..f32922e 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -262,7 +262,7 @@ static inline void purge_inode_data(struct inode *inode)
* Revalidate a parent chain of dentries, then the actual node.
* Assumes that dentry is locked, but will lock all parents if/when needed.
*
- * If 'willwrite' is 1, and the lower inode times are not in sync, then
+ * If 'willwrite' is true, and the lower inode times are not in sync, then
* *don't* purge_inode_data, as it could deadlock if ->write calls us and we
* try to truncate a locked page. Besides, if unionfs is about to write
* data to a file, then there's the data unionfs is about to write is more
diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c
index 980f125..c923e58 100644
--- a/fs/unionfs/dirfops.c
+++ b/fs/unionfs/dirfops.c
@@ -99,7 +99,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 0)))
+ if ((err = unionfs_file_revalidate(file, false)))
goto out;
inode = file->f_path.dentry->d_inode;
@@ -201,7 +201,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 0)))
+ if ((err = unionfs_file_revalidate(file, false)))
goto out;
rdstate = UNIONFS_F(file)->rdstate;
diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index e9e63b7..d8eaaa5 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -24,7 +24,7 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
int err;
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 0)))
+ if ((err = unionfs_file_revalidate(file, false)))
goto out;
unionfs_check_file(file);
@@ -47,7 +47,7 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
struct file *file = iocb->ki_filp;
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 0)))
+ if ((err = unionfs_file_revalidate(file, false)))
goto out;
unionfs_check_file(file);
@@ -72,7 +72,7 @@ static ssize_t unionfs_write(struct file *file, const char __user *buf,
int err = 0;
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 1)))
+ if ((err = unionfs_file_revalidate(file, true)))
goto out;
unionfs_check_file(file);
@@ -97,7 +97,7 @@ static int unionfs_file_readdir(struct file *file, void *dirent,
static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
{
int err = 0;
- int willwrite;
+ bool willwrite;
struct file *lower_file;
unionfs_read_lock(file->f_path.dentry->d_sb);
@@ -149,7 +149,7 @@ int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
int err = -EINVAL;
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 1)))
+ if ((err = unionfs_file_revalidate(file, true)))
goto out;
unionfs_check_file(file);
@@ -196,7 +196,7 @@ int unionfs_fasync(int fd, struct file *file, int flag)
int err = 0;
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 1)))
+ if ((err = unionfs_file_revalidate(file, true)))
goto out;
unionfs_check_file(file);
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 6ef19af..88ef6a6 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -211,7 +211,7 @@ static int unionfs_readpage(struct file *file, struct page *page)
int err;
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 0)))
+ if ((err = unionfs_file_revalidate(file, false)))
goto out;
unionfs_check_file(file);
@@ -254,7 +254,7 @@ static int unionfs_prepare_write(struct file *file, struct page *page,
* is here, in ->prepare_write.
*/
unionfs_copy_attr_times(file->f_path.dentry->d_inode);
- err = unionfs_file_revalidate(file, 1);
+ err = unionfs_file_revalidate(file, true);
unionfs_check_file(file);
unionfs_read_unlock(file->f_path.dentry->d_sb);
@@ -275,7 +275,7 @@ static int unionfs_commit_write(struct file *file, struct page *page,
BUG_ON(file == NULL);
unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, 1)))
+ if ((err = unionfs_file_revalidate(file, true)))
goto out;
unionfs_check_file(file);
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 97da8f9..1cb2e1d 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -311,7 +311,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(struct file *file, int 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);
extern int unionfs_flush(struct file *file, fl_owner_t id);
--
1.5.2.2
Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef Sipek <[email protected]>
---
fs/unionfs/dentry.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 91f9780..9e0742d 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -418,7 +418,8 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
unionfs_lock_dentry(dentry);
err = __unionfs_d_revalidate_chain(dentry, nd, false);
unionfs_unlock_dentry(dentry);
- unionfs_check_dentry(dentry);
+ if (err > 0) /* true==1: dentry is valid */
+ unionfs_check_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
--
1.5.2.2
Ensure that our lookup locking is consistent and symmetric: if a lock
existed before calling lookup_backend, it should remain so; only if
performing a lookup of a known new dentry, should lookup_backend return a
newly-locked dentry-inode info (and only if there was no error). Document
this behavior. This cleanup allowed us to remove two unnecessary int
declarations.
Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef Sipek <[email protected]>
---
fs/unionfs/inode.c | 6 +++++-
fs/unionfs/lookup.c | 38 +++++++++++++++++++-------------------
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 687b9a7..9638b64 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -163,7 +163,10 @@ static struct dentry *unionfs_lookup(struct inode *parent,
path_save.mnt = nd->mnt;
}
- /* The locking is done by unionfs_lookup_backend. */
+ /*
+ * unionfs_lookup_backend returns a locked dentry upon success,
+ * so we'll have to unlock it below.
+ */
ret = unionfs_lookup_backend(dentry, nd, INTERPOSE_LOOKUP);
/* restore the dentry & vfsmnt in namei */
@@ -176,6 +179,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
dentry = ret;
/* parent times may have changed */
unionfs_copy_attr_times(dentry->d_parent->d_inode);
+ unionfs_unlock_dentry(dentry);
}
unionfs_check_inode(parent);
diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 7fa6310..8eb9749 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -77,6 +77,11 @@ out:
*
* Returns: NULL (ok), ERR_PTR if an error occurred, or a non-null non-error
* PTR if d_splice returned a different dentry.
+ *
+ * If lookupmode is INTERPOSE_PARTIAL/REVAL/REVAL_NEG, the passed dentry's
+ * inode info must be locked. If lookupmode is INTERPOSE_LOOKUP (i.e., a
+ * newly looked-up dentry), then unionfs_lookup_backend will return a locked
+ * dentry's info, which the caller must unlock.
*/
struct dentry *unionfs_lookup_backend(struct dentry *dentry,
struct nameidata *nd, int lookupmode)
@@ -94,8 +99,6 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
struct dentry *first_lower_dentry = NULL;
struct vfsmount *first_lower_mnt = NULL;
int locked_parent = 0;
- int locked_child = 0;
- int allocated_new_info = 0;
int opaque;
char *whname = NULL;
const char *name;
@@ -109,24 +112,21 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
if (lookupmode == INTERPOSE_PARTIAL || lookupmode == INTERPOSE_REVAL ||
lookupmode == INTERPOSE_REVAL_NEG)
verify_locked(dentry);
- else {
+ else /* this could only be INTERPOSE_LOOKUP */
BUG_ON(UNIONFS_D(dentry) != NULL);
- locked_child = 1;
- }
- switch(lookupmode) {
- case INTERPOSE_PARTIAL:
- break;
- case INTERPOSE_LOOKUP:
- if ((err = new_dentry_private_data(dentry)))
- goto out;
- allocated_new_info = 1;
- break;
- default:
- if ((err = realloc_dentry_private_data(dentry)))
- goto out;
- allocated_new_info = 1;
- break;
+ switch (lookupmode) {
+ case INTERPOSE_PARTIAL:
+ break;
+ case INTERPOSE_LOOKUP:
+ if ((err = new_dentry_private_data(dentry)))
+ goto out;
+ break;
+ default:
+ /* default: can only be INTERPOSE_REVAL/REVAL_NEG */
+ if ((err = realloc_dentry_private_data(dentry)))
+ goto out;
+ break;
}
/* must initialize dentry operations */
@@ -419,7 +419,7 @@ out:
if (locked_parent)
unionfs_unlock_dentry(parent_dentry);
dput(parent_dentry);
- if (locked_child || (err && allocated_new_info))
+ if (err && (lookupmode == INTERPOSE_LOOKUP))
unionfs_unlock_dentry(dentry);
if (!err && d_interposed)
return d_interposed;
--
1.5.2.2
Prevent an oops if a lower file is deleted and then it is stat'ed from the
upper layer. Ensure that we return a negative dentry so the user will get
an ENOENT. Properly dput/mntput so we don't leak references at the lower
file system.
Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef Sipek <[email protected]>
---
fs/unionfs/lookup.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 8eb9749..963d622 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -92,7 +92,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
struct dentry *lower_dir_dentry = NULL;
struct dentry *parent_dentry = NULL;
struct dentry *d_interposed = NULL;
- int bindex, bstart, bend, bopaque;
+ int bindex, bstart = -1, bend, bopaque;
int dentry_count = 0; /* Number of positive dentries. */
int first_dentry_offset = -1; /* -1 is uninitialized */
struct dentry *first_dentry = NULL;
@@ -413,7 +413,14 @@ out:
if (!err && UNIONFS_D(dentry)) {
BUG_ON(dbend(dentry) > UNIONFS_D(dentry)->bcount);
BUG_ON(dbend(dentry) > sbmax(dentry->d_sb));
- BUG_ON(dbstart(dentry) < 0);
+ if (dbstart(dentry) < 0 &&
+ dentry->d_inode && bstart >= 0 &&
+ (!UNIONFS_I(dentry->d_inode) ||
+ !UNIONFS_I(dentry->d_inode)->lower_inodes)) {
+ unionfs_mntput(dentry->d_sb->s_root, bstart);
+ dput(first_lower_dentry);
+ UNIONFS_I(dentry->d_inode)->stale = 1;
+ }
}
kfree(whname);
if (locked_parent)
@@ -423,6 +430,9 @@ out:
unionfs_unlock_dentry(dentry);
if (!err && d_interposed)
return d_interposed;
+ if (dentry->d_inode && UNIONFS_I(dentry->d_inode)->stale &&
+ first_dentry_offset >= 0)
+ unionfs_mntput(dentry->d_sb->s_root, first_dentry_offset);
return ERR_PTR(err);
}
--
1.5.2.2