Re: [PATCH] block/file-posix.c: No need to sync data when aio + dio is enabled

[zhoushl]

Hi Kevin：

I’m sorry for missing commit message, next time I will be careful. When 

the application in guest vm execute fsync, qemu will execute fsync too. 

But when aio + dio is enabled, pagecache is bypassed and we could assure 

the data is on disk(at least on the disk cache), so there is no needto sync

anymore.  For example, we could execute the following python script in vm:

#!/usr/bin/python

import os

fo = os.open(“test.txt”, os.O_RDWR|os.O_CREAT)

while True:

      os.write(fo, “123\n”)

      os.fsync(fo)

os.closed(fo)

In this case, each write will take an fsync operation, which will search the

dirty page in pagecache, force flushing the metadata and data into disk, 

which is often useless and waste IO resource and maybe will cause write 

amplification in filesystem.

On Apr 27, 2023, at 20:23, Kevin Wolf <kwolf@redhat.com> wrote:

Am 18.04.2023 um 08:05 hat zhoushlu@163.com geschrieben:

From: zhoushl <zhoushl@inspur.com>

Signed-off-by: zhoushl <zhoushl@inspur.com>

block/file-posix.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 5760cf22d1..fe9568947c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1399,6 +1399,12 @@ static int handle_aiocb_flush(void *opaque)
        return -s->page_cache_inconsistent;
    }

+#ifdef CONFIG_LINUX_AIO
+    if (s->use_linux_aio && (s->open_flags & O_DIRECT)) {
+        return 0;
+    }
+#endif
+
    ret = qemu_fdatasync(aiocb->aio_fildes);
    if (ret == -1) {
        trace_file_flush_fdatasync_failed(errno);

As you didn't care to write a commit message that explains why you think
you don't need flushes in this specific case, I can't say much more than
that it is wrong. O_DIRECT bypasses the kernel page cache, but not any
lower level caches such as a volatile disk cache. So you still need to
flush before you can be sure that data is stable on disk.

Kevin