Re: [PATCH 1/2] [RFC for fuse-next ] fuse: DIO writes always use the same code path

[Bernd Schubert <bernd.schubert@fastmail.fm>]

On 8/22/23 11:53, Miklos Szeredi wrote:
> On Mon, 21 Aug 2023 at 19:48, Bernd Schubert <bschubert@ddn.com> wrote:
>>
>> There were two code paths direct-io writes could
>> take. When daemon/server side did not set FOPEN_DIRECT_IO
>>      fuse_cache_write_iter -> direct_write_fallback
>> and with FOPEN_DIRECT_IO being set
>>      fuse_direct_write_iter
>>
>> Advantage of fuse_direct_write_iter is that it has optimizations
>> for parallel DIO writes - it might only take a shared inode lock,
>> instead of the exclusive lock.
>>
>> With commits b5a2a3a0b776/80e4f25262f9 the fuse_direct_write_iter
>> path also handles concurrent page IO (dirty flush and page release),
>> just the condition on fc->direct_io_relax had to be removed.
>>
>> Performance wise this basically gives the same improvements as
>> commit 153524053bbb, just O_DIRECT is sufficient, without the need
>> that server side sets FOPEN_DIRECT_IO
>> (it has to set FOPEN_PARALLEL_DIRECT_WRITES), though.
> 
> Consolidating the various direct IO paths would be really nice.
> 
> Problem is that fuse_direct_write_iter() lacks some code from
> generic_file_direct_write() and also completely lacks
> direct_write_fallback().   So more thought needs to go into this.

Thanks for looking at it! Hmm, right, I see. I guess at least
direct_write_fallback() should be done for the new relaxed
mmap mode.

Entirely duplicating generic_file_direct_write()
to generic_file_direct_write doesn't seem to be nice either.

Regarding the inode lock, it might be easier to
change fuse_cache_write_iter() to a shared lock, although that
does not help when fc->writeback_cache is enabled, which has yet
another code path. Although I'm not sure that is needed
direct IO. For the start, what do you think about

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 1cdb6327511e..b1b9f2b9a37d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1307,7 +1307,7 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
         ssize_t err;
         struct fuse_conn *fc = get_fuse_conn(inode);
  
-       if (fc->writeback_cache) {
+       if (fc->writeback_cache && !(iocb->ki_flags & IOCB_DIRECT)) {
                 /* Update size (EOF optimization) and mode (SUID clearing) */
                 err = fuse_update_attributes(mapping->host, file,
                                              STATX_SIZE | STATX_MODE);


Thanks,
Bernd