Re: [RFC] [PATCH] fuse: DIO writes always use the same code path

[Hao Xu <hao.xu@linux.dev>]

Hi Bernd,

On 7/5/23 18:23, Bernd Schubert wrote:
> From: Bernd Schubert <bschubert@ddn.com>
> 
> In commit 153524053bbb0d27bb2e0be36d1b46862e9ce74c DIO
> writes can be handled in parallel, as long as the file
> is not extended. So far this only works when daemon/server
> side set FOPEN_DIRECT_IO and FOPEN_PARALLEL_DIRECT_WRITES,
> but O_DIRECT (iocb->ki_flags & IOCB_DIRECT) went another
> code path that doesn't have the parallel DIO write
> optimization.
> Given that fuse_direct_write_iter has to handle page writes
> and invalidation anyway (for mmap), the DIO handler in
> fuse_cache_write_iter() is removed and DIO writes are now
> only handled by fuse_direct_write_iter().
> 
> Note: Correctness of this patch depends on a non-merged
> series from Hao Xu <hao.xu@linux.dev>
> ( fuse: add a new fuse init flag to relax restrictions in no cache mode)
> ---
>   fs/fuse/file.c |   38 +++++---------------------------------
>   1 file changed, 5 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 89d97f6188e0..1490329b536c 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1337,11 +1337,9 @@ static ssize_t fuse_cache_write_iter(struct kiocb 
> *iocb, struct iov_iter *from)
>       struct file *file = iocb->ki_filp;
>       struct address_space *mapping = file->f_mapping;
>       ssize_t written = 0;
> -    ssize_t written_buffered = 0;
>       struct inode *inode = mapping->host;
>       ssize_t err;
>       struct fuse_conn *fc = get_fuse_conn(inode);
> -    loff_t endbyte = 0;
> 
>       if (fc->writeback_cache) {
>           /* Update size (EOF optimization) and mode (SUID clearing) */
> @@ -1377,37 +1375,10 @@ static ssize_t fuse_cache_write_iter(struct 
> kiocb *iocb, struct iov_iter *from)
>       if (err)
>           goto out;
> 
> -    if (iocb->ki_flags & IOCB_DIRECT) {
> -        loff_t pos = iocb->ki_pos;
> -        written = generic_file_direct_write(iocb, from);
> -        if (written < 0 || !iov_iter_count(from))
> -            goto out;
> -
> -        pos += written;
> -
> -        written_buffered = fuse_perform_write(iocb, mapping, from, pos);
> -        if (written_buffered < 0) {
> -            err = written_buffered;
> -            goto out;
> -        }
> -        endbyte = pos + written_buffered - 1;
> -
> -        err = filemap_write_and_wait_range(file->f_mapping, pos,
> -                           endbyte);
> -        if (err)
> -            goto out;
> -
> -        invalidate_mapping_pages(file->f_mapping,
> -                     pos >> PAGE_SHIFT,
> -                     endbyte >> PAGE_SHIFT);
> +    written = fuse_perform_write(iocb, mapping, from, iocb->ki_pos);
> +    if (written >= 0)
> +        iocb->ki_pos += written;
> 
> -        written += written_buffered;
> -        iocb->ki_pos = pos + written_buffered;
> -    } else {
> -        written = fuse_perform_write(iocb, mapping, from, iocb->ki_pos);
> -        if (written >= 0)
> -            iocb->ki_pos += written;
> -    }
>   out:
>       current->backing_dev_info = NULL;
>       inode_unlock(inode);
> @@ -1691,7 +1662,8 @@ static ssize_t fuse_file_write_iter(struct kiocb 
> *iocb, struct iov_iter *from)
>       if (FUSE_IS_DAX(inode))
>           return fuse_dax_write_iter(iocb, from);
> 
> -    if (!(ff->open_flags & FOPEN_DIRECT_IO))
> +    if (!(ff->open_flags & FOPEN_DIRECT_IO) &&
> +        !(iocb->ki_flags & IOCB_DIRECT))
>           return fuse_cache_write_iter(iocb, from);
>       else
>           return fuse_direct_write_iter(iocb, from);
> 

For normal direct io(IOCB_DIRECT set, FOPEN_DIRECT_IO not set), it now
goes to fuse_direct_write_iter() but the thing is the previous patchset
I sent adds page flush and invalidation in FOPEN_DIRECT_IO
and/or fc->direct_io_relax, so I guess this part(flush and invalidation)
is not included in the normal direct io code path.

Regards,
Hao