Message-Id: <20220624055825.29183-1-zhangjiachen.jaycee@bytedance.com>
Date:   Fri, 24 Jun 2022 13:58:25 +0800
From:   Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
To:     miklos@...redi.hu, linux-fsdevel@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, xieyongji@...edance.com,
        fam.zheng@...edance.com,
        Jiachen Zhang <zhangjiachen.jaycee@...edance.com>,
        Miklos Szeredi <mszeredi@...hat.com>
Subject: [PATCH] fuse: writeback_cache consistency enhancement (writeback_cache_v2)

Some users may want both the high performance of the writeback_cahe mode and
a little bit more consistency among FUSE mounts. In the current writeback
mode implementation, users of one FUSE mount can never see the file
expansion done by other FUSE mounts.

Based on the suggested writeback V2 patch in the upstream mailing-list [1],
this commit allows the cmtime and size to be updated from server in
writeback mode. Compared with the writeback V2 patch in [1], this commit has
several differences:

    1. Ensure c/mtime are not updated from kernel to server. IOW, the cmtime
    generated by kernel are just temporary values that are never flushed to
    server, and they can also be updated by the official server cmtime when
    the writeback cache is clean.

    2. Skip mtime-based revalidation when fc->auto_inval_data is set with
    fc->writeback_cache_v2. Because the kernel-generated temporary cmtime
    are likely not equal to the offical server cmtime.

    3. If any page is ever flushed to the server during FUSE_GETATTR
    handling on fuse server, even if the cache is clean when
    fuse_change_attributes() checks, we should not update the i_size. This
    is because the FUSE_GETATTR may get a staled size before the FUSE_WRITE
    request changes server inode size. This commit ensures this by
    increasing attr_version after writeback for writeback_cache_v2. In that
    case, we should also ensure the ordering of the attr_version updating
    and the fi->writepages RB-tree updating. So that if a fuse page
    writeback ever happens during fuse_change_attributes(), either the
    fi->writepages is not empty, or the attr_version is increased. So we
    never mistakenly update a stale file size from server to kernel.

With this patch, writeback mode can consider the server c/mtime as the
official one. When inode attr is timeout or invalidated, kernel has chance
to see size and c/mtime modified by others.

Together with another patch [2], a FUSE daemon is able to implement
close-to-open (CTO) consistency like what is done in NFS clients.

[1] https://lore.kernel.org/linux-fsdevel/Ymfu8fGbfYi4FxQ4@miu.piliscsaba.redhat.com
[2] https://lore.kernel.org/linux-fsdevel/20220608104202.19461-1-zhangjiachen.jaycee@bytedance.com/

Suggested-by: Miklos Szeredi <mszeredi@...hat.com>
Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
---
 fs/fuse/file.c            | 17 +++++++++++++++
 fs/fuse/fuse_i.h          |  3 +++
 fs/fuse/inode.c           | 44 +++++++++++++++++++++++++++++++++++++--
 include/uapi/linux/fuse.h |  5 +++++
 4 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 9b64e2ff1c96..35bdc7af8468 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1829,6 +1829,15 @@ static void fuse_writepage_end(struct fuse_mount *fm, struct fuse_args *args,
 		 */
 		fuse_send_writepage(fm, next, inarg->offset + inarg->size);
 	}
+
+	if (fc->writeback_cache_v2)
+		fi->attr_version = atomic64_inc_return(&fc->attr_version);
+	/*
+	 * Ensure attr_version increases before the page is move out of the
+	 * writepages rb-tree.
+	 */
+	smp_mb();
+
 	fi->writectr--;
 	fuse_writepage_finish(fm, wpa);
 	spin_unlock(&fi->lock);
@@ -1858,10 +1867,18 @@ static struct fuse_file *fuse_write_file_get(struct fuse_inode *fi)
 
 int fuse_write_inode(struct inode *inode, struct writeback_control *wbc)
 {
+	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_file *ff;
 	int err;
 
+	/*
+	 * Kernel c/mtime should not be updated to the server in the
+	 * writeback_cache_v2 mode as server c/mtime are official.
+	 */
+	if (fc->writeback_cache_v2)
+		return 0;
+
 	/*
 	 * Inode is always written before the last reference is dropped and
 	 * hence this should not be reached from reclaim.
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 488b460e046f..47de36146fb8 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -654,6 +654,9 @@ struct fuse_conn {
 	/* show legacy mount options */
 	unsigned int legacy_opts_show:1;
 
+	/* Improved writeback cache policy */
+	unsigned writeback_cache_v2:1;
+
 	/*
 	 * fs kills suid/sgid/cap on write/chown/trunc. suid is killed on
 	 * write/trunc only if caller did not have CAP_FSETID.  sgid is killed
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 8c0665c5dff8..2d5fa82b08b6 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -237,14 +237,41 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 	u32 cache_mask;
 	loff_t oldsize;
 	struct timespec64 old_mtime;
+	bool try_wb_update = false;
+
+	if (fc->writeback_cache_v2 && S_ISREG(inode->i_mode)) {
+		inode_lock(inode);
+		try_wb_update = true;
+	}
 
 	spin_lock(&fi->lock);
 	/*
 	 * In case of writeback_cache enabled, writes update mtime, ctime and
 	 * may update i_size.  In these cases trust the cached value in the
 	 * inode.
+	 *
+	 * In writeback_cache_v2 mode, if all the following conditions are met,
+	 * then we allow the attributes to be refreshed:
+	 *
+	 * - inode is not in the process of being written (I_SYNC)
+	 * - inode has no dirty pages (I_DIRTY_PAGES)
+	 * - inode data-related attributes are clean (I_DIRTY_DATASYNC)
+	 * - inode does not have any page writeback in progress
+	 *
+	 * Note: checking PAGECACHE_TAG_WRITEBACK is not sufficient in fuse,
+	 * since inode can appear to have no PageWriteback pages, yet still have
+	 * outstanding write request.
 	 */
 	cache_mask = fuse_get_cache_mask(inode);
+	if (try_wb_update && !(inode->i_state & (I_DIRTY_PAGES | I_SYNC |
+	    I_DIRTY_DATASYNC)) && RB_EMPTY_ROOT(&fi->writepages))
+		cache_mask &= ~(STATX_MTIME | STATX_CTIME | STATX_SIZE);
+	/*
+	 * Ensure the ordering of cleanness checking and following attr_version
+	 * comparison.
+	 */
+	smp_mb();
+
 	if (cache_mask & STATX_SIZE)
 		attr->size = i_size_read(inode);
 
@@ -283,7 +310,13 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 			truncate_pagecache(inode, attr->size);
 			if (!fc->explicit_inval_data)
 				inval = true;
-		} else if (fc->auto_inval_data) {
+		} else if (!fc->writeback_cache_v2 && fc->auto_inval_data) {
+			/*
+			 * When fc->writeback_cache_v2 is set, the old_mtime
+			 * can be generated by kernel and must not equal to
+			 * new_mtime generated by server. So skip in such
+			 * case.
+			 */
 			struct timespec64 new_mtime = {
 				.tv_sec = attr->mtime,
 				.tv_nsec = attr->mtimensec,
@@ -303,6 +336,9 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 
 	if (IS_ENABLED(CONFIG_FUSE_DAX))
 		fuse_dax_dontcache(inode, attr->flags);
+
+	if (try_wb_update)
+		inode_unlock(inode);
 }
 
 static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
@@ -1153,6 +1189,10 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 				fc->async_dio = 1;
 			if (flags & FUSE_WRITEBACK_CACHE)
 				fc->writeback_cache = 1;
+			if (flags & FUSE_WRITEBACK_CACHE_V2) {
+				fc->writeback_cache = 1;
+				fc->writeback_cache_v2 = 1;
+			}
 			if (flags & FUSE_PARALLEL_DIROPS)
 				fc->parallel_dirops = 1;
 			if (flags & FUSE_HANDLE_KILLPRIV)
@@ -1234,7 +1274,7 @@ void fuse_send_init(struct fuse_mount *fm)
 		FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
 		FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
 		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
-		FUSE_SECURITY_CTX;
+		FUSE_SECURITY_CTX | FUSE_WRITEBACK_CACHE_V2;
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		flags |= FUSE_MAP_ALIGNMENT;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index d6ccee961891..b474763bcf59 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -194,6 +194,7 @@
  *  - add FUSE_SECURITY_CTX init flag
  *  - add security context to create, mkdir, symlink, and mknod requests
  *  - add FUSE_HAS_INODE_DAX, FUSE_ATTR_DAX
+ *  - add FUSE_WRITEBACK_CACHE_V2 init flag
  */
 
 #ifndef _LINUX_FUSE_H
@@ -353,6 +354,9 @@ struct fuse_file_lock {
  * FUSE_SECURITY_CTX:	add security context to create, mkdir, symlink, and
  *			mknod
  * FUSE_HAS_INODE_DAX:  use per inode DAX
+ * FUSE_WRITEBACK_CACHE_V2:
+ *			allow time/size to be refreshed if no pending write
+ *			c/mtime not updated from kernel to server
  */
 #define FUSE_ASYNC_READ		(1 << 0)
 #define FUSE_POSIX_LOCKS	(1 << 1)
@@ -389,6 +393,7 @@ struct fuse_file_lock {
 /* bits 32..63 get shifted down 32 bits into the flags2 field */
 #define FUSE_SECURITY_CTX	(1ULL << 32)
 #define FUSE_HAS_INODE_DAX	(1ULL << 33)
+#define FUSE_WRITEBACK_CACHE_V2	(1ULL << 34)
 
 /**
  * CUSE INIT request/reply flags
-- 
2.20.1

Powered by blists - more mailing lists