[PATCH 0/7] fuse: fix handling mtime and cmtime in writeback_cache mode

[PATCH 0/7] fuse: fix handling mtime and cmtime in writeback_cache mode

[Maxim Patlasov]

Hi,

The patch-set fixes a few issues introduced by writeback_cache feature:

 - fuse_iget() used inode->i_mode before its initialization
 - in course of truncate(2), inode->i_mtime was not updated at all
 - the same for open(O_TRUNC) in case of fc->atomic_o_trunc is set
 - we cannot trust ctime from server anymore because we doesn't 
   flush all data changes to the server immediately
 - to maintain ctime locally, we have to update it in proper places
   (fuse_update_time, fallocate, setxattr, etc.)
 - locally updated ctime must be flushed to the server eventually.

Maxim
---

Maxim Patlasov (7):
      fuse: do not use uninitialized i_mode
      fuse: update mtime on truncate(2)
      fuse: update mtime on open(O_TRUNC) in atomic_o_trunc mode
      fuse: introduce FUSE_I_CTIME_DIRTY flag
      fuse: trust kernel i_ctime only
      fuse: clear FUSE_I_CTIME_DIRTY flag on setattr
      fuse: flush ctime in FUSE_FORGET requests


 fs/fuse/dev.c             |   30 ++++++++++++---
 fs/fuse/dir.c             |   88 +++++++++++++++++++++++++++++++++++++++------
 fs/fuse/file.c            |   37 +++++++++++++------
 fs/fuse/fuse_i.h          |    6 ++-
 fs/fuse/inode.c           |   14 ++++++-
 include/uapi/linux/fuse.h |   29 +++++++++++++--
 6 files changed, 165 insertions(+), 39 deletions(-)

-- 
Signature

[PATCH 1/7] fuse: do not use uninitialized i_mode

[Maxim Patlasov]

When inode is in I_NEW state, inode->i_mode is not initialized yet. Do not
use it before fuse_init_inode() is called.

Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
---
 fs/fuse/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 8d61169..299e553 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -303,7 +303,7 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 
 	if ((inode->i_state & I_NEW)) {
 		inode->i_flags |= S_NOATIME;
-		if (!fc->writeback_cache || !S_ISREG(inode->i_mode))
+		if (!fc->writeback_cache || !S_ISREG(attr->mode))
 			inode->i_flags |= S_NOCMTIME;
 		inode->i_generation = generation;
 		inode->i_data.backing_dev_info = &fc->bdi;

[PATCH 2/7] fuse: update mtime on truncate(2)

[Maxim Patlasov]

Handling truncate(2), VFS doesn't set ATTR_MTIME bit in iattr structure; only
ATTR_SIZE bit is set. In-kernel fuse must handle the case by setting mtime
fields of struct fuse_setattr_in to "now" and set FATTR_MTIME bit even
though ATTR_MTIME was not set.

Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
---
 fs/fuse/dir.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 5b4e035..c7cb41c 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1503,10 +1503,11 @@ static bool update_mtime(unsigned ivalid, bool trust_local_mtime)
 	return true;
 }
 
-static void iattr_to_fattr(struct iattr *iattr, struct fuse_setattr_in *arg,
-			   bool trust_local_mtime)
+static void iattr_to_fattr(struct inode *inode, struct iattr *iattr,
+			   struct fuse_setattr_in *arg, bool trust_local_mtime)
 {
 	unsigned ivalid = iattr->ia_valid;
+	struct timespec now = current_fs_time(inode->i_sb);
 
 	if (ivalid & ATTR_MODE)
 		arg->valid |= FATTR_MODE,   arg->mode = iattr->ia_mode;
@@ -1529,6 +1530,10 @@ static void iattr_to_fattr(struct iattr *iattr, struct fuse_setattr_in *arg,
 		arg->mtimensec = iattr->ia_mtime.tv_nsec;
 		if (!(ivalid & ATTR_MTIME_SET) && !trust_local_mtime)
 			arg->valid |= FATTR_MTIME_NOW;
+	} else if ((ivalid & ATTR_SIZE) && trust_local_mtime) {
+		arg->valid |= FATTR_MTIME;
+		arg->mtime = now.tv_sec;
+		arg->mtimensec = now.tv_nsec;
 	}
 }
 
@@ -1682,7 +1687,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
 
 	memset(&inarg, 0, sizeof(inarg));
 	memset(&outarg, 0, sizeof(outarg));
-	iattr_to_fattr(attr, &inarg, trust_local_mtime);
+	iattr_to_fattr(inode, attr, &inarg, trust_local_mtime);
 	if (file) {
 		struct fuse_file *ff = file->private_data;
 		inarg.valid |= FATTR_FH;
@@ -1711,8 +1716,9 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
 
 	spin_lock(&fc->lock);
 	/* the kernel maintains i_mtime locally */
-	if (trust_local_mtime && (attr->ia_valid & ATTR_MTIME)) {
-		inode->i_mtime = attr->ia_mtime;
+	if (trust_local_mtime && (attr->ia_valid & (ATTR_MTIME | ATTR_SIZE))) {
+		inode->i_mtime.tv_sec = inarg.mtime;
+		inode->i_mtime.tv_nsec = inarg.mtimensec;
 		clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
 	}
 

[PATCH 3/7] fuse: update mtime on open(O_TRUNC) in atomic_o_trunc mode

[Maxim Patlasov]

In case of fc->atomic_o_trunc is set, fuse does nothing in fuse_do_setattr()
while handling open(O_TRUNC). Hence, i_mtime must be updated explicitly in
fuse_finish_open(). The patch also adds extra locking encompassing
open(O_TRUNC) operation to avoid races between updating i_mtime and setting
FUSE_I_MTIME_DIRTY flag.

Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
---
 fs/fuse/file.c |   20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 13f8bde..c2c6df7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -223,6 +223,10 @@ void fuse_finish_open(struct inode *inode, struct file *file)
 		i_size_write(inode, 0);
 		spin_unlock(&fc->lock);
 		fuse_invalidate_attr(inode);
+		if (fc->writeback_cache) {
+			inode->i_mtime = current_fs_time(inode->i_sb);
+			set_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+		}
 	}
 	if ((file->f_mode & FMODE_WRITE) && fc->writeback_cache)
 		fuse_link_write_file(file);
@@ -232,18 +236,26 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	int err;
+	bool lock_inode = (file->f_flags & O_TRUNC) &&
+			  fc->atomic_o_trunc &&
+			  fc->writeback_cache;
 
 	err = generic_file_open(inode, file);
 	if (err)
 		return err;
 
+	if (lock_inode)
+		mutex_lock(&inode->i_mutex);
+
 	err = fuse_do_open(fc, get_node_id(inode), file, isdir);
-	if (err)
-		return err;
 
-	fuse_finish_open(inode, file);
+	if (!err)
+		fuse_finish_open(inode, file);
 
-	return 0;
+	if (lock_inode)
+		mutex_unlock(&inode->i_mutex);
+
+	return err;
 }
 
 static void fuse_prepare_release(struct fuse_file *ff, int flags, int opcode)

[PATCH 4/7] fuse: introduce FUSE_I_CTIME_DIRTY flag

[Maxim Patlasov]

The flag plays the role similar to FUSE_I_MTIME_DIRTY: local ctime
(inode->i_ctime) is newer than on the server; so, local ctime must be
flushed to the server afterwards.

The patch only introduces the flag, extends API (fuse_setattr_in) properly,
and implements the flush procedure (fuse_flush_cmtime()). Further patches of
the patch-set will implement the logic of setting and clearing the flag.

Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
---
 fs/fuse/dir.c             |   22 ++++++++++++++++++----
 fs/fuse/file.c            |   11 ++++-------
 fs/fuse/fuse_i.h          |    4 +++-
 include/uapi/linux/fuse.h |   11 ++++++++---
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c7cb41c..0596726 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1600,9 +1600,9 @@ static void fuse_setattr_fill(struct fuse_conn *fc, struct fuse_req *req,
 }
 
 /*
- * Flush inode->i_mtime to the server
+ * Flush inode->i_mtime and inode->i_ctime to the server
  */
-int fuse_flush_mtime(struct file *file, bool nofail)
+int fuse_flush_cmtime(struct file *file, bool nofail)
 {
 	struct inode *inode = file->f_mapping->host;
 	struct fuse_inode *fi = get_fuse_inode(inode);
@@ -1610,8 +1610,16 @@ int fuse_flush_mtime(struct file *file, bool nofail)
 	struct fuse_req *req = NULL;
 	struct fuse_setattr_in inarg;
 	struct fuse_attr_out outarg;
+	unsigned cmtime_flags = 0;
 	int err;
 
+	if (test_bit(FUSE_I_MTIME_DIRTY, &fi->state))
+		cmtime_flags |= FATTR_MTIME;
+	if (test_bit(FUSE_I_CTIME_DIRTY, &fi->state) && fc->minor >= 24)
+		cmtime_flags |= FATTR_CTIME;
+	if (!cmtime_flags)
+		return 0;
+
 	if (nofail) {
 		req = fuse_get_req_nofail_nopages(fc, file);
 	} else {
@@ -1623,17 +1631,23 @@ int fuse_flush_mtime(struct file *file, bool nofail)
 	memset(&inarg, 0, sizeof(inarg));
 	memset(&outarg, 0, sizeof(outarg));
 
-	inarg.valid |= FATTR_MTIME;
+	inarg.valid = cmtime_flags;
 	inarg.mtime = inode->i_mtime.tv_sec;
 	inarg.mtimensec = inode->i_mtime.tv_nsec;
+	if (fc->minor >= 24) {
+		inarg.ctime = inode->i_ctime.tv_sec;
+		inarg.ctimensec = inode->i_ctime.tv_nsec;
+	}
 
 	fuse_setattr_fill(fc, req, inode, &inarg, &outarg);
 	fuse_request_send(fc, req);
 	err = req->out.h.error;
 	fuse_put_request(fc, req);
 
-	if (!err)
+	if (!err) {
 		clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+		clear_bit(FUSE_I_CTIME_DIRTY, &fi->state);
+	}
 
 	return err;
 }
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index c2c6df7..9ed8590b 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -328,8 +328,7 @@ static int fuse_release(struct inode *inode, struct file *file)
 	if (fc->writeback_cache)
 		filemap_write_and_wait(file->f_mapping);
 
-	if (test_bit(FUSE_I_MTIME_DIRTY, &get_fuse_inode(inode)->state))
-		fuse_flush_mtime(file, true);
+	fuse_flush_cmtime(file, true);
 
 	fuse_release_common(file, FUSE_RELEASE);
 
@@ -512,11 +511,9 @@ int fuse_fsync_common(struct file *file, loff_t start, loff_t end,
 
 	fuse_sync_writes(inode);
 
-	if (test_bit(FUSE_I_MTIME_DIRTY, &get_fuse_inode(inode)->state)) {
-		int err = fuse_flush_mtime(file, false);
-		if (err)
-			goto out;
-	}
+	err = fuse_flush_cmtime(file, false);
+	if (err)
+		goto out;
 
 	req = fuse_get_req_nopages(fc);
 	if (IS_ERR(req)) {
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index a257ed8e..781a01d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -121,6 +121,8 @@ enum {
 	FUSE_I_SIZE_UNSTABLE,
 	/** i_mtime has been updated locally; a flush to userspace needed */
 	FUSE_I_MTIME_DIRTY,
+	/** i_ctime has been updated locally; a flush to userspace needed */
+	FUSE_I_CTIME_DIRTY,
 };
 
 struct fuse_conn;
@@ -891,7 +893,7 @@ int fuse_dev_release(struct inode *inode, struct file *file);
 
 bool fuse_write_update_size(struct inode *inode, loff_t pos);
 
-int fuse_flush_mtime(struct file *file, bool nofail);
+int fuse_flush_cmtime(struct file *file, bool nofail);
 
 int fuse_do_setattr(struct inode *inode, struct iattr *attr,
 		    struct file *file);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index cf4750e..8af06cc 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -96,6 +96,10 @@
  *
  * 7.23
  *  - add FUSE_WRITEBACK_CACHE
+ *
+ * 7.24
+ *  - ctime support for FUSE_WRITEBACK_CACHE:
+ *    changes in fuse_setattr_in and fuse_forget_in
  */
 
 #ifndef _LINUX_FUSE_H
@@ -131,7 +135,7 @@
 #define FUSE_KERNEL_VERSION 7
 
 /** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 23
+#define FUSE_KERNEL_MINOR_VERSION 24
 
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1
@@ -191,6 +195,7 @@ struct fuse_file_lock {
 #define FATTR_ATIME_NOW	(1 << 7)
 #define FATTR_MTIME_NOW	(1 << 8)
 #define FATTR_LOCKOWNER	(1 << 9)
+#define FATTR_CTIME	(1 << 10)
 
 /**
  * Flags returned by the OPEN request
@@ -438,10 +443,10 @@ struct fuse_setattr_in {
 	uint64_t	lock_owner;
 	uint64_t	atime;
 	uint64_t	mtime;
-	uint64_t	unused2;
+	uint64_t	ctime;
 	uint32_t	atimensec;
 	uint32_t	mtimensec;
-	uint32_t	unused3;
+	uint32_t	ctimensec;
 	uint32_t	mode;
 	uint32_t	unused4;
 	uint32_t	uid;

[PATCH 5/7] fuse: trust kernel i_ctime only

[Maxim Patlasov]

Let the kernel maintain i_ctime locally:
 - clear S_NOCMTIME (implemented earlier)
 - update i_ctime in i_op->update_time()
 - flush ctime on fsync and last close (previous patch)
 - update i_ctime explicitly on truncate, fallocate, open(O_TRUNC),
   setxattr, link, rename, unlink.

Fuse inode flag FUSE_I_CTIME_DIRTY serves as indication that local i_ctime
should be flushed to the server eventually. The patch sets the flag and updates
i_ctime in course of operations listed above.

Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
---
 fs/fuse/dir.c   |   24 +++++++++++++++++++++++-
 fs/fuse/file.c  |    8 ++++++--
 fs/fuse/inode.c |    6 ++++--
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 0596726..2187960 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -679,6 +679,15 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
 	return create_new_entry(fc, req, dir, entry, S_IFLNK);
 }
 
+static inline void fuse_update_ctime(struct fuse_conn *fc, struct inode *inode)
+{
+	if (fc->writeback_cache && S_ISREG(inode->i_mode)) {
+		struct fuse_inode *fi = get_fuse_inode(inode);
+		inode->i_ctime = current_fs_time(inode->i_sb);
+		set_bit(FUSE_I_CTIME_DIRTY, &fi->state);
+	}
+}
+
 static int fuse_unlink(struct inode *dir, struct dentry *entry)
 {
 	int err;
@@ -713,6 +722,7 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
 		fuse_invalidate_attr(inode);
 		fuse_invalidate_attr(dir);
 		fuse_invalidate_entry_cache(entry);
+		fuse_update_ctime(fc, inode);
 	} else if (err == -EINTR)
 		fuse_invalidate_entry(entry);
 	return err;
@@ -771,6 +781,7 @@ static int fuse_rename(struct inode *olddir, struct dentry *oldent,
 	if (!err) {
 		/* ctime changes */
 		fuse_invalidate_attr(oldent->d_inode);
+		fuse_update_ctime(fc, oldent->d_inode);
 
 		fuse_invalidate_attr(olddir);
 		if (olddir != newdir)
@@ -780,6 +791,7 @@ static int fuse_rename(struct inode *olddir, struct dentry *oldent,
 		if (newent->d_inode) {
 			fuse_invalidate_attr(newent->d_inode);
 			fuse_invalidate_entry_cache(newent);
+			fuse_update_ctime(fc, newent->d_inode);
 		}
 	} else if (err == -EINTR) {
 		/* If request was interrupted, DEITY only knows if the
@@ -829,6 +841,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 		inc_nlink(inode);
 		spin_unlock(&fc->lock);
 		fuse_invalidate_attr(inode);
+		fuse_update_ctime(fc, inode);
 	} else if (err == -EINTR) {
 		fuse_invalidate_attr(inode);
 	}
@@ -846,6 +859,8 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
 		attr->size = i_size_read(inode);
 		attr->mtime = inode->i_mtime.tv_sec;
 		attr->mtimensec = inode->i_mtime.tv_nsec;
+		attr->ctime = inode->i_ctime.tv_sec;
+		attr->ctimensec = inode->i_ctime.tv_nsec;
 	}
 
 	stat->dev = inode->i_sb->s_dev;
@@ -1830,8 +1845,10 @@ static int fuse_setxattr(struct dentry *entry, const char *name,
 		fc->no_setxattr = 1;
 		err = -EOPNOTSUPP;
 	}
-	if (!err)
+	if (!err) {
 		fuse_invalidate_attr(inode);
+		fuse_update_ctime(fc, inode);
+	}
 	return err;
 }
 
@@ -1974,6 +1991,11 @@ static int fuse_update_time(struct inode *inode, struct timespec *now,
 		set_bit(FUSE_I_MTIME_DIRTY, &get_fuse_inode(inode)->state);
 		BUG_ON(!S_ISREG(inode->i_mode));
 	}
+	if (flags & S_CTIME) {
+		inode->i_ctime = *now;
+		set_bit(FUSE_I_CTIME_DIRTY, &get_fuse_inode(inode)->state);
+		BUG_ON(!S_ISREG(inode->i_mode));
+	}
 	return 0;
 }
 
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 9ed8590b..f644aeb 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -224,8 +224,10 @@ void fuse_finish_open(struct inode *inode, struct file *file)
 		spin_unlock(&fc->lock);
 		fuse_invalidate_attr(inode);
 		if (fc->writeback_cache) {
-			inode->i_mtime = current_fs_time(inode->i_sb);
+			inode->i_ctime = inode->i_mtime =
+				current_fs_time(inode->i_sb);
 			set_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+			set_bit(FUSE_I_CTIME_DIRTY, &fi->state);
 		}
 	}
 	if ((file->f_mode & FMODE_WRITE) && fc->writeback_cache)
@@ -3029,8 +3031,10 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
 		if (changed && fc->writeback_cache) {
 			struct fuse_inode *fi = get_fuse_inode(inode);
 
-			inode->i_mtime = current_fs_time(inode->i_sb);
+			inode->i_ctime = inode->i_mtime =
+				current_fs_time(inode->i_sb);
 			set_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+			set_bit(FUSE_I_CTIME_DIRTY, &fi->state);
 		}
 	}
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 299e553..8b9a1d1 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -175,9 +175,9 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
 	if (!fc->writeback_cache || !S_ISREG(inode->i_mode)) {
 		inode->i_mtime.tv_sec   = attr->mtime;
 		inode->i_mtime.tv_nsec  = attr->mtimensec;
+		inode->i_ctime.tv_sec   = attr->ctime;
+		inode->i_ctime.tv_nsec  = attr->ctimensec;
 	}
-	inode->i_ctime.tv_sec   = attr->ctime;
-	inode->i_ctime.tv_nsec  = attr->ctimensec;
 
 	if (attr->blksize != 0)
 		inode->i_blkbits = ilog2(attr->blksize);
@@ -256,6 +256,8 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
 	inode->i_size = attr->size;
 	inode->i_mtime.tv_sec  = attr->mtime;
 	inode->i_mtime.tv_nsec = attr->mtimensec;
+	inode->i_ctime.tv_sec  = attr->ctime;
+	inode->i_ctime.tv_nsec = attr->ctimensec;
 	if (S_ISREG(inode->i_mode)) {
 		fuse_init_common(inode);
 		fuse_init_file_inode(inode);

[PATCH 6/7] fuse: clear FUSE_I_CTIME_DIRTY flag on setattr

[Maxim Patlasov]

The patch addresses two use-cases when the flag may be safely cleared:

1. fuse_do_setattr() is called with ATTR_CTIME flag set in attr->ia_valid.
In this case attr->ia_ctime bears actual value. In-kernel fuse must send it
to the userspace server and then assign the value to inode->i_ctime.

2. fuse_do_setattr() is called with ATTR_SIZE flag set in attr->ia_valid,
whereas ATTR_CTIME is not set (truncate(2)).
In this case in-kernel fuse must sent "now" to the userspace server and then
assign the value to inode->i_ctime.

In both cases fuse can clear FUSE_I_CTIME_DIRTY flag because actual ctime
value was flushed to the server.

Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
---
 fs/fuse/dir.c |   38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 2187960..3495aaf 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1518,8 +1518,9 @@ static bool update_mtime(unsigned ivalid, bool trust_local_mtime)
 	return true;
 }
 
-static void iattr_to_fattr(struct inode *inode, struct iattr *iattr,
-			   struct fuse_setattr_in *arg, bool trust_local_mtime)
+static void iattr_to_fattr(struct fuse_conn *fc, struct inode *inode,
+			   struct iattr *iattr, struct fuse_setattr_in *arg,
+			   bool trust_local_mtime, struct timespec *ctime)
 {
 	unsigned ivalid = iattr->ia_valid;
 	struct timespec now = current_fs_time(inode->i_sb);
@@ -1550,6 +1551,19 @@ static void iattr_to_fattr(struct inode *inode, struct iattr *iattr,
 		arg->mtime = now.tv_sec;
 		arg->mtimensec = now.tv_nsec;
 	}
+
+	if ((ivalid & ATTR_CTIME) && trust_local_mtime)
+		*ctime = iattr->ia_ctime;
+	else if ((ivalid & ATTR_SIZE) && trust_local_mtime)
+		*ctime = now;
+	else
+		ctime = NULL;
+
+	if (ctime && fc->minor >= 24) {
+		arg->valid |= FATTR_CTIME;
+		arg->ctime = ctime->tv_sec;
+		arg->ctimensec = ctime->tv_nsec;
+	}
 }
 
 /*
@@ -1688,6 +1702,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
 	loff_t oldsize;
 	int err;
 	bool trust_local_mtime = is_wb && S_ISREG(inode->i_mode);
+	struct timespec ctime;
 
 	if (!(fc->flags & FUSE_DEFAULT_PERMISSIONS))
 		attr->ia_valid |= ATTR_FORCE;
@@ -1716,7 +1731,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
 
 	memset(&inarg, 0, sizeof(inarg));
 	memset(&outarg, 0, sizeof(outarg));
-	iattr_to_fattr(inode, attr, &inarg, trust_local_mtime);
+	iattr_to_fattr(fc, inode, attr, &inarg, trust_local_mtime, &ctime);
 	if (file) {
 		struct fuse_file *ff = file->private_data;
 		inarg.valid |= FATTR_FH;
@@ -1744,11 +1759,18 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
 	}
 
 	spin_lock(&fc->lock);
-	/* the kernel maintains i_mtime locally */
-	if (trust_local_mtime && (attr->ia_valid & (ATTR_MTIME | ATTR_SIZE))) {
-		inode->i_mtime.tv_sec = inarg.mtime;
-		inode->i_mtime.tv_nsec = inarg.mtimensec;
-		clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+	/* the kernel maintains i_mtime and i_ctime locally */
+	if (trust_local_mtime) {
+		if (attr->ia_valid & (ATTR_MTIME | ATTR_SIZE)) {
+			inode->i_mtime.tv_sec = inarg.mtime;
+			inode->i_mtime.tv_nsec = inarg.mtimensec;
+			clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+		}
+
+		if (attr->ia_valid & (ATTR_CTIME | ATTR_SIZE)) {
+			inode->i_ctime = ctime;
+			clear_bit(FUSE_I_CTIME_DIRTY, &fi->state);
+		}
 	}
 
 	fuse_change_attributes_common(inode, &outarg.attr,

[PATCH 7/7] fuse: flush ctime in FUSE_FORGET requests

[Maxim Patlasov]

Some inode operations (e.g., rename) operate directly on inodes and dentries
without opened files involved. This means that even though fuse set
inode->i_ctime and FUSE_I_CTIME_DIRTY properly, a corresponding flush operation
will never happen (i.e. no fsync or close to call fuse_flush_cmtime()).

The patch solves the problem by passing local ctime to the userspace server
inside forget requests.

Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
---
 fs/fuse/dev.c             |   30 +++++++++++++++++++++++-------
 fs/fuse/fuse_i.h          |    2 +-
 fs/fuse/inode.c           |    6 ++++++
 include/uapi/linux/fuse.h |   18 ++++++++++++++++++
 4 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index aac71ce..97c7749 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -578,17 +578,25 @@ void fuse_request_send_background_locked(struct fuse_conn *fc,
 void fuse_force_forget(struct file *file, u64 nodeid)
 {
 	struct inode *inode = file_inode(file);
+	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_req *req;
-	struct fuse_forget_in inarg;
+	struct fuse_forget_in_ext inarg;
+	int inarg_size = fc->minor >= 24 ?
+		sizeof(inarg) : sizeof(struct fuse_forget_in);
 
 	memset(&inarg, 0, sizeof(inarg));
 	inarg.nlookup = 1;
+	if (test_bit(FUSE_I_CTIME_DIRTY, &fi->state)) {
+		inarg.forget_flags = FUSE_FORGET_CTIME_SET;
+		inarg.ctime = inode->i_ctime.tv_sec;
+		inarg.ctimensec = inode->i_ctime.tv_nsec;
+	}
 	req = fuse_get_req_nofail_nopages(fc, file);
 	req->in.h.opcode = FUSE_FORGET;
 	req->in.h.nodeid = nodeid;
 	req->in.numargs = 1;
-	req->in.args[0].size = sizeof(inarg);
+	req->in.args[0].size = inarg_size;
 	req->in.args[0].value = &inarg;
 	req->isreply = 0;
 	__fuse_request_send(fc, req);
@@ -1102,14 +1110,19 @@ __releases(fc->lock)
 {
 	int err;
 	struct fuse_forget_link *forget = dequeue_forget(fc, 1, NULL);
-	struct fuse_forget_in arg = {
+	struct fuse_forget_in_ext arg = {
 		.nlookup = forget->forget_one.nlookup,
+		.ctime = forget->forget_one.ctime,
+		.ctimensec = forget->forget_one.ctimensec,
+		.forget_flags = forget->forget_one.forget_flags,
 	};
+	int arg_size = fc->minor >= 24 ?
+		sizeof(arg) : sizeof(struct fuse_forget_in);
 	struct fuse_in_header ih = {
 		.opcode = FUSE_FORGET,
 		.nodeid = forget->forget_one.nodeid,
 		.unique = fuse_get_unique(fc),
-		.len = sizeof(ih) + sizeof(arg),
+		.len = sizeof(ih) + arg_size,
 	};
 
 	spin_unlock(&fc->lock);
@@ -1142,18 +1155,21 @@ __releases(fc->lock)
 		.unique = fuse_get_unique(fc),
 		.len = sizeof(ih) + sizeof(arg),
 	};
+	int forget_one_size = fc->minor >= 24 ?
+		sizeof(struct fuse_forget_one_ext) :
+		sizeof(struct fuse_forget_one);
 
 	if (nbytes < ih.len) {
 		spin_unlock(&fc->lock);
 		return -EINVAL;
 	}
 
-	max_forgets = (nbytes - ih.len) / sizeof(struct fuse_forget_one);
+	max_forgets = (nbytes - ih.len) / forget_one_size;
 	head = dequeue_forget(fc, max_forgets, &count);
 	spin_unlock(&fc->lock);
 
 	arg.count = count;
-	ih.len += count * sizeof(struct fuse_forget_one);
+	ih.len += count * forget_one_size;
 	err = fuse_copy_one(cs, &ih, sizeof(ih));
 	if (!err)
 		err = fuse_copy_one(cs, &arg, sizeof(arg));
@@ -1163,7 +1179,7 @@ __releases(fc->lock)
 
 		if (!err) {
 			err = fuse_copy_one(cs, &forget->forget_one,
-					    sizeof(forget->forget_one));
+					    forget_one_size);
 		}
 		head = forget->next;
 		kfree(forget);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 781a01d..d5172fa 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -59,7 +59,7 @@ extern unsigned max_user_congthresh;
 
 /* One forget request */
 struct fuse_forget_link {
-	struct fuse_forget_one forget_one;
+	struct fuse_forget_one_ext forget_one;
 	struct fuse_forget_link *next;
 };
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 8b9a1d1..932e04a 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -128,6 +128,12 @@ static void fuse_evict_inode(struct inode *inode)
 	if (inode->i_sb->s_flags & MS_ACTIVE) {
 		struct fuse_conn *fc = get_fuse_conn(inode);
 		struct fuse_inode *fi = get_fuse_inode(inode);
+		if (test_bit(FUSE_I_CTIME_DIRTY, &fi->state)) {
+			struct fuse_forget_link *fl = fi->forget;
+			fl->forget_one.forget_flags = FUSE_FORGET_CTIME_SET;
+			fl->forget_one.ctime = inode->i_ctime.tv_sec;
+			fl->forget_one.ctimensec = inode->i_ctime.tv_nsec;
+		}
 		fuse_queue_forget(fc, fi->forget, fi->nodeid, fi->nlookup);
 		fi->forget = NULL;
 	}
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 8af06cc..5354a8c 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -388,6 +388,24 @@ struct fuse_forget_in {
 	uint64_t	nlookup;
 };
 
+struct fuse_forget_in_ext {
+	uint64_t	nlookup;
+	uint64_t	ctime;
+	uint32_t	ctimensec;
+	uint32_t	forget_flags;
+};
+
+/* forget_flags */
+#define FUSE_FORGET_CTIME_SET	(1 << 0)
+
+struct fuse_forget_one_ext {
+	uint64_t	nodeid;
+	uint64_t	nlookup;
+	uint64_t	ctime;
+	uint32_t	ctimensec;
+	uint32_t	forget_flags;
+};
+
 struct fuse_forget_one {
 	uint64_t	nodeid;
 	uint64_t	nlookup;

Re: [PATCH 7/7] fuse: flush ctime in FUSE_FORGET requests

[Miklos Szeredi]

Maxim,

Thanks for the patches.

On Tue, Apr 15, 2014 at 1:33 PM, Maxim Patlasov <MPatlasov@parallels.com> wrote:
> Some inode operations (e.g., rename) operate directly on inodes and dentries
> without opened files involved. This means that even though fuse set
> inode->i_ctime and FUSE_I_CTIME_DIRTY properly, a corresponding flush operation
> will never happen (i.e. no fsync or close to call fuse_flush_cmtime()).
>
> The patch solves the problem by passing local ctime to the userspace server
> inside forget requests.

Hmm, I really don't like this.

1) What has forget to do with ctime?  It feels like being forced into
the interface
2) Forget may not be called for a long time after the modification and
it may not be called *at all*, which would result in the loss of the
ctime change after umount.

How about wiring up fuse_flush_cmtime() to be called from
s_op->write_inode() instead?

Updated patchset pushed to

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-linus

Survives some basic testing, but it would be great if you could also
take a look.

Thanks,
Miklos