Re: [PATCH] fuse: invalidate the correct range after O_APPEND direct write

[Bernd Schubert]

On 8/19/26 11:07, Baokun Li wrote:
> fuse_direct_write_iter() captures pos before generic_write_checks(),
> which moves ki_pos to EOF for O_APPEND writes:
> 
>   fuse_direct_write_iter()
>   {
>       pos = iocb->ki_pos;           /* 0 (user-supplied)       */
>       generic_write_checks();       /* ki_pos -> EOF           */
>       fuse_direct_io();             /* writes at EOF, correct  */
>       invalidate(pos, pos + res);   /* [0, res) -- wrong       */
>   }
> 
> The post-write invalidation targets a stale range instead of the
> actual written range at EOF.
> 
> This can cause data inconsistency when the file size is not
> page-aligned.  The tail page straddling EOF has a valid portion
> before EOF that concurrent readers can fault back in during the
> DIO write window:
> 
>   Tail page (file size X not page-aligned):
> 
>     page_start         X (EOF)   page_end
>     |--- valid data ----|-- stale --|
> 
>   CPU0 (O_APPEND DIO writer)    CPU1 (buffered reader)
>   --------------------------    ----------------------
>   invalidate [X, X+len)
>     tail page evicted
>   FUSE_WRITE in flight ...
>                                 read [page_start, X)
>                                   tail page re-faulted
>                                   [X, page_end) = stale
>   FUSE_WRITE completes
>   i_size = X + len
>   invalidate [0, len)  <- WRONG
>     tail page still cached
>                                 read [X, X+len)
>                                   hits stale tail page
>                                   returns old data
> 
> Fix by reading pos back from iocb->ki_pos after generic_write_checks(),
> as generic_file_direct_write() does.
> 
> Also fix a typo in the comment ("may have" -> "may have competed").
> 
> Fixes: 2b0408d0284f ("fuse: invalidate page cache after DIO and async DIO writes")
> Signed-off-by: Baokun Li <libaokun@xxxxxxxxxxxxxxxxx>
> ---
>  fs/fuse/file.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 45ebd1b15874..d12a9fdf770e 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1789,13 +1789,14 @@ static ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  {
>  	struct inode *inode = file_inode(iocb->ki_filp);
>  	struct address_space *mapping = inode->i_mapping;
> -	loff_t pos = iocb->ki_pos;
>  	ssize_t res;
>  	bool exclusive;
>  
>  	fuse_dio_lock(iocb, from, &exclusive);
>  	res = generic_write_checks(iocb, from);
>  	if (res > 0) {
> +		loff_t pos = iocb->ki_pos;
> +
>  		task_io_account_write(res);
>  		if (!is_sync_kiocb(iocb)) {
>  			res = fuse_direct_IO(iocb, from);
> @@ -1810,7 +1811,7 @@ static ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  			/*
>  			 * As in generic_file_direct_write(), invalidate after
>  			 * write, to invalidate read-ahead cache that may have
> -			 * with the write.
> +			 * competed with the write.
>  			 */
>  			invalidate_inode_pages2_range(mapping,
>  				pos >> PAGE_SHIFT,

Good catch! Looks like there is a missing xfstest scenario.

Reviewed-by: Bernd Schubert <bernd@xxxxxxxxxxx>