[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/3] linux-aio: use LinuxAioState from the running thread
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v4 1/3] linux-aio: use LinuxAioState from the running thread |
Date: |
Mon, 31 Oct 2022 15:41:22 -0400 |
On Mon, Oct 31, 2022 at 08:59:34AM -0400, Emanuele Giuseppe Esposito wrote:
> @@ -56,10 +59,8 @@ struct LinuxAioState {
> io_context_t ctx;
> EventNotifier e;
>
> - /* io queue for submit at batch. Protected by AioContext lock. */
> + /* All data is only used in one I/O thread. */
> LaioQueue io_q;
/* No locking required, only accessed from AioContext home thread */
This is more general because it includes the main loop, which is not an
IOThread.
(Please write "IOThread" for consistency. Most of the documentation and
comments uses "IOThread".)
> -
> - /* I/O completion processing. Only runs in I/O thread. */
> QEMUBH *completion_bh;
> int event_idx;
> int event_max;
> @@ -102,6 +103,7 @@ static void qemu_laio_process_completion(struct
> qemu_laiocb *laiocb)
> * later. Coroutines cannot be entered recursively so avoid doing
> * that!
> */
> + assert(laiocb->co->ctx == laiocb->ctx->aio_context);
> if (!qemu_coroutine_entered(laiocb->co)) {
> aio_co_wake(laiocb->co);
> }
> @@ -232,13 +234,11 @@ static void qemu_laio_process_completions(LinuxAioState
> *s)
>
> static void qemu_laio_process_completions_and_submit(LinuxAioState *s)
> {
> - aio_context_acquire(s->aio_context);
> qemu_laio_process_completions(s);
>
> if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
> ioq_submit(s);
> }
> - aio_context_release(s->aio_context);
> }
>
> static void qemu_laio_completion_bh(void *opaque)
> @@ -354,14 +354,19 @@ static uint64_t laio_max_batch(LinuxAioState *s,
> uint64_t dev_max_batch)
> return max_batch;
> }
>
> -void laio_io_plug(BlockDriverState *bs, LinuxAioState *s)
> +void laio_io_plug(void)
> {
> + AioContext *ctx = qemu_get_current_aio_context();
> + LinuxAioState *s = aio_get_linux_aio(ctx);
> +
> s->io_q.plugged++;
I see the following code path:
blk_io_plug -> bdrv_io_plug -> raw_aio_plug -> laio_io_plug
blk_io_plug() can be called from any thread but laio_io_plug()
implicitly operates on the current thread's AioContext's LinuxAioState.
This changes the semantics of blk_io_plug() from a global BDS operation
to a thread-local one. The new blk_io_plug() semantics need to be
documented, because it's not obvious that multiple threads can
blk_io_plug/unplug() independently and don't affect each other. It means
the caller must be careful to pair plug/unplug in the same thread.
> }
>
> -void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s,
> - uint64_t dev_max_batch)
> +void laio_io_unplug(uint64_t dev_max_batch)
> {
> + AioContext *ctx = qemu_get_current_aio_context();
> + LinuxAioState *s = aio_get_linux_aio(ctx);
> +
> assert(s->io_q.plugged);
> s->io_q.plugged--;
>
> @@ -411,15 +416,15 @@ static int laio_do_submit(int fd, struct qemu_laiocb
> *laiocb, off_t offset,
> return 0;
> }
>
> -int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int
> fd,
> - uint64_t offset, QEMUIOVector *qiov, int
> type,
> - uint64_t dev_max_batch)
> +int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov,
> + int type, uint64_t dev_max_batch)
This function needs documentation. It submits I/O requests in the
thread's current AioContext. Before it was explicit via the function
arguments but now it's no longer obvious.
signature.asc
Description: PGP signature