[PATCH] fuse: invalidate the page cache after direct write

[PATCH] fuse: invalidate the page cache after direct write

[Jingbo Xu]

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().

Signed-off-by: Jingbo Xu <jefflexu@linux.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);
-- 
2.19.1.6.gb485710b

Re: [PATCH] fuse: invalidate the page cache after direct write

[Bernd Schubert]

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@linux.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@ddn.com>


Thanks,
Bernd

Re: [PATCH] fuse: invalidate the page cache after direct write

[Jingbo Xu]

On 1/9/26 12:16 AM, Bernd Schubert wrote:
> 
> 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
> 

Actually I think it's more complicated:

```
/*
 * without FOPEN_DIRECT_IO generic_file_direct_write() does the
 * invalidation for synchronous write.
 */
if (!err && written && mapping->nrpages &&
    ((ff->open_flags & FOPEN_DIRECT_IO) || !io->blocking)) {
```

I will send v2 soon if you feel good about the above diff.

-- 
Thanks,
Jingbo

Re: [PATCH] fuse: invalidate the page cache after direct write

[Jingbo Xu]

Hi Bernd,

On 1/9/26 10:13 AM, Jingbo Xu wrote:
> 
> 
> On 1/9/26 12:16 AM, Bernd Schubert wrote:
>>
>> 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
>>
> 
> Actually I think it's more complicated:
> 
> ```
> /*
>  * without FOPEN_DIRECT_IO generic_file_direct_write() does the
>  * invalidation for synchronous write.
>  */
> if (!err && written && mapping->nrpages &&
>     ((ff->open_flags & FOPEN_DIRECT_IO) || !io->blocking)) {
> ```
> 
> I will send v2 soon if you feel good about the above diff.
> 

Sorry I just realized that the invalidation for asynchronous write will
be done in fuse_aio_complete() of v1 patch.

I will apply the diff you suggested and send v2 soon.

-- 
Thanks,
Jingbo