tux3: Fix tux3_iattrdirty() usages
Now, on some paths, mark_inode_dirty() is not called after
tux3_iattrdirty() was called.
E.g. If inode->i_ctime is same with current time, file_update_time()
skips to dirty inode.
tux3_iattrdirty()
file_update_time() {
if (!timespec_equal(&inode->i_ctime, &now))
sync_it |= S_CTIME;
if (sync_it)
mark_inode_dirty_sync()
}
This is wrong as tux3_iattrdirty() usage. tux3_iattrdirty() was
called, the caller must call mark_inode_dirty() too.
Otherwise,
delta = 1
tux3_iattrdirty(inode)
iattr_delta = delta
/* flush delta 1 */
inode is not dirty, so iattr_delta == 1 is remaining
/* flush delta 2 for data pages */
read_idata_for_i_size()
if (iattr_delta != delta)
/*
* iattr_delta is still 1, so read from idata[], but
* idata[] is invalid.
*/
By remaining iattr_delta, future inode flush is confused, and hits to
assertion.
To fix this, this makes sure to call tux3_iattrdirty() only when we call
mark_inode_dirty().
[FIXME: file_update_time() of mmap and write can race.]
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
diff --git a/fs/tux3/inode.c b/fs/tux3/inode.c
index 9d9f6ff..1bfb28f 100644
--- a/fs/tux3/inode.c
+++ b/fs/tux3/inode.c
@@ -572,17 +572,15 @@
}
/* Change i_size, then clean buffers */
+ tux3_iattrdirty(inode);
i_size_write(inode, newsize);
/* Roundup. Partial page is handled by tux3_truncate_partial_block() */
holebegin = round_up(newsize, boundary);
if (newsize <= holebegin) /* Check overflow */
tux3_truncate_pagecache(inode, holebegin);
- if (!is_expand) {
+ if (!is_expand)
err = tux3_add_truncate_hole(inode, newsize);
- if (err)
- goto error;
- }
inode->i_mtime = inode->i_ctime = gettime();
tux3_mark_inode_dirty(inode);
@@ -833,13 +831,13 @@
down_write(&tux_inode(inode)->truncate_lock);
change_begin(sb);
- tux3_iattrdirty(inode);
-
if (need_truncate)
err = tux3_truncate(inode, iattr->ia_size);
- if (!err)
+ if (!err) {
+ tux3_iattrdirty(inode);
setattr_copy(inode, iattr);
- tux3_mark_inode_dirty(inode);
+ tux3_mark_inode_dirty(inode);
+ }
change_end(sb);
if (need_lock)
@@ -848,6 +846,26 @@
return err;
}
+static int tux3_file_update_time(struct inode *inode, struct timespec *time,
+ int flags)
+{
+ /* FIXME: atime is not supported yet */
+ if (flags & S_ATIME)
+ inode->i_atime = *time;
+ if (!(flags & ~S_ATIME))
+ return 0;
+
+ tux3_iattrdirty(inode);
+ if (flags & S_VERSION)
+ inode_inc_iversion(inode);
+ if (flags & S_CTIME)
+ inode->i_ctime = *time;
+ if (flags & S_MTIME)
+ inode->i_mtime = *time;
+ mark_inode_dirty_sync(inode);
+ return 0;
+}
+
#include "inode_vfslib.c"
static const struct file_operations tux_file_fops = {
@@ -870,7 +888,7 @@
static const struct inode_operations tux_file_iops = {
// .permission = ext4_permission,
.setattr = tux3_setattr,
- .getattr = tux3_getattr
+ .getattr = tux3_getattr,
#ifdef CONFIG_EXT4DEV_FS_XATTR
// .setxattr = generic_setxattr,
// .getxattr = generic_getxattr,
@@ -879,6 +897,7 @@
#endif
// .fallocate = ext4_fallocate,
// .fiemap = ext4_fiemap,
+ .update_time = tux3_file_update_time,
};
static const struct inode_operations tux_special_iops = {
diff --git a/fs/tux3/inode_vfslib.c b/fs/tux3/inode_vfslib.c
index cc09623..819a0fd 100644
--- a/fs/tux3/inode_vfslib.c
+++ b/fs/tux3/inode_vfslib.c
@@ -25,8 +25,7 @@
mutex_lock(&inode->i_mutex);
/* For each ->write_end() calls change_end(). */
change_begin(sb);
- /* For timestamp. FIXME: convert this to ->update_time handler? */
- tux3_iattrdirty(inode);
+ /* FIXME: file_update_time() in this can be race with mmap */
ret = __generic_file_aio_write(iocb, iov, nr_segs, &iocb->ki_pos);
change_end_if_needed(sb);
mutex_unlock(&inode->i_mutex);
@@ -71,11 +70,9 @@
mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
/* For each ->write_end() calls change_end(). */
change_begin(sb);
- /* For timestamp. FIXME: convert this to ->update_time
- * handler? */
- tux3_iattrdirty(inode);
ret = file_remove_suid(out);
if (!ret) {
+ /* FIXME: file_update_time() can be race with mmap */
ret = file_update_time(out);
if (!ret)
ret = splice_from_pipe_feed(pipe, &sd,
diff --git a/fs/tux3/writeback_iattrfork.c b/fs/tux3/writeback_iattrfork.c
index 53f197b..8e6c42c 100644
--- a/fs/tux3/writeback_iattrfork.c
+++ b/fs/tux3/writeback_iattrfork.c
@@ -45,6 +45,15 @@
idata->i_version = inode->i_version;
}
+/*
+ * Inode attributes fork. (See comment on top of this source)
+ *
+ * NOTE: caller must call tux3_mark_inode_dirty() after
+ * this. Otherwise, inode state will be remaining after flush, and
+ * will confuses flusher in future.
+ *
+ * FIXME: this is better to call tux3_mark_inode_dirty() too?
+ */
void tux3_iattrdirty(struct inode *inode)
{
struct tux3_inode *tuxnode = tux_inode(inode);
