linux-kernel - [RFCv2] fuse: Sync attributes when the inode is clean in writeback mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Wed, 20 Oct 2021 00:02:51 +0800
From:   Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
To:     miklos@...redi.hu
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        xieyongji@...edance.com,
        Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
Subject: [RFCv2] fuse: Sync attributes when the inode is clean in writeback mode

This is the RFCv2 version of this patch. Compared with the previous
version, v2 based on the latest fuse for-next branch commit 83d9bf94c077
("fuse: add cache_mask"). With the new cleanups and fuse interfaces, this
commit is more clean compared with v1 [1].

The propose of this commit is still to relax the c/mtime and size updating
for writeback_cache mode. Because when writeback cache is enabled, kernel
will locally maintain the attributes, and never trusts any server side
attribute changes. This limitaion is too strict in some use cases. For
example, if a file is not actively wrote from the fuse mount in writeback
mode, the writeback related caches should be clean, and the user may expect
to see the new size changed from the server side. This commit tires to
relax the limitation.

If there is no dirty page of an fuse inode, update its ctime, atime and
size even in writeback_cache mode. The page cache cleaness checking is
based on a new fuse writeback helper (fuse_file_is_writeback_locked) and a
mm/filemap helper introduced in a recent commit 63135aa3866d ("mm: provide
filemap_range_needs_writeback() helper").

[1] https://patchwork.kernel.org/project/linux-fsdevel/patch/20211012145558.19137-1-zhangjiachen.jaycee@bytedance.com/

Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
---
 fs/fuse/file.c   | 21 +++++++++++++++++++++
 fs/fuse/fuse_i.h |  1 +
 fs/fuse/inode.c  | 10 ++++++++++
 3 files changed, 32 insertions(+)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index bc450daf27a2..635624d65c76 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -412,6 +412,27 @@ static struct fuse_writepage_args *fuse_find_writeback(struct fuse_inode *fi,
 	return NULL;
 }
 
+/*
+ * Check if any page of this file is under writeback.
+ *
+ * The fuse_inode lock should be held by the caller.
+ */
+bool fuse_file_is_writeback_locked(struct inode *inode)
+{
+	struct fuse_inode *fi = get_fuse_inode(inode);
+	pgoff_t idx_from = 0;
+	pgoff_t idx_to = 0;
+	size_t fuse_inode_size = i_size_read(inode);
+	bool found;
+
+	if (fuse_inode_size > 0)
+		idx_to = (fuse_inode_size - 1) >> PAGE_SHIFT;
+
+	found = fuse_find_writeback(fi, idx_from, idx_to);
+
+	return found;
+}
+
 /*
  * Check if any page in a range is under writeback
  *
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 119459ad9e50..dbb2a5ae99b6 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1288,5 +1288,6 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 				 unsigned int open_flags, bool isdir);
 void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 		       unsigned int open_flags, fl_owner_t id, bool isdir);
+bool fuse_file_is_writeback_locked(struct inode *inode);
 
 #endif /* _FS_FUSE_I_H */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index d1620bf01246..72316ca5ecca 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -23,6 +23,7 @@
 #include <linux/exportfs.h>
 #include <linux/posix_acl.h>
 #include <linux/pid_namespace.h>
+#include <linux/fs.h>
 
 MODULE_AUTHOR("Miklos Szeredi <miklos@...redi.hu>");
 MODULE_DESCRIPTION("Filesystem in Userspace");
@@ -244,8 +245,17 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 	 * 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.
+	 *
+	 * One expection is if the page cache is clean and there is no in-flight
+	 * fuse writeback request. The c/mtime and file size are allowed to be
+	 * updated from server, so clear cache_mask in this case.
 	 */
 	cache_mask = fuse_get_cache_mask(inode);
+	if (!filemap_range_needs_writeback(inode->i_mapping, 0,
+	    i_size_read(inode) - 1) && !fuse_file_is_writeback_locked(inode)) {
+		cache_mask = 0;
+	}
+
 	if (cache_mask & STATX_SIZE)
 		attr->size = i_size_read(inode);
 
-- 
2.20.1