Re: [PATCH 4/5] [RFC] fuse: Set and use IOCB_DIRECT when FOPEN_DIRECT_IO is set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/29/23 15:26, Bernd Schubert wrote:


On 8/29/23 15:08, Bernd Schubert wrote:


On 8/28/23 17:05, Miklos Szeredi wrote:
On Mon, 28 Aug 2023 at 16:48, Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote:

On 8/28/23 13:59, Miklos Szeredi wrote:
On Thu, 24 Aug 2023 at 17:07, Bernd Schubert <bschubert@xxxxxxx> wrote:

-               if (!is_sync_kiocb(iocb) && iocb->ki_flags & IOCB_DIRECT) {
-                       res = fuse_direct_IO(iocb, from);
-               } else {
-                       res = fuse_direct_io(&io, from, &iocb->ki_pos,
-                                            FUSE_DIO_WRITE);
-                       fuse_write_update_attr(inode, iocb->ki_pos, res);

While I think this is correct, I'd really like if the code to be
replaced and the replacement are at least somewhat comparable.

Sorry, I have a hard to time to understand "I'd really like if the code
to be replaced".

What I meant is that generic_file_direct_write() is not an obvious
replacement for the  above lines of code.

The reason is that fuse_direct_IO() is handling the sync and async
cases in one function, while the above splits handling it based on
IOCB_DIRECT (which is now lost) and is_sync_kiocb(iocb).  If it's okay
to lose IOCB_DIRECT then what's the explanation for the above
condition?  It could be historic garbage, but we still need to
understand what is exactly happening.

While checking all code path again, I found an additional difference, which I had missed before. FOPEN_DIRECT_IO will now act on ff->fm->fc->async_dio when is is_sync_kiocb(iocb) is set.

Do you think that is a problem? If so, I could fix it in fuse_direct_IO.

What I mean is something like this

+    /* FOPEN_DIRECT_IO historically does not use async for blocking O_DIRECT */
+    if (ff->open_flags & FOPEN_DIRECT_IO) {
+        if (!is_sync_kiocb(iocb) && ff->iocb_direct) {
+            /* no change */
+        } else {
+            io->async = 0;
+        }
+    }

Besides that it could use file->f_flags & O_DIRECT, I guess we can keep async. It relates to commit 23c94e1cdcbf5953cd380555d0781caa42311870, which actually introduced async for FOPEN_DIRECT_IO. I'm just going to add it to the commit message.


Thanks,
Bernd



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux