fusefs: automatically update mtime and ctime on write

Writing should implicitly update a file's mtime and ctime.  For fuse, the
server is supposed to do that.  But the client needs to do it too, because
the FUSE_WRITE response does not include time attributes, and it's not
desirable to issue a GETATTR after every WRITE.  When using the writeback
cache, there's another hitch: the kernel should ignore the mtime and ctime
fields in any GETATTR response for files with a dirty write cache.

Sponsored by:	The FreeBSD Foundation

sys/fs/fuse/fuse_internal.c

@@ -818,6 +818,8 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap,
 	struct fuse_getattr_in *fgai;
 	struct fuse_attr_out *fao;
 	off_t old_filesize = fvdat->cached_attrs.va_size;
+	struct timespec old_ctime = fvdat->cached_attrs.va_ctime;
+	struct timespec old_mtime = fvdat->cached_attrs.va_mtime;
 	enum vtype vtyp;
 	int err;
 
@@ -840,6 +842,14 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap,
 	vtyp = IFTOVT(fao->attr.mode);
 	if (fvdat->flag & FN_SIZECHANGE)
 		fao->attr.size = old_filesize;
+	if (fvdat->flag & FN_CTIMECHANGE) {
+		fao->attr.ctime = old_ctime.tv_sec;
+		fao->attr.ctimensec = old_ctime.tv_nsec;
+	}
+	if (fvdat->flag & FN_MTIMECHANGE) {
+		fao->attr.mtime = old_mtime.tv_sec;
+		fao->attr.mtimensec = old_mtime.tv_nsec;
+	}
 	fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
 		fao->attr_valid_nsec, vap);
 	if (vtyp != vnode_vtype(vp)) {
@@ -996,6 +1006,7 @@ fuse_internal_send_init(struct fuse_data *data, struct thread *td)
 int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
 	struct thread *td, struct ucred *cred)
 {
+	struct fuse_vnode_data *fvdat;
 	struct fuse_dispatcher fdi;
 	struct fuse_setattr_in *fsai;
 	struct mount *mp;
@@ -1008,6 +1019,7 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
 	uint64_t newsize = 0;
 
 	mp = vnode_mount(vp);
+	fvdat = VTOFUD(vp);
 	data = fuse_get_mpdata(mp);
 	dataflags = data->dataflags;
 
@@ -1057,6 +1069,10 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
 		fsai->valid |= FATTR_MTIME;
 		if (vap->va_vaflags & VA_UTIMES_NULL)
 			fsai->valid |= FATTR_MTIME_NOW;
+	} else if (fvdat->flag & FN_MTIMECHANGE) {
+		fsai->mtime = fvdat->cached_attrs.va_mtime.tv_sec;
+		fsai->mtimensec = fvdat->cached_attrs.va_mtime.tv_nsec;
+		fsai->valid |= FATTR_MTIME;
 	}
 	if (vap->va_mode != (mode_t)VNOVAL) {
 		fsai->mode = vap->va_mode & ALLPERMS;
@@ -1089,6 +1105,7 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
 	}
 	if (err == 0) {
 		struct fuse_attr_out *fao = (struct fuse_attr_out*)fdi.answ;
+		fuse_vnode_undirty_cached_timestamps(vp);
 		fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
 			fao->attr_valid_nsec, NULL);
 	}

sys/fs/fuse/fuse_io.c

@@ -229,6 +229,7 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag,
 		}
 		break;
 	case UIO_WRITE:
+		fuse_vnode_update(vp, FN_MTIMECHANGE | FN_CTIMECHANGE);
 		if (directio) {
 			const int iosize = fuse_iosize(vp);
 			off_t start, end, filesize;
@@ -458,6 +459,7 @@ fuse_write_directbackend(struct vnode *vp, struct uio *uio,
 	int diff;
 	int err = 0;
 	bool direct_io = fufh->fuse_open_flags & FOPEN_DIRECT_IO;
+	bool wrote_anything = false;
 	uint32_t write_flags;
 
 	data = fuse_get_mpdata(vp->v_mount);
@@ -533,6 +535,8 @@ fuse_write_directbackend(struct vnode *vp, struct uio *uio,
 			break;
 		} else if (err) {
 			break;
+		} else {
+			wrote_anything = true;
 		}
 
 		fwo = ((struct fuse_write_out *)fdi.answ);
@@ -586,6 +590,9 @@ fuse_write_directbackend(struct vnode *vp, struct uio *uio,
 
 	fdisp_destroy(&fdi);
 
+	if (wrote_anything)
+		fuse_vnode_undirty_cached_timestamps(vp);
+
 	return (err);
 }
 

sys/fs/fuse/fuse_node.c

@@ -441,3 +441,28 @@ fuse_vnode_size(struct vnode *vp, off_t *filesize, struct ucred *cred,
 
 	return error;
 }
+
+void
+fuse_vnode_undirty_cached_timestamps(struct vnode *vp)
+{
+	struct fuse_vnode_data *fvdat = VTOFUD(vp);
+
+	fvdat->flag &= ~(FN_MTIMECHANGE | FN_CTIMECHANGE);
+}
+
+/* Update a fuse file's cached timestamps */
+void
+fuse_vnode_update(struct vnode *vp, int flags)
+{
+	struct fuse_vnode_data *fvdat = VTOFUD(vp);
+	struct timespec ts;
+
+	vfs_timestamp(&ts);
+
+	if (flags & FN_MTIMECHANGE)
+		fvdat->cached_attrs.va_mtime = ts;
+	if (flags & FN_CTIMECHANGE)
+		fvdat->cached_attrs.va_ctime = ts;
+	
+	fvdat->flag |= flags;
+}

sys/fs/fuse/fuse_node.h

@@ -66,18 +66,26 @@
 
 #include "fuse_file.h"
 
-#define FN_REVOKED           0x00000020
-#define FN_FLUSHINPROG       0x00000040
-#define FN_FLUSHWANT         0x00000080
+#define	FN_REVOKED		0x00000020
+#define	FN_FLUSHINPROG		0x00000040
+#define	FN_FLUSHWANT		0x00000080
 /* 
  * Indicates that the file's size is dirty; the kernel has changed it but not
  * yet send the change to the daemon.  When this bit is set, the
- * cache_attrs.va_size field does not time out
+ * cache_attrs.va_size field does not time out.
  */
-#define FN_SIZECHANGE        0x00000100
-#define FN_DIRECTIO          0x00000200
+#define	FN_SIZECHANGE		0x00000100
+#define	FN_DIRECTIO		0x00000200
 /* Indicates that parent_nid is valid */
-#define FN_PARENT_NID        0x00000400
+#define	FN_PARENT_NID		0x00000400
+
+/* 
+ * Indicates that the file's cached timestamps are dirty.  They will be flushed
+ * during the next SETATTR or WRITE.  Until then, the cached fields will not
+ * time out.
+ */
+#define	FN_MTIMECHANGE		0x00000800
+#define	FN_CTIMECHANGE		0x00001000
 
 struct fuse_vnode_data {
 	/** self **/
@@ -180,4 +188,7 @@ int fuse_vnode_savesize(struct vnode *vp, struct ucred *cred, pid_t pid);
 
 int fuse_vnode_setsize(struct vnode *vp, off_t newsize);
 
+void fuse_vnode_undirty_cached_timestamps(struct vnode *vp);
+
+void fuse_vnode_update(struct vnode *vp, int flags);
 #endif /* _FUSE_NODE_H_ */

tests/sys/fs/fusefs/io.cc

@@ -140,10 +140,10 @@ void SetUp()
 	})));
 	EXPECT_CALL(*m_mock, process(
 		ResultOf([=](auto in) {
-			uint32_t valid = FATTR_SIZE | FATTR_FH;
 			return (in.header.opcode == FUSE_SETATTR &&
 				in.header.nodeid == ino &&
-				in.body.setattr.valid == valid);
+				(in.body.setattr.valid & FATTR_SIZE));
+				
 		}, Eq(true)),
 		_)
 	).WillRepeatedly(Invoke(ReturnImmediate([=](auto in, auto& out) {

tests/sys/fs/fusefs/write.cc

@@ -638,6 +638,35 @@ TEST_F(WriteThrough, pwrite)
 	/* Deliberately leak fd.  close(2) will be tested in release.cc */
 }
 
+/* Writing a file should update its cached mtime and ctime */
+TEST_F(Write, timestamps)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const char *CONTENTS = "abcdefgh";
+	ssize_t bufsize = strlen(CONTENTS);
+	uint64_t ino = 42;
+	struct stat sb0, sb1;
+	int fd;
+
+	expect_lookup(RELPATH, ino, 0);
+	expect_open(ino, 0, 1);
+	maybe_expect_write(ino, 0, bufsize, CONTENTS);
+
+	fd = open(FULLPATH, O_RDWR);
+	EXPECT_LE(0, fd) << strerror(errno);
+	ASSERT_EQ(0, fstat(fd, &sb0)) << strerror(errno);
+	ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno);
+
+	nap();
+
+	ASSERT_EQ(0, fstat(fd, &sb1)) << strerror(errno);
+
+	EXPECT_EQ(sb0.st_atime, sb1.st_atime);
+	EXPECT_NE(sb0.st_mtime, sb1.st_mtime);
+	EXPECT_NE(sb0.st_ctime, sb1.st_ctime);
+}
+
 TEST_F(Write, write)
 {
 	const char FULLPATH[] = "mountpoint/some_file.txt";
@@ -1014,6 +1043,99 @@ TEST_F(WriteBackAsync, eof)
 	/* Deliberately leak fd.  close(2) will be tested in release.cc */
 }
 
+/* 
+ * When a file has dirty writes that haven't been flushed, the server's notion
+ * of its mtime and ctime will be wrong.  The kernel should ignore those if it
+ * gets them from a FUSE_GETATTR before flushing.
+ */
+TEST_F(WriteBackAsync, timestamps)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const char *CONTENTS = "abcdefgh";
+	ssize_t bufsize = strlen(CONTENTS);
+	uint64_t ino = 42;
+	uint64_t attr_valid = 0;
+	uint64_t attr_valid_nsec = 0;
+	uint64_t server_time = 12345;
+	mode_t mode = S_IFREG | 0644;
+	int fd;
+
+	struct stat sb;
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.WillRepeatedly(Invoke(
+		ReturnImmediate([=](auto in __unused, auto& out) {
+		SET_OUT_HEADER_LEN(out, entry);
+		out.body.entry.attr.mode = mode;
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr.nlink = 1;
+		out.body.entry.attr_valid = attr_valid;
+		out.body.entry.attr_valid_nsec = attr_valid_nsec;
+	})));
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_GETATTR &&
+				in.header.nodeid == ino);
+		}, Eq(true)),
+		_)
+	).WillRepeatedly(Invoke(
+	ReturnImmediate([=](auto i __unused, auto& out) {
+		SET_OUT_HEADER_LEN(out, attr);
+		out.body.attr.attr.ino = ino;
+		out.body.attr.attr.mode = mode;
+		out.body.attr.attr_valid = attr_valid;
+		out.body.attr.attr_valid_nsec = attr_valid_nsec;
+		out.body.attr.attr.atime = server_time;
+		out.body.attr.attr.mtime = server_time;
+		out.body.attr.attr.ctime = server_time;
+	})));
+
+	fd = open(FULLPATH, O_RDWR);
+	EXPECT_LE(0, fd) << strerror(errno);
+	ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno);
+
+	ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno);
+	EXPECT_EQ((time_t)server_time, sb.st_atime);
+	EXPECT_NE((time_t)server_time, sb.st_mtime);
+	EXPECT_NE((time_t)server_time, sb.st_ctime);
+}
+
+/* Any dirty timestamp fields should be flushed during a SETATTR */
+TEST_F(WriteBackAsync, timestamps_during_setattr)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const char *CONTENTS = "abcdefgh";
+	ssize_t bufsize = strlen(CONTENTS);
+	uint64_t ino = 42;
+	const mode_t newmode = 0755;
+	int fd;
+
+	expect_lookup(RELPATH, ino, 0);
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			/* In protocol 7.23, ctime will be changed too */
+			uint32_t valid = FATTR_MODE | FATTR_MTIME;
+			return (in.header.opcode == FUSE_SETATTR &&
+				in.header.nodeid == ino &&
+				in.body.setattr.valid == valid);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+		SET_OUT_HEADER_LEN(out, attr);
+		out.body.attr.attr.ino = ino;
+		out.body.attr.attr.mode = S_IFREG | newmode;
+	})));
+
+	fd = open(FULLPATH, O_RDWR);
+	EXPECT_LE(0, fd) << strerror(errno);
+	ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno);
+	ASSERT_EQ(0, fchmod(fd, newmode)) << strerror(errno);
+}
+
 /*
  * Without direct_io, writes should be committed to cache
  */