From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39712) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b049u-0001U7-8q for qemu-devel@nongnu.org; Tue, 10 May 2016 05:40:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b049o-0003B2-5y for qemu-devel@nongnu.org; Tue, 10 May 2016 05:40:53 -0400 Date: Tue, 10 May 2016 11:40:33 +0200 From: Kevin Wolf Message-ID: <20160510094033.GI4921@noname.str.redhat.com> References: <1460046816-102846-1-git-send-email-pbonzini@redhat.com> <1460046816-102846-9-git-send-email-pbonzini@redhat.com> <20160419090953.GA16312@stefanha-x1.localdomain> <5730BB70.8010600@redhat.com> <20160510093040.GB11408@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="0ntfKIWw70PvrIHh" Content-Disposition: inline In-Reply-To: <20160510093040.GB11408@stefanha-x1.localdomain> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v4 8/8] linux-aio: share one LinuxAioState within an AioContext List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Paolo Bonzini , qemu-devel@nongnu.org, famz@redhat.com, qemu-block@nongnu.org, stefanha@redhat.com --0ntfKIWw70PvrIHh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 10.05.2016 um 11:30 hat Stefan Hajnoczi geschrieben: > On Mon, May 09, 2016 at 06:31:44PM +0200, Paolo Bonzini wrote: > > On 19/04/2016 11:09, Stefan Hajnoczi wrote: > > >> > This has better performance because it executes fewer system calls > > >> > and does not use a bottom half per disk. > > > Each aio_context_t is initialized for 128 in-flight requests in > > > laio_init(). > > >=20 > > > Will it be possible to hit the limit now that all drives share the sa= me > > > aio_context_t? > >=20 > > It was also possible before, because the virtqueue can be bigger than > > 128 items; that's why there is logic to submit I/O requests after an > > io_get_events. As usual when the answer seems trivial, am I > > misunderstanding your question? >=20 > I'm concerned about a performance regression rather than correctness. >=20 > But looking at linux-aio.c there *is* a correctness problem: >=20 > static void ioq_submit(struct qemu_laio_state *s) > { > int ret, len; > struct qemu_laiocb *aiocb; > struct iocb *iocbs[MAX_QUEUED_IO]; > QSIMPLEQ_HEAD(, qemu_laiocb) completed; >=20 > do { > len =3D 0; > QSIMPLEQ_FOREACH(aiocb, &s->io_q.pending, next) { > iocbs[len++] =3D &aiocb->iocb; > if (len =3D=3D MAX_QUEUED_IO) { > break; > } > } >=20 > ret =3D io_submit(s->ctx, len, iocbs); > if (ret =3D=3D -EAGAIN) { > break; > } > if (ret < 0) { > abort(); > } >=20 > s->io_q.n -=3D ret; > aiocb =3D container_of(iocbs[ret - 1], struct qemu_laiocb, iocb= ); > QSIMPLEQ_SPLIT_AFTER(&s->io_q.pending, aiocb, next, &completed); > } while (ret =3D=3D len && !QSIMPLEQ_EMPTY(&s->io_q.pending)); > s->io_q.blocked =3D (s->io_q.n > 0); > } >=20 > io_submit() may have submitted some of the requests when -EAGAIN is > returned. QEMU gets no indication of which requests were submitted. My understanding (which is based on the manpage rather than code) is that -EAGAIN is only returned if no request could be submitted. In other cases, the number of submitted requests is returned (similar to how short reads work). > It may be possible to dig around in the s->ctx rings to find out or we > need to keep track of the number of in-flight requests so we can > prevent ever hitting EAGAIN. >=20 > ioq_submit() pretends that no requests were submitted on -EAGAIN and > will submit them again next time. This could result in double > completions. Did you check in the code that this can happen? > Regarding performance, I'm thinking about a guest with 8 disks (queue > depth 32). The worst case is when the guest submits 32 requests at once > but the Linux AIO event limit has already been reached. Then the disk > is starved until other disks' requests complete. Sounds like a valid concern. Kevin --0ntfKIWw70PvrIHh Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXMayRAAoJEH8JsnLIjy/WQu8P/RjpK5aMzxbm9rhK5gCVv7J+ n2FZ1fJr7J/4HmDUF2tmhgmDWuF9o3HURtqe/2CW43K0tU/xXf071yUxLtXa5R8z SMm8Sh2J1tfsujHK64/nDwOoRmTPniwC3l+4xJSSVBkTHv4Zng64oJp2Y3ems8GX /GJAZ8yPEP4mKphMhX6FT6wSoYzt5DYmZYQPotUqbj8zLqFQO71Gou+zSF+voe38 pebPxZ661hvFdUAcyE3/u1QGbzagHPI9RuIewy4rbHkEAkFNfalyHNAo3yuoSHjQ gNQfRJdk2pfzk2NzJ1gjliNE7NgD2pe7SJlrpNBW6QWt+C7YAbYYc6u7AV4tld5U xIs+YuNppzIVTvrA92q75fkaN+Py0WKCBNaTl1Hek9icciUwRaqd9RlvMfU7l75S xCa8Vt9Iz+W3hcw8Teh9lC8qIiInjrMZknkm5jT6Dbo6ogrC27WsI/Sdn55Rovzu XrIF5n0OXLyRoaQG0+ceXkhC/bOt0QiYHIE1kLpeaMHNa9A+5v537sXVbTgIXzXk LBiax+3s0vVUp2Fp+zAgVEOvvjvNsrf6jGSxnJTHHpMS8R8PIiVPj5ON6PoCvMje xvmcT56VGDE5FQWNKmRkxlrFezfeKcWSEUflgJNY37Kn1lVHwlbxnMNKh3UkovkG 4hWbmlemZtT72mMY7D1a =z8eg -----END PGP SIGNATURE----- --0ntfKIWw70PvrIHh--