The following changes since commit 825b96dbcee23d134b691fc75618b59c5f53da32:

Merge tag 'migration-20250310-pull-request' of https://gitlab.com/farosas/qemu into staging (2025-03-11 09:32:07 +0800)

for you to fetch changes up to a93c04f3cbe690877b3297a9df4767aa811fcd97:

virtio-scsi: only expose cmd vqs via iothread-vq-mapping (2025-03-11 15:49:22 +0100)

- virtio-scsi: add iothread-vq-mapping parameter

- Improve writethrough performance

- Fix missing zero init in bdrv_snapshot_goto()

- Code cleanup and iotests fixes

Kevin Wolf (8):

block: Remove unused blk_op_is_blocked()

block: Zero block driver state before reopening

file-posix: Support FUA writes

block/io: Ignore FUA with cache.no-flush=on

aio: Create AioPolledEvent

aio-posix: Factor out adjust_polling_time()

aio-posix: Separate AioPolledEvent per AioHandler

aio-posix: Adjust polling time also for new handlers

Stefan Hajnoczi (13):

scsi-disk: drop unused SCSIDiskState->bh field

dma: use current AioContext for dma_blk_io()

scsi: track per-SCSIRequest AioContext

scsi: introduce requests_lock

virtio-scsi: introduce event and ctrl virtqueue locks

virtio-scsi: protect events_dropped field

virtio-scsi: perform TMFs in appropriate AioContexts

virtio-blk: extract cleanup_iothread_vq_mapping() function

virtio-blk: tidy up iothread_vq_mapping functions

virtio: extract iothread-vq-mapping.h API

virtio-scsi: add iothread-vq-mapping parameter

virtio-scsi: handle ctrl virtqueue in main loop

virtio-scsi: only expose cmd vqs via iothread-vq-mapping

Thomas Huth (1):

iotests: Limit qsd-migrate to working formats

include/block/aio.h | 5 +-

include/block/raw-aio.h | 8 +-

include/hw/scsi/scsi.h | 8 +-

include/hw/virtio/iothread-vq-mapping.h | 45 +++

include/hw/virtio/virtio-scsi.h | 15 +-

include/system/block-backend-global-state.h | 1 -

include/system/dma.h | 3 +-

util/aio-posix.h | 1 +

block/block-backend.c | 12 -

block/file-posix.c | 26 +-

block/io.c | 4 +

block/io_uring.c | 13 +-

block/linux-aio.c | 24 +-

block/snapshot.c | 1 +

hw/block/virtio-blk.c | 132 +-------

hw/ide/core.c | 3 +-

hw/ide/macio.c | 3 +-

hw/scsi/scsi-bus.c | 121 +++++--

hw/scsi/scsi-disk.c | 24 +-

hw/scsi/virtio-scsi-dataplane.c | 103 ++++--

hw/scsi/virtio-scsi.c | 502 ++++++++++++++++------------

hw/virtio/iothread-vq-mapping.c | 131 ++++++++

system/dma-helpers.c | 8 +-

util/aio-posix.c | 114 ++++---

util/async.c | 1 -

hw/virtio/meson.build | 1 +

meson.build | 4 +

tests/qemu-iotests/tests/qsd-migrate | 2 +-

28 files changed, 803 insertions(+), 512 deletions(-)

create mode 100644 include/hw/virtio/iothread-vq-mapping.h

create mode 100644 hw/virtio/iothread-vq-mapping.c

Commit fc4e394b28 removed the last caller of blk_op_is_blocked(). Remove

the now unused function.

Message-ID: <20250206165331.379033-1-kwolf@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

include/system/block-backend-global-state.h | 1 -

block/block-backend.c | 12 ------------

2 files changed, 13 deletions(-)

diff --git a/include/system/block-backend-global-state.h b/include/system/block-backend-global-state.h

--- a/include/system/block-backend-global-state.h

+++ b/include/system/block-backend-global-state.h

@@ -XXX,XX +XXX,XX @@ bool blk_supports_write_perm(BlockBackend *blk);

bool blk_is_sg(BlockBackend *blk);

void blk_set_enable_write_cache(BlockBackend *blk, bool wce);

int blk_get_flags(BlockBackend *blk);

-bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp);

int blk_set_aio_context(BlockBackend *blk, AioContext *new_context,

Error **errp);

void blk_add_aio_context_notifier(BlockBackend *blk,

diff --git a/block/block-backend.c b/block/block-backend.c

index XXXXXXX..XXXXXXX 100644

--- a/block/block-backend.c

+++ b/block/block-backend.c

@@ -XXX,XX +XXX,XX @@ void *blk_blockalign(BlockBackend *blk, size_t size)

return qemu_blockalign(blk ? blk_bs(blk) : NULL, size);

-bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp)

-{

- BlockDriverState *bs = blk_bs(blk);

- GLOBAL_STATE_CODE();

- GRAPH_RDLOCK_GUARD_MAINLOOP();

-

- if (!bs) {

- return false;

- }

-

- return bdrv_op_is_blocked(bs, op, errp);

-}

/**

* Return BB's current AioContext. Note that this context may change

2.48.1

Block drivers assume in their .bdrv_open() implementation that their

state in bs->opaque has been zeroed; it is initially allocated with

g_malloc0() in bdrv_open_driver().

bdrv_snapshot_goto() needs to make sure that it is zeroed again before

calling drv->bdrv_open() to avoid that block drivers use stale values.

One symptom of this bug is VMDK running into a double free when the user

tries to apply an internal snapshot like 'qemu-img snapshot -a test

test.vmdk'. This should be a graceful error because VMDK doesn't support

internal snapshots.

==25507== Invalid free() / delete / delete[] / realloc()

==25507== at 0x484B347: realloc (vg_replace_malloc.c:1801)

==25507== by 0x54B592A: g_realloc (gmem.c:171)

==25507== by 0x1B221D: vmdk_add_extent (../block/vmdk.c:570)

==25507== by 0x1B1084: vmdk_open_sparse (../block/vmdk.c:1059)

==25507== by 0x1AF3D8: vmdk_open (../block/vmdk.c:1371)

==25507== by 0x1A2AE0: bdrv_snapshot_goto (../block/snapshot.c:299)

==25507== by 0x205C77: img_snapshot (../qemu-img.c:3500)

==25507== by 0x58FA087: (below main) (libc_start_call_main.h:58)

==25507== Address 0x832f3e0 is 0 bytes inside a block of size 272 free'd

==25507== at 0x4846B83: free (vg_replace_malloc.c:989)

==25507== by 0x54AEAC4: g_free (gmem.c:208)

==25507== by 0x1AF629: vmdk_close (../block/vmdk.c:2889)

==25507== by 0x1A2A9C: bdrv_snapshot_goto (../block/snapshot.c:290)

==25507== by 0x205C77: img_snapshot (../qemu-img.c:3500)

==25507== by 0x58FA087: (below main) (libc_start_call_main.h:58)

This error was discovered by fuzzing qemu-img.

Cc: qemu-stable@nongnu.org

Closes: https://gitlab.com/qemu-project/qemu/-/issues/2853

Closes: https://gitlab.com/qemu-project/qemu/-/issues/2851

Reported-by: Denis Rastyogin <gerben@altlinux.org>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-ID: <20250310104858.28221-1-kwolf@redhat.com>

block/snapshot.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/block/snapshot.c b/block/snapshot.c

--- a/block/snapshot.c

+++ b/block/snapshot.c

@@ -XXX,XX +XXX,XX @@ int bdrv_snapshot_goto(BlockDriverState *bs,

bdrv_graph_wrunlock();

ret = bdrv_snapshot_goto(fallback_bs, snapshot_id, errp);

+ memset(bs->opaque, 0, drv->instance_size);

open_ret = drv->bdrv_open(bs, options, bs->open_flags, &local_err);

qobject_unref(options);

if (open_ret < 0) {

2.48.1

Until now, FUA was always emulated with a separate flush after the write

for file-posix. The overhead of processing a second request can reduce

performance significantly for a guest disk that has disabled the write

cache, especially if the host disk is already write through, too, and

the flush isn't actually doing anything.

Advertise support for REQ_FUA in write requests and implement it for

Linux AIO and io_uring using the RWF_DSYNC flag for write requests. The

thread pool still performs a separate fdatasync() call. This can be

improved later by using the pwritev2() syscall if available.

As an example, this is how fio numbers can be improved in some scenarios

with this patch (all using virtio-blk with cache=directsync on an nvme

block device for the VM, fio with ioengine=libaio,direct=1,sync=1):

| old | with FUA support

------------------------------+---------------+-------------------

bs=4k, iodepth=1, numjobs=1 | 45.6k iops | 56.1k iops

bs=4k, iodepth=1, numjobs=16 | 183.3k iops | 236.0k iops

bs=4k, iodepth=16, numjobs=1 | 258.4k iops | 311.1k iops

However, not all scenarios are clear wins. On another slower disk I saw

little to no improvment. In fact, in two corner case scenarios, I even

observed a regression, which I however consider acceptable:

1. On slow host disks in a write through cache mode, when the guest is

using virtio-blk in a separate iothread so that polling can be

enabled, and each completion is quickly followed up with a new

request (so that polling gets it), it can happen that enabling FUA

makes things slower - the additional very fast no-op flush we used to

have gave the adaptive polling algorithm a success so that it kept

polling. Without it, we only have the slow write request, which

disables polling. This is a problem in the polling algorithm that

will be fixed later in this series.

2. With a high queue depth, it can be beneficial to have flush requests

for another reason: The optimisation in bdrv_co_flush() that flushes

only once per write generation acts as a synchronisation mechanism

that lets all requests complete at the same time. This can result in

better batching and if the disk is very fast (I only saw this with a

null_blk backend), this can make up for the overhead of the flush and

improve throughput. In theory, we could optionally introduce a

similar artificial latency in the normal completion path to achieve

the same kind of completion batching. This is not implemented in this

series.

Compatibility is not a concern for io_uring, it has supported RWF_DSYNC

from the start. Linux AIO started supporting it in Linux 4.13 and libaio

0.3.111. The kernel is not a problem for any supported build platform,

so it's not necessary to add runtime checks. However, openSUSE is still

stuck with an older libaio version that would break the build. We must

detect this at build time to avoid build failures.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-ID: <20250307221634.71951-2-kwolf@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

include/block/raw-aio.h | 8 ++++++--

block/file-posix.c | 26 ++++++++++++++++++--------

block/io_uring.c | 13 ++++++++-----

block/linux-aio.c | 24 +++++++++++++++++++++---

meson.build | 4 ++++

5 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h

index XXXXXXX..XXXXXXX 100644

--- a/include/block/raw-aio.h

+++ b/include/block/raw-aio.h

@@ -XXX,XX +XXX,XX @@

#define QEMU_RAW_AIO_H

#include "block/aio.h"

+#include "block/block-common.h"

#include "qemu/iov.h"

/* AIO request types */

@@ -XXX,XX +XXX,XX @@ void laio_cleanup(LinuxAioState *s);

/* laio_co_submit: submit I/O requests in the thread's current AioContext. */

int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov,

- int type, uint64_t dev_max_batch);

+ int type, BdrvRequestFlags flags,

+ uint64_t dev_max_batch);

bool laio_has_fdsync(int);

+bool laio_has_fua(void);

void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);

void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);

#endif

@@ -XXX,XX +XXX,XX @@ void luring_cleanup(LuringState *s);

/* luring_co_submit: submit I/O requests in the thread's current AioContext. */

int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset,

- QEMUIOVector *qiov, int type);

+ QEMUIOVector *qiov, int type,

+ BdrvRequestFlags flags);

void luring_detach_aio_context(LuringState *s, AioContext *old_context);

void luring_attach_aio_context(LuringState *s, AioContext *new_context);

#endif

}

static int64_t raw_getlength(BlockDriverState *bs);

+static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs);

@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,

#endif

s->needs_alignment = raw_needs_alignment(bs);

+ if (!s->use_linux_aio || laio_has_fua()) {

+ bs->supported_write_flags = BDRV_REQ_FUA;

+ }

+

bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK;

if (S_ISREG(st.st_mode)) {

/* When extending regular files, we get zeros from the OS */

@@ -XXX,XX +XXX,XX @@ static inline bool raw_check_linux_aio(BDRVRawState *s)

#endif

static int coroutine_fn raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr,

- uint64_t bytes, QEMUIOVector *qiov, int type)

+ uint64_t bytes, QEMUIOVector *qiov, int type,

+ int flags)

{

BDRVRawState *s = bs->opaque;

RawPosixAIOData acb;

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr,

#ifdef CONFIG_LINUX_IO_URING

} else if (raw_check_linux_io_uring(s)) {

assert(qiov->size == bytes);

- ret = luring_co_submit(bs, s->fd, offset, qiov, type);

+ ret = luring_co_submit(bs, s->fd, offset, qiov, type, flags);

goto out;

#endif

#ifdef CONFIG_LINUX_AIO

} else if (raw_check_linux_aio(s)) {

assert(qiov->size == bytes);

- ret = laio_co_submit(s->fd, offset, qiov, type,

+ ret = laio_co_submit(s->fd, offset, qiov, type, flags,

s->aio_max_batch);

goto out;

#endif

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr,

assert(qiov->size == bytes);

ret = raw_thread_pool_submit(handle_aiocb_rw, &acb);

+ if (ret == 0 && (flags & BDRV_REQ_FUA)) {

+ /* TODO Use pwritev2() instead if it's available */

+ ret = raw_co_flush_to_disk(bs);

+ }

goto out; /* Avoid the compiler err of unused label */

out:

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,

int64_t bytes, QEMUIOVector *qiov,

BdrvRequestFlags flags)

{

- return raw_co_prw(bs, &offset, bytes, qiov, QEMU_AIO_READ);

+ return raw_co_prw(bs, &offset, bytes, qiov, QEMU_AIO_READ, flags);

}

static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,

int64_t bytes, QEMUIOVector *qiov,

BdrvRequestFlags flags)

{

- return raw_co_prw(bs, &offset, bytes, qiov, QEMU_AIO_WRITE);

+ return raw_co_prw(bs, &offset, bytes, qiov, QEMU_AIO_WRITE, flags);

}

static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)

#ifdef CONFIG_LINUX_IO_URING

if (raw_check_linux_io_uring(s)) {

- return luring_co_submit(bs, s->fd, 0, NULL, QEMU_AIO_FLUSH);

+ return luring_co_submit(bs, s->fd, 0, NULL, QEMU_AIO_FLUSH, 0);

#endif

#ifdef CONFIG_LINUX_AIO

if (s->has_laio_fdsync && raw_check_linux_aio(s)) {

- return laio_co_submit(s->fd, 0, NULL, QEMU_AIO_FLUSH, 0);

+ return laio_co_submit(s->fd, 0, NULL, QEMU_AIO_FLUSH, 0, 0);

}

#endif

return raw_thread_pool_submit(handle_aiocb_flush, &acb);

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_zone_append(BlockDriverState *bs,

}

trace_zbd_zone_append(bs, *offset >> BDRV_SECTOR_BITS);

- return raw_co_prw(bs, offset, len, qiov, QEMU_AIO_ZONE_APPEND);

+ return raw_co_prw(bs, offset, len, qiov, QEMU_AIO_ZONE_APPEND, 0);

}

#endif

diff --git a/block/io_uring.c b/block/io_uring.c

index XXXXXXX..XXXXXXX 100644

--- a/block/io_uring.c

+++ b/block/io_uring.c

@@ -XXX,XX +XXX,XX @@ static void luring_deferred_fn(void *opaque)

*

*/

static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,

- uint64_t offset, int type)

+ uint64_t offset, int type, BdrvRequestFlags flags)

{

struct io_uring_sqe *sqes = &luringcb->sqeq;

+ int luring_flags;

switch (type) {

case QEMU_AIO_WRITE:

- io_uring_prep_writev(sqes, fd, luringcb->qiov->iov,

- luringcb->qiov->niov, offset);

+ luring_flags = (flags & BDRV_REQ_FUA) ? RWF_DSYNC : 0;

+ io_uring_prep_writev2(sqes, fd, luringcb->qiov->iov,

+ luringcb->qiov->niov, offset, luring_flags);

break;

case QEMU_AIO_ZONE_APPEND:

io_uring_prep_writev(sqes, fd, luringcb->qiov->iov,

@@ -XXX,XX +XXX,XX @@ static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,

}

int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset,

- QEMUIOVector *qiov, int type)

+ QEMUIOVector *qiov, int type,

+ BdrvRequestFlags flags)

{

int ret;

AioContext *ctx = qemu_get_current_aio_context();

@@ -XXX,XX +XXX,XX @@ int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset,

};

trace_luring_co_submit(bs, s, &luringcb, fd, offset, qiov ? qiov->size : 0,

type);

- ret = luring_do_submit(fd, &luringcb, s, offset, type);

+ ret = luring_do_submit(fd, &luringcb, s, offset, type, flags);

if (ret < 0) {

return ret;

diff --git a/block/linux-aio.c b/block/linux-aio.c

index XXXXXXX..XXXXXXX 100644

--- a/block/linux-aio.c

+++ b/block/linux-aio.c

@@ -XXX,XX +XXX,XX @@ static void laio_deferred_fn(void *opaque)

}

static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset,

- int type, uint64_t dev_max_batch)

+ int type, BdrvRequestFlags flags,

+ uint64_t dev_max_batch)

{

LinuxAioState *s = laiocb->ctx;

struct iocb *iocbs = &laiocb->iocb;

QEMUIOVector *qiov = laiocb->qiov;

+ int laio_flags;

switch (type) {

case QEMU_AIO_WRITE:

+#ifdef HAVE_IO_PREP_PWRITEV2

+ laio_flags = (flags & BDRV_REQ_FUA) ? RWF_DSYNC : 0;

+ io_prep_pwritev2(iocbs, fd, qiov->iov, qiov->niov, offset, laio_flags);

+#else

+ assert(flags == 0);

io_prep_pwritev(iocbs, fd, qiov->iov, qiov->niov, offset);

+#endif

break;

case QEMU_AIO_ZONE_APPEND:

io_prep_pwritev(iocbs, fd, qiov->iov, qiov->niov, offset);

@@ -XXX,XX +XXX,XX @@ static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset,

}

int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov,

- int type, uint64_t dev_max_batch)

+ int type, BdrvRequestFlags flags,

+ uint64_t dev_max_batch)

{

int ret;

AioContext *ctx = qemu_get_current_aio_context();

@@ -XXX,XX +XXX,XX @@ int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov,

.qiov = qiov,

};

- ret = laio_do_submit(fd, &laiocb, offset, type, dev_max_batch);

+ ret = laio_do_submit(fd, &laiocb, offset, type, flags, dev_max_batch);

if (ret < 0) {

return ret;

}

@@ -XXX,XX +XXX,XX @@ bool laio_has_fdsync(int fd)

io_destroy(ctx);

return (ret == -EINVAL) ? false : true;

}

+

+bool laio_has_fua(void)

+#ifdef HAVE_IO_PREP_PWRITEV2

+ return true;

+#else

+ return false;

+#endif

diff --git a/meson.build b/meson.build

index XXXXXXX..XXXXXXX 100644

--- a/meson.build

+++ b/meson.build

@@ -XXX,XX +XXX,XX @@ config_host_data.set('HAVE_OPTRESET',

cc.has_header_symbol('getopt.h', 'optreset'))

config_host_data.set('HAVE_IPPROTO_MPTCP',

cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))

+if libaio.found()

+ config_host_data.set('HAVE_IO_PREP_PWRITEV2',

+ cc.has_header_symbol('libaio.h', 'io_prep_pwritev2'))

+endif

# has_member

config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',

2.48.1

For block drivers that don't advertise FUA support, we already call

bdrv_co_flush(), which considers BDRV_O_NO_FLUSH. However, drivers that

do support FUA still see the FUA flag with BDRV_O_NO_FLUSH and get the

associated performance penalty that cache.no-flush=on was supposed to

avoid.

Clear FUA for write requests if BDRV_O_NO_FLUSH is set.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-ID: <20250307221634.71951-3-kwolf@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

block/io.c | 4 ++++

1 file changed, 4 insertions(+)

@@ -XXX,XX +XXX,XX @@ bdrv_driver_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,

return -ENOMEDIUM;

+ if (bs->open_flags & BDRV_O_NO_FLUSH) {

+ flags &= ~BDRV_REQ_FUA;

+ }

+

if ((flags & BDRV_REQ_FUA) &&

(~bs->supported_write_flags & BDRV_REQ_FUA)) {

flags &= ~BDRV_REQ_FUA;

2.48.1

As a preparation for having multiple adaptive polling states per

AioContext, move the 'ns' field into a separate struct.

Message-ID: <20250307221634.71951-4-kwolf@redhat.com>

include/block/aio.h | 6 +++++-

util/aio-posix.c | 31 ++++++++++++++++---------------

util/async.c | 3 ++-

3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h

--- a/include/block/aio.h

+++ b/include/block/aio.h

@@ -XXX,XX +XXX,XX @@ struct BHListSlice {

typedef QSLIST_HEAD(, AioHandler) AioHandlerSList;

+typedef struct AioPolledEvent {

+ int64_t ns; /* current polling time in nanoseconds */

+} AioPolledEvent;

+

struct AioContext {

GSource source;

@@ -XXX,XX +XXX,XX @@ struct AioContext {

int poll_disable_cnt;

/* Polling mode parameters */

- int64_t poll_ns; /* current polling time in nanoseconds */

+ AioPolledEvent poll;

int64_t poll_max_ns; /* maximum polling time in nanoseconds */

int64_t poll_grow; /* polling time growth factor */

int64_t poll_shrink; /* polling time shrink factor */

diff --git a/util/aio-posix.c b/util/aio-posix.c

index XXXXXXX..XXXXXXX 100644

--- a/util/aio-posix.c

+++ b/util/aio-posix.c

@@ -XXX,XX +XXX,XX @@ static bool try_poll_mode(AioContext *ctx, AioHandlerList *ready_list,

return false;

}

- max_ns = qemu_soonest_timeout(*timeout, ctx->poll_ns);

+ max_ns = qemu_soonest_timeout(*timeout, ctx->poll.ns);

if (max_ns && !ctx->fdmon_ops->need_wait(ctx)) {

/*

* Enable poll mode. It pairs with the poll_set_started() in

@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)

if (ctx->poll_max_ns) {

int64_t block_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - start;

- if (block_ns <= ctx->poll_ns) {

+ if (block_ns <= ctx->poll.ns) {

/* This is the sweet spot, no adjustment needed */

} else if (block_ns > ctx->poll_max_ns) {

/* We'd have to poll for too long, poll less */

- int64_t old = ctx->poll_ns;

+ int64_t old = ctx->poll.ns;

if (ctx->poll_shrink) {

- ctx->poll_ns /= ctx->poll_shrink;

+ ctx->poll.ns /= ctx->poll_shrink;

} else {

- ctx->poll_ns = 0;

+ ctx->poll.ns = 0;

}

- trace_poll_shrink(ctx, old, ctx->poll_ns);

- } else if (ctx->poll_ns < ctx->poll_max_ns &&

+ trace_poll_shrink(ctx, old, ctx->poll.ns);

+ } else if (ctx->poll.ns < ctx->poll_max_ns &&

block_ns < ctx->poll_max_ns) {

/* There is room to grow, poll longer */

- int64_t old = ctx->poll_ns;

+ int64_t old = ctx->poll.ns;

int64_t grow = ctx->poll_grow;

if (grow == 0) {

grow = 2;

}

- if (ctx->poll_ns) {

- ctx->poll_ns *= grow;

+ if (ctx->poll.ns) {

+ ctx->poll.ns *= grow;

} else {

- ctx->poll_ns = 4000; /* start polling at 4 microseconds */

+ ctx->poll.ns = 4000; /* start polling at 4 microseconds */

}

- if (ctx->poll_ns > ctx->poll_max_ns) {

- ctx->poll_ns = ctx->poll_max_ns;

+ if (ctx->poll.ns > ctx->poll_max_ns) {

+ ctx->poll.ns = ctx->poll_max_ns;

}

- trace_poll_grow(ctx, old, ctx->poll_ns);

+ trace_poll_grow(ctx, old, ctx->poll.ns);

@@ -XXX,XX +XXX,XX @@ void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,

/* No thread synchronization here, it doesn't matter if an incorrect value

* is used once.

*/

+ ctx->poll.ns = 0;

ctx->poll_max_ns = max_ns;

- ctx->poll_ns = 0;

ctx->poll_grow = grow;

ctx->poll_shrink = shrink;

diff --git a/util/async.c b/util/async.c

index XXXXXXX..XXXXXXX 100644

--- a/util/async.c

+++ b/util/async.c

@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)

qemu_rec_mutex_init(&ctx->lock);

timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);

- ctx->poll_ns = 0;

+ ctx->poll.ns = 0;

ctx->poll_max_ns = 0;

ctx->poll_grow = 0;

ctx->poll_shrink = 0;

2.48.1

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-ID: <20250307221634.71951-5-kwolf@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

util/aio-posix.c | 77 ++++++++++++++++++++++++++----------------------

1 file changed, 41 insertions(+), 36 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c

--- a/util/aio-posix.c

+++ b/util/aio-posix.c

@@ -XXX,XX +XXX,XX @@ static bool try_poll_mode(AioContext *ctx, AioHandlerList *ready_list,

return false;

+static void adjust_polling_time(AioContext *ctx, AioPolledEvent *poll,

+ int64_t block_ns)

+{

+ if (block_ns <= poll->ns) {

+ /* This is the sweet spot, no adjustment needed */

+ } else if (block_ns > ctx->poll_max_ns) {

+ /* We'd have to poll for too long, poll less */

+ int64_t old = poll->ns;

+

+ if (ctx->poll_shrink) {

+ poll->ns /= ctx->poll_shrink;

+ } else {

+ poll->ns = 0;

+ }

+

+ trace_poll_shrink(ctx, old, poll->ns);

+ } else if (poll->ns < ctx->poll_max_ns &&

+ block_ns < ctx->poll_max_ns) {

+ /* There is room to grow, poll longer */

+ int64_t old = poll->ns;

+ int64_t grow = ctx->poll_grow;

+

+ if (grow == 0) {

+ grow = 2;

+ }

+

+ if (poll->ns) {

+ poll->ns *= grow;

+ } else {

+ poll->ns = 4000; /* start polling at 4 microseconds */

+ }

+

+ if (poll->ns > ctx->poll_max_ns) {

+ poll->ns = ctx->poll_max_ns;

+ }

+

+ trace_poll_grow(ctx, old, poll->ns);

+ }

+}

+

bool aio_poll(AioContext *ctx, bool blocking)

AioHandlerList ready_list = QLIST_HEAD_INITIALIZER(ready_list);

@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)

/* Adjust polling time */

if (ctx->poll_max_ns) {

int64_t block_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - start;

-

- if (block_ns <= ctx->poll.ns) {

- /* This is the sweet spot, no adjustment needed */

- } else if (block_ns > ctx->poll_max_ns) {

- /* We'd have to poll for too long, poll less */

- int64_t old = ctx->poll.ns;

-

- if (ctx->poll_shrink) {

- ctx->poll.ns /= ctx->poll_shrink;

- } else {

- ctx->poll.ns = 0;

- }

-

- trace_poll_shrink(ctx, old, ctx->poll.ns);

- } else if (ctx->poll.ns < ctx->poll_max_ns &&

- block_ns < ctx->poll_max_ns) {

- /* There is room to grow, poll longer */

- int64_t old = ctx->poll.ns;

- int64_t grow = ctx->poll_grow;

-

- if (grow == 0) {

- grow = 2;

- }

-

- if (ctx->poll.ns) {

- ctx->poll.ns *= grow;

- } else {

- ctx->poll.ns = 4000; /* start polling at 4 microseconds */

- }

-

- if (ctx->poll.ns > ctx->poll_max_ns) {

- ctx->poll.ns = ctx->poll_max_ns;

- }

-

- trace_poll_grow(ctx, old, ctx->poll.ns);

- }

+ adjust_polling_time(ctx, &ctx->poll, block_ns);

}

progress |= aio_bh_poll(ctx);

2.48.1

Adaptive polling has a big problem: It doesn't consider that an event

loop can wait for many different events that may have very different

typical latencies.

For example, think of a guest that tends to send a new I/O request soon

after the previous I/O request completes, but the storage on the host is

rather slow. In this case, getting the new request from guest quickly

means that polling is enabled, but the next thing is performing the I/O

request on the backend, which is slow and disables polling again for the

next guest request. This means that in such a scenario, polling could

help for every other event, but is only ever enabled when it can't

succeed.

In order to fix this, keep a separate AioPolledEvent for each

AioHandler. We will then know that the backend file descriptor always

has a high latency and isn't worth polling for, but we also know that

the guest is always fast and we should poll for it. This solves at least

half of the problem, we can now keep polling for those cases where it

makes sense and get the improved performance from it.

Since the event loop doesn't know which event will be next, we still do

some unnecessary polling while we're waiting for the slow disk. I made

some attempts to be more clever than just randomly growing and shrinking

the polling time, and even to let callers be explicit about when they

expect a new event, but so far this hasn't resulted in improved

performance or even caused performance regressions. For now, let's just

fix the part that is easy enough to fix, we can revisit the rest later.

Message-ID: <20250307221634.71951-6-kwolf@redhat.com>

include/block/aio.h | 1 -

util/aio-posix.h | 1 +

util/aio-posix.c | 26 ++++++++++++++++++++++----

util/async.c | 2 --

4 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h

--- a/include/block/aio.h

+++ b/include/block/aio.h

@@ -XXX,XX +XXX,XX @@ struct AioContext {

int poll_disable_cnt;

/* Polling mode parameters */

- AioPolledEvent poll;

int64_t poll_max_ns; /* maximum polling time in nanoseconds */

int64_t poll_grow; /* polling time growth factor */

int64_t poll_shrink; /* polling time shrink factor */

diff --git a/util/aio-posix.h b/util/aio-posix.h

index XXXXXXX..XXXXXXX 100644

--- a/util/aio-posix.h

+++ b/util/aio-posix.h

@@ -XXX,XX +XXX,XX @@ struct AioHandler {

#endif

int64_t poll_idle_timeout; /* when to stop userspace polling */

bool poll_ready; /* has polling detected an event? */

+ AioPolledEvent poll;

};

/* Add a handler to a ready list */

diff --git a/util/aio-posix.c b/util/aio-posix.c

index XXXXXXX..XXXXXXX 100644

--- a/util/aio-posix.c

+++ b/util/aio-posix.c

@@ -XXX,XX +XXX,XX @@ static bool run_poll_handlers(AioContext *ctx, AioHandlerList *ready_list,

static bool try_poll_mode(AioContext *ctx, AioHandlerList *ready_list,

int64_t *timeout)

+ AioHandler *node;

int64_t max_ns;

if (QLIST_EMPTY_RCU(&ctx->poll_aio_handlers)) {

return false;

- max_ns = qemu_soonest_timeout(*timeout, ctx->poll.ns);

+ max_ns = 0;

+ QLIST_FOREACH(node, &ctx->poll_aio_handlers, node_poll) {

+ max_ns = MAX(max_ns, node->poll.ns);

+ }

+ max_ns = qemu_soonest_timeout(*timeout, max_ns);

+

if (max_ns && !ctx->fdmon_ops->need_wait(ctx)) {

/*

* Enable poll mode. It pairs with the poll_set_started() in

@@ -XXX,XX +XXX,XX @@ bool aio_poll(AioContext *ctx, bool blocking)

/* Adjust polling time */

if (ctx->poll_max_ns) {

+ AioHandler *node;

int64_t block_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - start;

- adjust_polling_time(ctx, &ctx->poll, block_ns);

+

+ QLIST_FOREACH(node, &ctx->poll_aio_handlers, node_poll) {

+ if (QLIST_IS_INSERTED(node, node_ready)) {

+ adjust_polling_time(ctx, &node->poll, block_ns);

+ }

progress |= aio_bh_poll(ctx);

@@ -XXX,XX +XXX,XX @@ void aio_context_use_g_source(AioContext *ctx)

void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,

int64_t grow, int64_t shrink, Error **errp)

{

+ AioHandler *node;

+

+ qemu_lockcnt_inc(&ctx->list_lock);

+ QLIST_FOREACH(node, &ctx->aio_handlers, node) {

+ node->poll.ns = 0;

+ }

+ qemu_lockcnt_dec(&ctx->list_lock);

+

/* No thread synchronization here, it doesn't matter if an incorrect value

* is used once.

*/

- ctx->poll.ns = 0;

-

ctx->poll_max_ns = max_ns;

ctx->poll_grow = grow;

ctx->poll_shrink = shrink;

diff --git a/util/async.c b/util/async.c

index XXXXXXX..XXXXXXX 100644

--- a/util/async.c

+++ b/util/async.c

@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)

qemu_rec_mutex_init(&ctx->lock);

timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);

- ctx->poll.ns = 0;

-

ctx->poll_max_ns = 0;

ctx->poll_grow = 0;

ctx->poll_shrink = 0;

2.48.1

aio_dispatch_handler() adds handlers to ctx->poll_aio_handlers if

polling should be enabled. If we call adjust_polling_time() for all

polling handlers before this, new polling handlers are still left at

poll->ns = 0 and polling is only actually enabled after the next event.

Move the adjust_polling_time() call after aio_dispatch_handler().

This fixes test-nested-aio-poll, which expects that polling becomes