qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/3] linux-aio: process completions from ioq_sub


From: Roman Penyaev
Subject: Re: [Qemu-devel] [PATCH 3/3] linux-aio: process completions from ioq_submit()
Date: Tue, 19 Jul 2016 13:18:24 +0200

On Tue, Jul 19, 2016 at 12:36 PM, Paolo Bonzini <address@hidden> wrote:
>
>
> On 19/07/2016 12:25, Roman Pen wrote:
>>      if (laiocb->co) {
>> -        qemu_coroutine_enter(laiocb->co, NULL);
>> +        if (laiocb->co == qemu_coroutine_self()) {
>> +            laiocb->self_completed = true;
>
> No need for this new field.  You can just do nothing here and check
> laiocb.ret == -EINPROGRESS here in laio_co_submit.

I have thought but did not like it, because we depend on the value,
which kernel writes there.  If kernel by chance writes -EINPROGRESS
(whatever that means, bug in some ll driver?) we are screwed up.
But probably that is my paranoia.

>
>> +        } else {
>> +            qemu_coroutine_enter(laiocb->co, NULL);
>> +        }
>>      } else {
>>          laiocb->common.cb(laiocb->common.opaque, ret);
>>          qemu_aio_unref(laiocb);
>> @@ -312,6 +317,12 @@ static void ioq_submit(LinuxAioState *s)
>>          QSIMPLEQ_SPLIT_AFTER(&s->io_q.pending, aiocb, next, &completed);
>>      } while (ret == len && !QSIMPLEQ_EMPTY(&s->io_q.pending));
>>      s->io_q.blocked = (s->io_q.in_queue > 0);
>> +
>> +    if (s->io_q.in_flight) {
>> +        /* We can try to complete something just right away if there are
>> +         * still requests in-flight. */
>> +        qemu_laio_process_completions(s);
>> +    }
>
> Can this leave I/O stuck if in_queue > 0 && in_flight == 0 after the
> return from qemu_laio_process_completions?  I think you need to goto the
> beginning of the function to submit more I/O requests in that case.

Indeed that can happen.  Will resend.

>
> In fact, perhaps it's useful to always do so if any I/Os were completed.
>  Should qemu_laio_process_completions return the number of completed
> requests?

I would always check the in_flight counter, since we are interested in what
is really left.  Also, returning the value of completed requests by _this_
qemu_laio_process_completions() call does not mean anything, since we can
be nested and bunch of completions can happen below us.  We can realy on true
of false.  Not exact value.

Also, I hope (I do not know how to reproduce this, virtio_blk does not nest),
that we are allowed to nest (calling aio_poll() and all this machinery) from
co-routine.  Are we?  We can lead to deep nesting inside the following sequence:
ioq_submit() -> complete() -> aio_poll() -> ioq_submit() -> complete() ...
but that can happen of course even without this patch.  I would say this nesting
is a clumsy stuff.

Locally I have another series, which allow completions from ioq_submit() only
if upper block devices does not nest (like virtio_blk), but I decided to send
the current changes and not to overcomplicate things.

--
Roman



reply via email to

[Prev in Thread] Current Thread [Next in Thread]