[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/4] linux-aio: simplify removal of completed io
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 4/4] linux-aio: simplify removal of completed iocbs from the list |
Date: |
Thu, 11 Dec 2014 14:13:36 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 10.12.2014 um 15:52 hat Paolo Bonzini geschrieben:
> There is no need to do another O(n) pass on the list; the iocb to
> splice the list at is already available in the array we passed to
> io_submit.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> block/linux-aio.c | 12 ++++++------
> include/qemu/queue.h | 11 +++++++++++
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/block/linux-aio.c b/block/linux-aio.c
> index b223d9e..6c98f72 100644
> --- a/block/linux-aio.c
> +++ b/block/linux-aio.c
> @@ -186,9 +186,10 @@ static void ioq_init(LaioQueue *io_q)
>
> static int ioq_submit(struct qemu_laio_state *s)
> {
> - int ret, i, len;
> + int ret, len;
> struct qemu_laiocb *aiocb;
> struct iocb *iocbs[MAX_QUEUED_IO];
> + QSIMPLEQ_HEAD(, qemu_laiocb) completed;
>
> do {
> len = 0;
> @@ -201,16 +202,15 @@ static int ioq_submit(struct qemu_laio_state *s)
>
> ret = io_submit(s->ctx, len, iocbs);
> if (ret == -EAGAIN) {
> - ret = 0;
> + break;
> }
ioq_submit() returns -EAGAIN now instead of 0. Almost all callers ignore
the return value, except laio_io_unplug(), which inherits this change.
Fortunately, raw_aio_unplug() completely ignores the return value.
So seems that this didn't cause a bug, but we have some cleanup to do
and make functions void if their return value isn't used anywhere.
> if (ret < 0) {
> abort();
> }
>
> - for (i = 0; i < ret; i++) {
> - s->io_q.n--;
> - QSIMPLEQ_REMOVE_HEAD(&s->io_q.pending, next);
> - }
> + s->io_q.n -= ret;
> + aiocb = container_of(iocbs[ret - 1], struct qemu_laiocb, iocb);
> + QSIMPLEQ_SPLIT_AFTER(&completed, &s->io_q.pending, aiocb, next);
> } while (ret == len && !QSIMPLEQ_EMPTY(&s->io_q.pending));
> s->io_q.blocked = (s->io_q.n > 0);
>
> diff --git a/include/qemu/queue.h b/include/qemu/queue.h
> index 0dedd29..2c21d28 100644
> --- a/include/qemu/queue.h
> +++ b/include/qemu/queue.h
> @@ -279,6 +279,17 @@ struct {
> \
> (head)->sqh_last = &(head)->sqh_first; \
> } while (/*CONSTCOND*/0)
>
> +#define QSIMPLEQ_SPLIT_AFTER(head1, head2, elm, field) do { \
> + if (((head1)->sqh_first = (head2)->sqh_first) == NULL) { \
> + (head1)->sqh_last = &(head1)->sqh_first; \
> + } else { \
> + (head1)->sqh_last = &(elm)->field.sqe_next; \
> + if (((head2)->sqh_first = (elm)->field.sqe_next) == NULL) { \
> + (head2)->sqh_last = &(head2)->sqh_first; \
> + } \
> + } \
> +} while (/*CONSTCOND*/0)
Wouldn't it be easier to write a macro that doesn't split a queue in
two, but simply removes everything up to a given element?
Anyway, if you really want to implement a split operation, I think you
also need to break the actual chain, i.e. (head1)->sqh_last.sqe_next =
NULL.
Kevin
[Qemu-devel] [PATCH 2/4] linux-aio: track whether the queue is blocked, Paolo Bonzini, 2014/12/10
[Qemu-devel] [PATCH 4/4] linux-aio: simplify removal of completed iocbs from the list, Paolo Bonzini, 2014/12/10
- Re: [Qemu-devel] [PATCH 4/4] linux-aio: simplify removal of completed iocbs from the list,
Kevin Wolf <=
[Qemu-devel] [PATCH 3/4] linux-aio: rename LaioQueue idx field to "n", Paolo Bonzini, 2014/12/10