Re: [Qemu-devel] [PATCH 09/15] linux-aio: fix submit aio as a batch

[Ming Lei]

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 is a change for !s->io_q.plugged case, and it isn't good to do that in
this patch, IMO.

> will just work.  Then you can only go to out_free_aiocb if the queue is
> full (independent of the "plug" state).


Thanks,