| 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: <b6cfc411-8d2e-45c2-b237-e79f71016bbb@bsbernd.com>
Date: Thu, 8 Jan 2026 17:16:05 +0100
From: Bernd Schubert <bernd@...ernd.com>
To: Jingbo Xu <jefflexu@...ux.alibaba.com>, miklos@...redi.hu,
linux-fsdevel@...r.kernel.org, bschubert@....com
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fuse: invalidate the page cache after direct write
Hi Jingbo,
thanks a lot, I have an internal ticket assigned for that, but never
found the time to look into it.
On 1/6/26 08:52, Jingbo Xu wrote:
> This fixes xfstests generic/451 (for both O_DIRECT and FOPEN_DIRECT_IO
> direct write).
>
> Commit b359af8275a9 ("fuse: Invalidate the page cache after
> FOPEN_DIRECT_IO write") tries to fix the similar issue for
> FOPEN_DIRECT_IO write, which can be reproduced by xfstests generic/209.
> It only fixes the issue for synchronous direct write, while omitting
> the case for asynchronous direct write (exactly targeted by
> generic/451).
>
> While for O_DIRECT direct write, it's somewhat more complicated. For
> synchronous direct write, generic_file_direct_write() will invalidate
> the page cache after the write, and thus it can pass generic/209. While
> for asynchronous direct write, the invalidation in
> generic_file_direct_write() is bypassed since the invalidation shall be
> done when the asynchronous IO completes. This is omitted in FUSE and
> generic/451 fails whereby.
>
> Fix this by conveying the invalidation in both sync and async
> (FUSE_ASYNC_DIO) request submission. The only side effect is that
> there's a redundant invalidation for synchronous direct write when
> FUSE_ASYNC_DIO is disabled (in fuse_send_write()), given the same
> range has been invalidated once in generic_file_direct_write().
The side effect should only come without FOPEN_DIRECT_IO. Could you add
this diff to avoid it?
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index d6ae3b4652f8..c04296316a82 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1177,7 +1177,13 @@ static ssize_t fuse_send_write(struct fuse_io_args *ia, loff_t pos,
written = ia->write.out.size;
if (!err && written > count)
err = -EIO;
- if (!err && written && mapping->nrpages) {
+
+ /*
+ * without FOPEN_DIRECT_IO generic_file_direct_write() does the
+ * invalidation for us
+ */
+ if (!err && written && mapping->nrpages &&
+ (ff->open_flags & FOPEN_DIRECT_IO)) {
/*
* As in generic_file_direct_write(), invalidate after the
* write, to invalidate read-ahead cache that may have competed
>
> Signed-off-by: Jingbo Xu <jefflexu@...ux.alibaba.com>
> ---
> fs/fuse/file.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 01bc894e9c2b..d6ae3b4652f8 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -667,6 +667,18 @@ static void fuse_aio_complete(struct fuse_io_priv *io, int err, ssize_t pos)
> struct inode *inode = file_inode(io->iocb->ki_filp);
> struct fuse_conn *fc = get_fuse_conn(inode);
> struct fuse_inode *fi = get_fuse_inode(inode);
> + struct address_space *mapping = io->iocb->ki_filp->f_mapping;
> +
> + /*
> + * As in generic_file_direct_write(), invalidate after the
> + * write, to invalidate read-ahead cache that may have competed
> + * with the write.
> + */
> + if (io->write && res && mapping->nrpages) {
> + invalidate_inode_pages2_range(mapping,
> + io->offset >> PAGE_SHIFT,
> + (io->offset + res - 1) >> PAGE_SHIFT);
> + }
>
> spin_lock(&fi->lock);
> fi->attr_version = atomic64_inc_return(&fc->attr_version);
> @@ -1144,9 +1156,11 @@ static ssize_t fuse_send_write(struct fuse_io_args *ia, loff_t pos,
> {
> struct kiocb *iocb = ia->io->iocb;
> struct file *file = iocb->ki_filp;
> + struct address_space *mapping = file->f_mapping;
> struct fuse_file *ff = file->private_data;
> struct fuse_mount *fm = ff->fm;
> struct fuse_write_in *inarg = &ia->write.in;
> + ssize_t written;
> ssize_t err;
>
> fuse_write_args_fill(ia, ff, pos, count);
> @@ -1160,10 +1174,20 @@ static ssize_t fuse_send_write(struct fuse_io_args *ia, loff_t pos,
> return fuse_async_req_send(fm, ia, count);
>
> err = fuse_simple_request(fm, &ia->ap.args);
> - if (!err && ia->write.out.size > count)
> + written = ia->write.out.size;
> + if (!err && written > count)
> err = -EIO;
> + if (!err && written && mapping->nrpages) {
> + /*
> + * As in generic_file_direct_write(), invalidate after the
> + * write, to invalidate read-ahead cache that may have competed
> + * with the write.
> + */
> + invalidate_inode_pages2_range(mapping, pos >> PAGE_SHIFT,
> + (pos + written - 1) >> PAGE_SHIFT);
> + }
>
> - return err ?: ia->write.out.size;
> + return err ?: written;
> }
>
> bool fuse_write_update_attr(struct inode *inode, loff_t pos, ssize_t written)
> @@ -1738,15 +1762,6 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
> if (res > 0)
> *ppos = pos;
>
> - if (res > 0 && write && fopen_direct_io) {
> - /*
> - * As in generic_file_direct_write(), invalidate after the
> - * write, to invalidate read-ahead cache that may have competed
> - * with the write.
> - */
> - invalidate_inode_pages2_range(mapping, idx_from, idx_to);
> - }
> -
> return res > 0 ? res : err;
> }
> EXPORT_SYMBOL_GPL(fuse_direct_io);
Assuming you apply the diff above, feel free to add for v2
Reviewed-by: Bernd Schubert <bschubert@....com>
Thanks,
Bernd
Powered by blists - more mailing lists