Re: [Qemu-devel] [PATCH v1 2/3] linux-aio: implement io plug and unplug

[Ming Lei]

On Tue, Jul 1, 2014 at 1:31 AM, Paolo Bonzini <address@hidden> wrote:
> Il 30/06/2014 17:47, Ming Lei ha scritto:
>
>> From: Ming Lei <address@hidden>
>>
>> This patch implements .bdrv_io_plug and .bdrv_io_unplug
>> callbacks for linux-aio Block Drivers, so that submitting
>> I/O at batch can be supported on linux-aio.
>>
>> Signed-off-by: Ming Lei <address@hidden>
>> ---
>>  block/linux-aio.c |   85
>> +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  block/raw-aio.h   |    2 ++
>>  block/raw-posix.c |   31 +++++++++++++++++++
>>  3 files changed, 116 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/linux-aio.c b/block/linux-aio.c
>> index f0a2c08..7794162 100644
>> --- a/block/linux-aio.c
>> +++ b/block/linux-aio.c
>> @@ -25,6 +25,8 @@
>>   */
>>  #define MAX_EVENTS 128
>>
>> +#define MAX_QUEUED_IO  128
>> +
>>  struct qemu_laiocb {
>>      BlockDriverAIOCB common;
>>      struct qemu_laio_state *ctx;
>> @@ -36,11 +38,84 @@ struct qemu_laiocb {
>>      QLIST_ENTRY(qemu_laiocb) node;
>>  };
>>
>> +struct laio_queue {
>> +    struct iocb *iocbs[MAX_QUEUED_IO];
>> +    bool plugged;
>> +    unsigned int size;
>> +    unsigned int idx;
>> +};
>> +
>>  struct qemu_laio_state {
>>      io_context_t ctx;
>>      EventNotifier e;
>> +
>> +    /* io queue for submit at batch */
>> +    struct laio_queue io_q;
>>  };
>>
>> +static void ioq_init(struct laio_queue *io_q)
>> +{
>> +    io_q->size = MAX_QUEUED_IO;
>> +    io_q->idx = 0;
>> +    io_q->plugged = false;
>> +}
>> +
>> +static int ioq_submit(struct qemu_laio_state *s)
>> +{
>> +    int ret, i, len = s->io_q.idx;
>> +
>> +    while (1) {
>> +        ret = io_submit(s->ctx, len, s->io_q.iocbs);
>> +        if (ret != -EAGAIN)
>> +            break;
>
>
> Busy waiting is not acceptable here (it can be unbounded if, for example, an
> NFS server is on the other side of a network partition). You have to add a
> bottom half to qemu_laio_state that calls ioq_submit, and schedule it after
> calling io_getevents.
>
> If the bottom half is already scheduled and the queue is full, I guess
> there's no other choice than returning -EAGAIN from ioq_enqueue and
> ultimately to the guest.

That is a bit complicated, as you mentioned it is close to
2.1 release, could we just keep it simple to return failure to guest
after retrying several times? Actually, previous dataplane handles
it by exit(-1), which is unfriendly absolutely.

Also other failures should be handled as this way too.

If there is no objection, I will update it in v2.

Thanks,
--
Ming Lei