The following is a series of patches related to Unionfs. The main changes
here are bug fixes and improved cache-coherency methods.
These patches were tested (where appropriate) on Linus's 2.6.24 latest code
(as of v2.6.24-rc2-330-g325d22d), for the first time on MM
(mmotm-2007-11-10-19-05), as well as the backports to
2.6.{23,22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs, nfs2/3/4, 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
to receive the following:
Erez Zadok (9):
Unionfs: flush and release updates
Unionfs: use i_size wrappers
Unionfs: update cache-coherency detection heuristics
Unionfs: writepage updates
Unionfs: clear partial read in readpage
Unionfs: debugging updates
Unionfs: remove unnecessary lower atime updates
Unionfs: fold do_readpage into unionfs_readpage
Unionfs: move debugging checks inside locks
Documentation/filesystems/unionfs/concepts.txt | 18 ++++++
fs/unionfs/commonfops.c | 47 +++++++-----------
fs/unionfs/debug.c | 2
fs/unionfs/dentry.c | 31 +++++++----
fs/unionfs/file.c | 18 +-----
fs/unionfs/inode.c | 29 +++++------
fs/unionfs/mmap.c | 65 ++++++++-----------------
fs/unionfs/rdstate.c | 4 -
fs/unionfs/rename.c | 4 -
fs/unionfs/super.c | 6 +-
fs/unionfs/union.h | 5 +
fs/unionfs/xattr.c | 8 +--
12 files changed, 115 insertions(+), 122 deletions(-)
---
Erez Zadok
[email protected]
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/commonfops.c | 10 +++++-----
fs/unionfs/inode.c | 4 ++--
fs/unionfs/rdstate.c | 4 ++--
fs/unionfs/rename.c | 4 ++--
fs/unionfs/super.c | 2 +-
5 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index ba84529..624f920 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -72,7 +72,8 @@ retry:
dput(tmp_dentry);
err = copyup_named_file(dentry->d_parent->d_inode, file, name, bstart,
- bindex, file->f_path.dentry->d_inode->i_size);
+ bindex,
+ i_size_read(file->f_path.dentry->d_inode));
if (err) {
if (unlikely(err == -EEXIST))
goto retry;
@@ -199,7 +200,6 @@ static int open_highest_file(struct file *file, bool willwrite)
struct dentry *dentry = file->f_path.dentry;
struct inode *parent_inode = dentry->d_parent->d_inode;
struct super_block *sb = dentry->d_sb;
- size_t inode_size = dentry->d_inode->i_size;
bstart = dbstart(dentry);
bend = dbend(dentry);
@@ -208,7 +208,7 @@ static int open_highest_file(struct file *file, bool willwrite)
if (willwrite && IS_WRITE_FLAG(file->f_flags) && is_robranch(dentry)) {
for (bindex = bstart - 1; bindex >= 0; bindex--) {
err = copyup_file(parent_inode, file, bstart, bindex,
- inode_size);
+ i_size_read(dentry->d_inode));
if (!err)
break;
}
@@ -243,7 +243,6 @@ static int do_delayed_copyup(struct file *file)
int bindex, bstart, bend, err = 0;
struct dentry *dentry = file->f_path.dentry;
struct inode *parent_inode = dentry->d_parent->d_inode;
- loff_t inode_size = dentry->d_inode->i_size;
bstart = fbstart(file);
bend = fbend(file);
@@ -255,7 +254,8 @@ static int do_delayed_copyup(struct file *file)
for (bindex = bstart - 1; bindex >= 0; bindex--) {
if (!d_deleted(dentry))
err = copyup_file(parent_inode, file, bstart,
- bindex, inode_size);
+ bindex,
+ i_size_read(dentry->d_inode));
else
err = copyup_deleted_file(file, dentry, bstart,
bindex);
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index bcefe45..223a64a 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -312,7 +312,7 @@ docopyup:
old_dentry, old_bstart,
bindex, old_dentry->d_name.name,
old_dentry->d_name.len, NULL,
- old_dentry->d_inode->i_size);
+ i_size_read(old_dentry->d_inode));
if (!err) {
lower_new_dentry =
create_parents(dir, new_dentry,
@@ -1043,7 +1043,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
continue;
/* Only if its the leftmost file, copyup the file */
for (i = bstart - 1; i >= 0; i--) {
- loff_t size = dentry->d_inode->i_size;
+ loff_t size = i_size_read(dentry->d_inode);
if (ia->ia_valid & ATTR_SIZE)
size = ia->ia_size;
err = copyup_dentry(dentry->d_parent->d_inode,
diff --git a/fs/unionfs/rdstate.c b/fs/unionfs/rdstate.c
index 0df5f52..bdf335d 100644
--- a/fs/unionfs/rdstate.c
+++ b/fs/unionfs/rdstate.c
@@ -76,10 +76,10 @@ static int guesstimate_hash_size(struct inode *inode)
if (!lower_inode)
continue;
- if (lower_inode->i_size == DENTPAGE)
+ if (i_size_read(lower_inode) == DENTPAGE)
hashsize += DENTPERONEPAGE;
else
- hashsize += (lower_inode->i_size / DENTPAGE) *
+ hashsize += (i_size_read(lower_inode) / DENTPAGE) *
DENTPERPAGE;
}
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 4a35b5e..452d1e7 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -234,8 +234,8 @@ static int do_unionfs_rename(struct inode *old_dir,
err = copyup_dentry(old_dentry->d_parent->d_inode,
old_dentry, old_bstart, bindex,
old_dentry->d_name.name,
- old_dentry->d_name.len,
- NULL, old_dentry->d_inode->i_size);
+ old_dentry->d_name.len, NULL,
+ i_size_read(old_dentry->d_inode));
/* if copyup failed, try next branch to the left */
if (err)
continue;
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 7d28045..70487bf 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -69,7 +69,7 @@ static void unionfs_read_inode(struct inode *inode)
*/
static void unionfs_delete_inode(struct inode *inode)
{
- inode->i_size = 0; /* every f/s seems to do that */
+ i_size_write(inode, 0); /* every f/s seems to do that */
if (inode->i_data.nrpages)
truncate_inode_pages(&inode->i_data, 0);
--
1.5.2.2
Remove the totalopens counter which was intended to reduce unnecessary
processing of d_deleted dentries. Move that processing from file_release to
flush.
Cc: Hugh Dickins <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/commonfops.c | 30 +++++++++++-------------------
fs/unionfs/union.h | 2 --
2 files changed, 11 insertions(+), 21 deletions(-)
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 50e5775..ba84529 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -551,9 +551,6 @@ int unionfs_open(struct inode *inode, struct file *file)
bstart = fbstart(file) = dbstart(dentry);
bend = fbend(file) = dbend(dentry);
- /* increment, so that we can flush appropriately */
- atomic_inc(&UNIONFS_I(dentry->d_inode)->totalopens);
-
/*
* open all directories and make the unionfs file struct point to
* these lower file structs
@@ -565,7 +562,6 @@ int unionfs_open(struct inode *inode, struct file *file)
/* freeing the allocated resources, and fput the opened files */
if (err) {
- atomic_dec(&UNIONFS_I(dentry->d_inode)->totalopens);
for (bindex = bstart; bindex <= bend; bindex++) {
lower_file = unionfs_lower_file_idx(file, bindex);
if (!lower_file)
@@ -606,6 +602,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
struct unionfs_file_info *fileinfo;
struct unionfs_inode_info *inodeinfo;
struct super_block *sb = inode->i_sb;
+ struct dentry *dentry = file->f_path.dentry;
int bindex, bstart, bend;
int fgen, err = 0;
@@ -628,6 +625,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
bstart = fbstart(file);
bend = fbend(file);
+ unionfs_lock_dentry(dentry);
for (bindex = bstart; bindex <= bend; bindex++) {
lower_file = unionfs_lower_file_idx(file, bindex);
@@ -635,7 +633,15 @@ int unionfs_file_release(struct inode *inode, struct file *file)
fput(lower_file);
branchput(sb, bindex);
}
+
+ /* if there are no more refs to the dentry, dput it */
+ if (d_deleted(dentry)) {
+ dput(unionfs_lower_dentry_idx(dentry, bindex));
+ unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
+ }
}
+ unionfs_unlock_dentry(dentry);
+
kfree(fileinfo->lower_files);
kfree(fileinfo->saved_branch_ids);
@@ -799,11 +805,6 @@ int unionfs_flush(struct file *file, fl_owner_t id)
goto out;
unionfs_check_file(file);
- if (!atomic_dec_and_test(&UNIONFS_I(dentry->d_inode)->totalopens))
- goto out;
-
- unionfs_lock_dentry(dentry);
-
bstart = fbstart(file);
bend = fbend(file);
for (bindex = bstart; bindex <= bend; bindex++) {
@@ -813,14 +814,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
lower_file->f_op->flush) {
err = lower_file->f_op->flush(lower_file, id);
if (err)
- goto out_lock;
-
- /* if there are no more refs to the dentry, dput it */
- if (d_deleted(dentry)) {
- dput(unionfs_lower_dentry_idx(dentry, bindex));
- unionfs_set_lower_dentry_idx(dentry, bindex,
- NULL);
- }
+ goto out;
}
}
@@ -830,8 +824,6 @@ int unionfs_flush(struct file *file, fl_owner_t id)
/* parent time could have changed too (async) */
unionfs_copy_attr_times(dentry->d_parent->d_inode);
-out_lock:
- unionfs_unlock_dentry(dentry);
out:
unionfs_read_unlock(dentry->d_sb);
unionfs_check_file(file);
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 0476f97..09b9ac9 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -102,8 +102,6 @@ struct unionfs_inode_info {
/* The lower inodes */
struct inode **lower_inodes;
- /* to keep track of reads/writes for unlinks before closes */
- atomic_t totalopens;
struct inode vfs_inode;
};
--
1.5.2.2
No need for this because our readpage calls vfs_read on the lower objects,
which would update the atime as/if needed.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/file.c | 8 --------
fs/unionfs/mmap.c | 6 ------
2 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index 126df5e..809e0f1 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -31,10 +31,6 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
err = do_sync_read(file, buf, count, ppos);
- if (err >= 0)
- touch_atime(unionfs_lower_mnt(file->f_path.dentry),
- unionfs_lower_dentry(file->f_path.dentry));
-
out:
unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
@@ -58,10 +54,6 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
if (err == -EIOCBQUEUED)
err = wait_on_sync_kiocb(iocb);
- if (err >= 0)
- touch_atime(unionfs_lower_mnt(file->f_path.dentry),
- unionfs_lower_dentry(file->f_path.dentry));
-
out:
unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index bb00fd5..34fd8aa 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -212,12 +212,6 @@ static int unionfs_readpage(struct file *file, struct page *page)
err = unionfs_do_readpage(file, page);
- if (!err) {
- touch_atime(unionfs_lower_mnt(file->f_path.dentry),
- unionfs_lower_dentry(file->f_path.dentry));
- unionfs_copy_attr_times(file->f_path.dentry->d_inode);
- }
-
/*
* we have to unlock our page, b/c we _might_ have gotten a locked
* page. but we no longer have to wakeup on our page here, b/c
--
1.5.2.2
Use a small delay to reduce the number of times unionfs has to detect
changed mtime's/ctime's, and also reduce the potential for false positives.
See Documentation/filesystems/unionfs/concepts.txt for a detailed
discussion.
Signed-off-by: Erez Zadok <[email protected]>
---
Documentation/filesystems/unionfs/concepts.txt | 18 ++++++++++++++
fs/unionfs/dentry.c | 29 ++++++++++++++---------
fs/unionfs/union.h | 3 ++
3 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/Documentation/filesystems/unionfs/concepts.txt b/Documentation/filesystems/unionfs/concepts.txt
index 37a62d8..7654ccc 100644
--- a/Documentation/filesystems/unionfs/concepts.txt
+++ b/Documentation/filesystems/unionfs/concepts.txt
@@ -177,5 +177,23 @@ would be really nice if the VFS would export an optional file system hook
->file_revalidate (similarly to dentry->d_revalidate) that will be called
before each VFS op that has a "struct file" in it.
+Certain file systems have micro-second granularity (or better) for inode
+times, and asynchronous actions could cause those times to change with some
+small delay. In such cases, Unionfs may see a changed inode time that only
+differs by a tiny fraction of a second: such a change may be a false
+positive indication that the lower object has changed, whereas if unionfs
+waits a little longer, that false indication will not be seen. (These false
+positives are harmless, because they would at most cause unionfs to
+re-validate an object that may need no revalidation, and print a debugging
+message that clutters the console/logs.) Therefore, to minimize the chances
+of these situations, we delay the detection of changed times by a small
+factor of a few seconds, called UNIONFS_MIN_CC_TIME (which defaults to 3
+seconds, as does NFS). This means that we will detect the change, only a
+couple of seconds later, if indeed the time change persists in the lower
+file object. This delayed detection has an added performance benefit: we
+reduce the number of times that unionfs has to revalidate objects, in case
+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).
For more information, see <http://unionfs.filesystems.org/>.
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index a3d7b6e..aed4d39 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -192,6 +192,16 @@ out:
/*
* Determine if the lower inode objects have changed from below the unionfs
* inode. Return true if changed, false otherwise.
+ *
+ * We check if the mtime or ctime have changed. However, the inode times
+ * can be changed by anyone without much protection, including
+ * asynchronously. This can sometimes cause unionfs to find that the lower
+ * file system doesn't change its inode times quick enough, resulting in a
+ * false positive indication (which is harmless, it just makes unionfs do
+ * extra work in re-validating the objects). To minimize the chances of
+ * these situations, we delay the detection of changed times by
+ * UNIONFS_MIN_CC_TIME (which defaults to 3 seconds, as with NFS's
+ * acregmin).
*/
bool is_newer_lower(const struct dentry *dentry)
{
@@ -211,26 +221,23 @@ bool is_newer_lower(const struct dentry *dentry)
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (!lower_inode)
continue;
- /*
- * We may want to apply other tests to determine if the
- * lower inode's data has changed, but checking for changed
- * ctime and mtime on the lower inode should be enough.
- */
- if (unlikely(timespec_compare(&inode->i_mtime,
- &lower_inode->i_mtime) < 0)) {
+
+ /* check if mtime/ctime have changed */
+ if (unlikely((lower_inode->i_mtime.tv_sec -
+ inode->i_mtime.tv_sec) > UNIONFS_MIN_CC_TIME)) {
pr_info("unionfs: new lower inode mtime "
"(bindex=%d, name=%s)\n", bindex,
dentry->d_name.name);
show_dinode_times(dentry);
- return true; /* mtime changed! */
+ return true;
}
- if (unlikely(timespec_compare(&inode->i_ctime,
- &lower_inode->i_ctime) < 0)) {
+ if (unlikely((lower_inode->i_ctime.tv_sec -
+ inode->i_ctime.tv_sec) > UNIONFS_MIN_CC_TIME)) {
pr_info("unionfs: new lower inode ctime "
"(bindex=%d, name=%s)\n", bindex,
dentry->d_name.name);
show_dinode_times(dentry);
- return true; /* ctime changed! */
+ return true;
}
}
return false; /* default: lower is not newer */
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 09b9ac9..f61e32d 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -63,6 +63,9 @@
/* maximum number of branches we support, to avoid memory blowup */
#define UNIONFS_MAX_BRANCHES 128
+/* minimum time (seconds) required for time-based cache-coherency */
+#define UNIONFS_MIN_CC_TIME 3
+
/* Operations vectors defined in specific files. */
extern struct file_operations unionfs_main_fops;
extern struct file_operations unionfs_dir_fops;
--
1.5.2.2
Simplify the code and reduce stack pressure a bit.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/mmap.c | 44 ++++++++++++++------------------------------
1 files changed, 14 insertions(+), 30 deletions(-)
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 34fd8aa..ef8822f 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -136,22 +136,22 @@ out:
return err;
}
-/*
- * readpage is called from generic_page_read and the fault handler.
- * If your file system uses generic_page_read for the read op, it
- * must implement readpage.
- *
- * Readpage expects a locked page, and must unlock it.
- */
-static int unionfs_do_readpage(struct file *file, struct page *page)
+/* Readpage expects a locked page, and must unlock it */
+static int unionfs_readpage(struct file *file, struct page *page)
{
- int err = -EIO;
+ int err;
struct file *lower_file;
struct inode *inode;
mm_segment_t old_fs;
char *page_data = NULL;
loff_t offset;
+ unionfs_read_lock(file->f_path.dentry->d_sb);
+ err = unionfs_file_revalidate(file, false);
+ if (unlikely(err))
+ goto out;
+ unionfs_check_file(file);
+
if (!UNIONFS_F(file)) {
err = -ENOENT;
goto out;
@@ -191,33 +191,17 @@ static int unionfs_do_readpage(struct file *file, struct page *page)
flush_dcache_page(page);
-out:
- if (err == 0)
- SetPageUptodate(page);
- else
- ClearPageUptodate(page);
-
- return err;
-}
-
-static int unionfs_readpage(struct file *file, struct page *page)
-{
- int err;
-
- unionfs_read_lock(file->f_path.dentry->d_sb);
- err = unionfs_file_revalidate(file, false);
- if (unlikely(err))
- goto out;
- unionfs_check_file(file);
-
- err = unionfs_do_readpage(file, page);
-
/*
* we have to unlock our page, b/c we _might_ have gotten a locked
* page. but we no longer have to wakeup on our page here, b/c
* UnlockPage does it
*/
out:
+ if (err == 0)
+ SetPageUptodate(page);
+ else
+ ClearPageUptodate(page);
+
unlock_page(page);
unionfs_check_file(file);
unionfs_read_unlock(file->f_path.dentry->d_sb);
--
1.5.2.2
Don't perform dentry+inode checks unless both are valid.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/debug.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
index 0066ccd..8464fbb 100644
--- a/fs/unionfs/debug.c
+++ b/fs/unionfs/debug.c
@@ -299,7 +299,7 @@ check_inode:
* be NULL. But, check that all three are NULL: lower dentry, mnt,
* and inode.
*/
- if (S_ISDIR(inode->i_mode))
+ if (dstart >= 0 && dend >= 0 && S_ISDIR(inode->i_mode))
for (bindex = dstart+1; bindex < dend; bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
lower_dentry = unionfs_lower_dentry_idx(dentry,
--
1.5.2.2
This is to ensure that the objects we want to check aren't being destroyed
or changed by another thread.
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/commonfops.c | 7 ++++---
fs/unionfs/dentry.c | 2 +-
fs/unionfs/file.c | 10 +++++-----
fs/unionfs/inode.c | 25 +++++++++++++------------
fs/unionfs/mmap.c | 2 +-
fs/unionfs/super.c | 4 ++--
fs/unionfs/xattr.c | 8 ++++----
7 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 624f920..b33917e 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -573,6 +573,7 @@ int unionfs_open(struct inode *inode, struct file *file)
}
}
+ /* XXX: should this unlock be moved to the function bottom? */
unionfs_unlock_dentry(dentry);
out:
@@ -582,12 +583,12 @@ out:
kfree(UNIONFS_F(file));
}
out_nofree:
- unionfs_read_unlock(inode->i_sb);
unionfs_check_inode(inode);
if (!err) {
unionfs_check_file(file);
unionfs_check_dentry(file->f_path.dentry->d_parent);
}
+ unionfs_read_unlock(inode->i_sb);
return err;
}
@@ -786,8 +787,8 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
}
out:
- unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
+ unionfs_read_unlock(file->f_path.dentry->d_sb);
return err;
}
@@ -825,7 +826,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
unionfs_copy_attr_times(dentry->d_parent->d_inode);
out:
- unionfs_read_unlock(dentry->d_sb);
unionfs_check_file(file);
+ unionfs_read_unlock(dentry->d_sb);
return err;
}
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index aed4d39..edea1a4 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -433,11 +433,11 @@ 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);
if (likely(err > 0)) { /* true==1: dentry is valid */
unionfs_check_dentry(dentry);
unionfs_check_nd(nd);
}
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index 809e0f1..cfbfb8e 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -32,8 +32,8 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
err = do_sync_read(file, buf, count, ppos);
out:
- unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
+ unionfs_read_unlock(file->f_path.dentry->d_sb);
return err;
}
@@ -55,8 +55,8 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
err = wait_on_sync_kiocb(iocb);
out:
- unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
+ unionfs_read_unlock(file->f_path.dentry->d_sb);
return err;
}
@@ -127,13 +127,13 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
}
out:
- unionfs_read_unlock(file->f_path.dentry->d_sb);
if (!err) {
/* copyup could cause parent dir times to change */
unionfs_copy_attr_times(file->f_path.dentry->d_parent->d_inode);
unionfs_check_file(file);
unionfs_check_dentry(file->f_path.dentry->d_parent);
}
+ unionfs_read_unlock(file->f_path.dentry->d_sb);
return err;
}
@@ -180,8 +180,8 @@ int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
unionfs_copy_attr_times(inode);
out:
- unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
+ unionfs_read_unlock(file->f_path.dentry->d_sb);
return err;
}
@@ -226,8 +226,8 @@ int unionfs_fasync(int fd, struct file *file, int flag)
unionfs_copy_attr_times(inode);
out:
- unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
+ unionfs_read_unlock(file->f_path.dentry->d_sb);
return err;
}
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 223a64a..1708f40 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -158,8 +158,6 @@ out:
if (!err)
unionfs_postcopyup_setmnt(dentry);
- unionfs_unlock_dentry(dentry);
- unionfs_read_unlock(dentry->d_sb);
unionfs_check_inode(parent);
if (!err) {
@@ -167,6 +165,8 @@ out:
unionfs_check_nd(nd);
}
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
+ unionfs_read_unlock(dentry->d_sb);
return err;
}
@@ -206,13 +206,14 @@ 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);
unionfs_check_dentry(dentry);
unionfs_check_dentry(dentry->d_parent);
unionfs_check_nd(nd);
+ if (!IS_ERR(ret))
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return ret;
@@ -356,12 +357,12 @@ out:
if (!err)
unionfs_postcopyup_setmnt(new_dentry);
- unionfs_unlock_dentry(new_dentry);
- unionfs_unlock_dentry(old_dentry);
-
unionfs_check_inode(dir);
unionfs_check_dentry(new_dentry);
unionfs_check_dentry(old_dentry);
+
+ unionfs_unlock_dentry(new_dentry);
+ unionfs_unlock_dentry(old_dentry);
unionfs_read_unlock(old_dentry->d_sb);
return err;
@@ -522,10 +523,10 @@ out:
kfree(name);
if (!err)
unionfs_postcopyup_setmnt(dentry);
- unionfs_unlock_dentry(dentry);
unionfs_check_inode(dir);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
@@ -675,9 +676,9 @@ out:
if (!err)
unionfs_copy_attr_times(dentry->d_inode);
- unionfs_unlock_dentry(dentry);
unionfs_check_inode(parent);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
@@ -805,10 +806,10 @@ out:
if (!err)
unionfs_postcopyup_setmnt(dentry);
- unionfs_unlock_dentry(dentry);
unionfs_check_inode(dir);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
@@ -843,8 +844,8 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf,
lower_dentry->d_inode);
out:
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
@@ -906,11 +907,11 @@ static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
if (unlikely(!__unionfs_d_revalidate_chain(dentry, nd, false)))
printk(KERN_ERR
"unionfs: put_link failed to revalidate dentry\n");
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
unionfs_check_nd(nd);
kfree(nd_get_link(nd));
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
}
@@ -1090,9 +1091,9 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
/* if setattr succeeded, then parent dir may have changed */
unionfs_copy_attr_times(dentry->d_parent->d_inode);
out:
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
unionfs_check_dentry(dentry->d_parent);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index ef8822f..4918f77 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -296,8 +296,8 @@ out:
if (err < 0)
ClearPageUptodate(page);
- unionfs_read_unlock(file->f_path.dentry->d_sb);
unionfs_check_file(file);
+ unionfs_read_unlock(file->f_path.dentry->d_sb);
return err; /* assume all is ok */
}
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 70487bf..88f77d7 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -153,8 +153,8 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
memset(&buf->f_spare, 0, sizeof(buf->f_spare));
out:
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(sb);
return err;
}
@@ -797,8 +797,8 @@ out_free:
kfree(new_data);
kfree(new_lower_inodes);
out_error:
- unionfs_write_unlock(sb);
unionfs_check_dentry(sb->s_root);
+ unionfs_write_unlock(sb);
return err;
}
diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index 602cedf..00c6d0d 100644
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -58,8 +58,8 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value,
err = vfs_getxattr(lower_dentry, (char *) name, value, size);
out:
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
}
@@ -88,8 +88,8 @@ int unionfs_setxattr(struct dentry *dentry, const char *name,
size, flags);
out:
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
}
@@ -116,8 +116,8 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
err = vfs_removexattr(lower_dentry, (char *) name);
out:
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
}
@@ -146,8 +146,8 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size)
err = vfs_listxattr(lower_dentry, encoded_list, size);
out:
- unionfs_unlock_dentry(dentry);
unionfs_check_dentry(dentry);
+ unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
}
--
1.5.2.2
Signed-off-by: Hugh Dickins <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/mmap.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 468dc61..bb00fd5 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -178,7 +178,8 @@ static int unionfs_do_readpage(struct file *file, struct page *page)
err = vfs_read(lower_file, page_data, PAGE_CACHE_SIZE,
&lower_file->f_pos);
set_fs(old_fs);
-
+ if (err >= 0 && err < PAGE_CACHE_SIZE)
+ memset(page_data + err, 0, PAGE_CACHE_SIZE - err);
kunmap(page);
if (err < 0)
--
1.5.2.2
Don't set/reset the PageUptodate flag on our page. Call flush_dcache_page
on the lower page after copy_highpage, and set it uptodate. Call
set_page_dirty right before clear_page_dirty_for_io.
CC: Hugh Dickins <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/mmap.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 4b00b98..468dc61 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -28,6 +28,7 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
struct address_space *lower_mapping; /* lower inode mapping */
gfp_t mask;
+ BUG_ON(!PageUptodate(page));
inode = page->mapping->host;
lower_inode = unionfs_lower_inode(inode);
lower_mapping = lower_inode->i_mapping;
@@ -53,6 +54,8 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
/* copy page data from our upper page to the lower page */
copy_highpage(lower_page, page);
+ flush_dcache_page(lower_page);
+ SetPageUptodate(lower_page);
/*
* Call lower writepage (expects locked page). However, if we are
@@ -68,12 +71,11 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
goto out_release;
}
BUG_ON(!lower_mapping->a_ops->writepage);
+ set_page_dirty(lower_page);
clear_page_dirty_for_io(lower_page); /* emulate VFS behavior */
err = lower_mapping->a_ops->writepage(lower_page, wbc);
- if (err < 0) {
- ClearPageUptodate(page);
+ if (err < 0)
goto out_release;
- }
/*
* Lower file systems such as ramfs and tmpfs, may return
@@ -97,7 +99,7 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
}
/* all is well */
- SetPageUptodate(page);
+
/* lower mtimes have changed: update ours */
unionfs_copy_attr_times(inode);
--
1.5.2.2