| 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
| ||
|
Message-Id: <20211012145558.19137-1-zhangjiachen.jaycee@bytedance.com>
Date: Tue, 12 Oct 2021 22:55:58 +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: [RFC] fuse: Sync attributes when the inode is clean in writeback mode
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 recently (detials see commit 63135aa3866d
("mm: provide filemap_range_needs_writeback() helper") ).
Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
---
fs/fuse/dir.c | 3 ++-
fs/fuse/file.c | 21 +++++++++++++++++++++
fs/fuse/fuse_i.h | 3 ++-
fs/fuse/inode.c | 36 +++++++++++++++++++++++++++++++-----
4 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 915493613a31..2ca68aaeb26a 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1678,8 +1678,9 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
/* FIXME: clear I_DIRTY_SYNC? */
}
+ /* Don't update the c/mtime unconditionally when we trust_local_cmtime. */
fuse_change_attributes_common(inode, &outarg.attr,
- attr_timeout(&outarg));
+ attr_timeout(&outarg), !trust_local_cmtime);
oldsize = inode->i_size;
/* see the comment in fuse_change_attributes() */
if (!is_wb || is_truncate || !S_ISREG(inode->i_mode))
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index bb3321098b69..108ab5106b52 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -413,6 +413,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 0b2673a58cf5..0429f39de36a 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1031,7 +1031,7 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
u64 attr_valid, u64 attr_version);
void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
- u64 attr_valid);
+ u64 attr_valid, bool update_cmtime);
/**
* Initialize the client device
@@ -1286,5 +1286,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 8068c666e1e6..371478f29425 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");
@@ -164,7 +165,7 @@ static ino_t fuse_squash_ino(u64 ino64)
}
void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
- u64 attr_valid)
+ u64 attr_valid, bool update_cmtime)
{
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_inode *fi = get_fuse_inode(inode);
@@ -183,8 +184,7 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
inode->i_blocks = attr->blocks;
inode->i_atime.tv_sec = attr->atime;
inode->i_atime.tv_nsec = attr->atimensec;
- /* mtime from server may be stale due to local buffered write */
- if (!fc->writeback_cache || !S_ISREG(inode->i_mode)) {
+ if (update_cmtime) {
inode->i_mtime.tv_sec = attr->mtime;
inode->i_mtime.tv_nsec = attr->mtimensec;
inode->i_ctime.tv_sec = attr->ctime;
@@ -226,8 +226,22 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
bool is_wb = fc->writeback_cache;
loff_t oldsize;
struct timespec64 old_mtime;
+ bool is_dirty = false;
spin_lock(&fi->lock);
+
+ /*
+ * is_dirty will be true if:
+ * 1. Writeback cache is enabled,
+ * 2. the file is a regular one, and
+ * 3. at least one dirty page in the inode mapping, or at least
+ * one fuse writeback page is in-flight.
+ */
+ is_dirty = is_wb && S_ISREG(inode->i_mode) &&
+ (filemap_range_needs_writeback(inode->i_mapping, 0,
+ i_size_read(inode) - 1) ||
+ fuse_file_is_writeback_locked(inode));
+
if ((attr_version != 0 && fi->attr_version > attr_version) ||
test_bit(FUSE_I_SIZE_UNSTABLE, &fi->state)) {
spin_unlock(&fi->lock);
@@ -235,7 +249,11 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
}
old_mtime = inode->i_mtime;
- fuse_change_attributes_common(inode, attr, attr_valid);
+ /*
+ * mtime from server may be stale due to local buffered write, so
+ * don't update c/mtime when is_dirty is true.
+ */
+ fuse_change_attributes_common(inode, attr, attr_valid, !is_dirty);
oldsize = inode->i_size;
/*
@@ -243,8 +261,16 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
* extend local i_size without keeping userspace server in sync. So,
* attr->size coming from server can be stale. We cannot trust it.
*/
- if (!is_wb || !S_ISREG(inode->i_mode))
+ if (!is_dirty) {
i_size_write(inode, attr->size);
+ /*
+ * If writeback cache is enabled, the truncated_pagecache should
+ * be executed with fi->lock held, in case of racing with write
+ * operations.
+ */
+ if (is_wb && S_ISREG(inode->i_mode) && (oldsize != attr->size))
+ truncate_pagecache(inode, attr->size);
+ }
spin_unlock(&fi->lock);
if (!is_wb && S_ISREG(inode->i_mode)) {
--
2.20.1
Powered by blists - more mailing lists