Re: [Qemu-devel] [PATCH v6 2/3] linux-aio: handling -EAGAIN for !s->io_q.plugged case

[Kevin Wolf <kwolf@redhat.com>]

Am 28.11.2014 um 03:27 hat Ming Lei geschrieben:
> Hi Kevin,
> 
> On Wed, Nov 26, 2014 at 7:27 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 25.11.2014 um 08:23 hat Ming Lei geschrieben:
> >> Previously -EAGAIN is simply ignored for !s->io_q.plugged case,
> >> and sometimes it is easy to cause -EIO to VM, such as NVME device.
> >>
> >> This patch handles -EAGAIN by io queue for !s->io_q.plugged case,
> >> and it will be retried in following aio completion cb.
> >>
> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> >> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> >> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> >> ---
> >>  block/linux-aio.c |   24 ++++++++++++++++--------
> >>  1 file changed, 16 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/block/linux-aio.c b/block/linux-aio.c
> >> index 11ac828..ac25722 100644
> >> --- a/block/linux-aio.c
> >> +++ b/block/linux-aio.c
> >> @@ -282,8 +282,13 @@ static int ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb)
> >>      s->io_q.iocbs[idx++] = iocb;
> >>      s->io_q.idx = idx;
> >>
> >> -    /* submit immediately if queue depth is above 2/3 */
> >> -    if (idx > s->io_q.size * 2 / 3) {
> >> +    /*
> >> +     * This is reached in two cases: queue not plugged but io_submit
> >> +     * returned -EAGAIN, or queue plugged.  In the latter case, start
> >> +     * submitting some I/O if the queue is getting too full.  In the
> >> +     * former case, instead, wait until an I/O operation is completed.
> >> +     */
> >
> > Are we guaranteed that an I/O operation is in flight when we get
> > -EAGAIN? The manpage of io_submit isn't very clear on this,
> > "insufficient resources" could be for any reason.
> >
> 
> That is a good question.
> 
> From fs/aio.c in linux kernel, io_submit_one() returns -EAGAIN when
> either there isn't enough requests which are reserved in io_setup(), or
> kmem_cache_alloc(GFP_KERNEL) returns NULL.
> 
> In the former case, it means I/O operation is in flight.
> 
> In the later case, it should be very difficult to trigger since GFP_KERNEL
> allocation will wait for memory reclaiming.
> 
> So most of times, it is reasonable to resubmit in completion for
> -EAGAIN.  When there is no pending I/O, we still can handle
> the very unlikely case either by returning failure to caller or
> try to submit in one BH. Does it make sense for you?

I think returning an error is fine in this case.

Kevin