From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53816) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XuJhC-0004xK-Hn for qemu-devel@nongnu.org; Fri, 28 Nov 2014 06:26:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XuJh7-0007UH-J5 for qemu-devel@nongnu.org; Fri, 28 Nov 2014 06:26:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39537) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XuJh7-0007UC-AN for qemu-devel@nongnu.org; Fri, 28 Nov 2014 06:26:37 -0500 Date: Fri, 28 Nov 2014 12:26:31 +0100 From: Kevin Wolf Message-ID: <20141128112631.GC4035@noname.redhat.com> References: <1416900193-3763-1-git-send-email-ming.lei@canonical.com> <1416900193-3763-3-git-send-email-ming.lei@canonical.com> <20141126112717.GC3548@noname.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v6 2/3] linux-aio: handling -EAGAIN for !s->io_q.plugged case List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei Cc: Paolo Bonzini , qemu-devel , Stefan Hajnoczi Am 28.11.2014 um 03:27 hat Ming Lei geschrieben: > Hi Kevin, > > On Wed, Nov 26, 2014 at 7:27 PM, Kevin Wolf 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 > >> Suggested-by: Paolo Bonzini > >> Signed-off-by: Ming Lei > >> --- > >> 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