[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/15] linux-aio: fix submit aio as a batch
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 09/15] linux-aio: fix submit aio as a batch |
Date: |
Thu, 31 Jul 2014 01:41:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
Il 30/07/2014 19:32, Ming Lei ha scritto:
> On Wed, Jul 30, 2014 at 9:59 PM, Paolo Bonzini <address@hidden> wrote:
>> Il 30/07/2014 13:39, Ming Lei ha scritto:
>>> In the enqueue path, we can't complete request, otherwise
>>> "Co-routine re-entered recursively" may be caused, so this
>>> patch fixes the issue with below ideas:
>>>
>>> - for -EAGAIN or partial completion, retry the submission by
>>> an introduced event handler
>>> - for part of completion, also update the io queue
>>> - for other failure, return the failure if in enqueue path,
>>> otherwise, abort all queued I/O
>>>
>>> Signed-off-by: Ming Lei <address@hidden>
>>> ---
>>> block/linux-aio.c | 90
>>> ++++++++++++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 68 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/block/linux-aio.c b/block/linux-aio.c
>>> index 7ac7e8c..5eb9c92 100644
>>> --- a/block/linux-aio.c
>>> +++ b/block/linux-aio.c
>>> @@ -51,6 +51,7 @@ struct qemu_laio_state {
>>>
>>> /* io queue for submit at batch */
>>> LaioQueue io_q;
>>> + EventNotifier retry; /* handle -EAGAIN and partial completion */
>>> };
>>>
>>> static inline ssize_t io_event_ret(struct io_event *ev)
>>> @@ -154,45 +155,80 @@ static void ioq_init(LaioQueue *io_q)
>>> io_q->plugged = 0;
>>> }
>>>
>>> -static int ioq_submit(struct qemu_laio_state *s)
>>> +static void abort_queue(struct qemu_laio_state *s)
>>> +{
>>> + int i;
>>> + for (i = 0; i < s->io_q.idx; i++) {
>>> + struct qemu_laiocb *laiocb = container_of(s->io_q.iocbs[i],
>>> + struct qemu_laiocb,
>>> + iocb);
>>> + laiocb->ret = -EIO;
>>> + qemu_laio_process_completion(s, laiocb);
>>> + }
>>> +}
>>> +
>>> +static int ioq_submit(struct qemu_laio_state *s, bool enqueue)
>>> {
>>> int ret, i = 0;
>>> int len = s->io_q.idx;
>>> + int j = 0;
>>>
>>> - do {
>>> - ret = io_submit(s->ctx, len, s->io_q.iocbs);
>>> - } while (i++ < 3 && ret == -EAGAIN);
>>> + if (!len) {
>>> + return 0;
>>> + }
>>>
>>> - /* empty io queue */
>>> - s->io_q.idx = 0;
>>> + ret = io_submit(s->ctx, len, s->io_q.iocbs);
>>> + if (ret == -EAGAIN) {
>>> + event_notifier_set(&s->retry);
>>
>> Retrying immediately (and just doing a couple of system calls to waste
>> time) is not an improvement. The right place to retry is in
>> qemu_laio_completion_cb, after io_getevents has been called and
>> presumably the queue depth has decreased.
>
> Good point.
>
>>
>> If !s->io_q.plugged but io_submit fails you can call ioq_enqueue and it
>
> When will the queued I/O be submitted? That will introduce extra
> complexity definitely.
It will be submitted when qemu_laio_completion_cb is called.
> It is a change for !s->io_q.plugged case, and it isn't good to do that in
> this patch, IMO.
I agree with you that this series is doing too many things at a single
time. You can submit separate series for 1) no-coroutine fast path, 2)
full queue, 3) multiqueue. If you do things properly you won't have a
single conflict, since they affect respectively block.c,
block/linux-aio.c and hw/block/.
Paolo
[Qemu-devel] [PATCH 10/15] linux-aio: increase max event to 256, Ming Lei, 2014/07/30
[Qemu-devel] [PATCH 11/15] linux-aio: remove 'node' from 'struct qemu_laiocb', Ming Lei, 2014/07/30
[Qemu-devel] [PATCH 12/15] hw/virtio-pci: introduce num_queues property, Ming Lei, 2014/07/30
[Qemu-devel] [PATCH 13/15] hw/virtio/virtio-blk.h: introduce VIRTIO_BLK_F_MQ, Ming Lei, 2014/07/30
[Qemu-devel] [PATCH 14/15] hw/block/virtio-blk: create num_queues vqs if dataplane is enabled, Ming Lei, 2014/07/30