From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfnMu-00040s-Pa for qemu-devel@nongnu.org; Wed, 08 Apr 2015 06:38:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YfnMt-0004EN-VR for qemu-devel@nongnu.org; Wed, 08 Apr 2015 06:38:00 -0400 Date: Wed, 8 Apr 2015 11:37:52 +0100 From: Stefan Hajnoczi Message-ID: <20150408103752.GH28835@stefanha-thinkpad.redhat.com> References: <1428069921-2957-1-git-send-email-famz@redhat.com> <1428069921-2957-3-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="9jHkwA2TBA/ec6v+" Content-Disposition: inline In-Reply-To: <1428069921-2957-3-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 2/4] block: Pause block jobs in bdrv_drain_all List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , berto@igalia.com, qemu-block@nongnu.org, Jeff Cody , qemu-devel@nongnu.org, pbonzini@redhat.com --9jHkwA2TBA/ec6v+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Apr 03, 2015 at 10:05:19PM +0800, Fam Zheng wrote: > This is necessary to suppress more IO requests from being generated from > block job coroutines. >=20 > Signed-off-by: Fam Zheng > --- > block.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) >=20 > diff --git a/block.c b/block.c > index f2f8ae7..00cd91e 100644 > --- a/block.c > +++ b/block.c > @@ -2033,6 +2033,16 @@ void bdrv_drain_all(void) > bool busy =3D true; > BlockDriverState *bs; > =20 > + QTAILQ_FOREACH(bs, &bdrv_states, device_list) { > + AioContext *aio_context =3D bdrv_get_aio_context(bs); > + > + aio_context_acquire(aio_context); > + if (bs->job) { > + block_job_pause(bs->job); > + } > + aio_context_release(aio_context); > + } > + > while (busy) { > busy =3D false; > =20 > @@ -2044,6 +2054,16 @@ void bdrv_drain_all(void) > aio_context_release(aio_context); > } > } > + > + QTAILQ_FOREACH(bs, &bdrv_states, device_list) { > + AioContext *aio_context =3D bdrv_get_aio_context(bs); > + > + aio_context_acquire(aio_context); > + if (bs->job) { > + block_job_resume(bs->job); > + } > + aio_context_release(aio_context); > + } > } There is a tiny chance that we pause a job (which actually just sets job->paused =3D true but there's no guarantee the job's coroutine reacts to this) right before it terminates. Then aio_poll() enters the coroutine one last time and the job terminates. We then reach the resume portion of bdrv_drain_all() and the job no longer exists. Hopefully nothing started a new job in the meantime. bs->job should either be the original block job or NULL. This code seems under current assumptions, but I just wanted to raise these issues in case someone sees problems that I've missed. Stefan --9jHkwA2TBA/ec6v+ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVJQUAAAoJEJykq7OBq3PI7p0H/0WyCpJXLVQ3PmVF967sxz9G zGw4QcN4utScCk+en9oeryQI3sdIrNY1o5uQjTbf0MrbWykKfnawHAxhYRL+re98 jxRRHLPXOXs/qOUl2PLoYlnvGso2o3V/8Wbdn3YlPY/zjGv8MwuX9VuWuIyCQc9W QdiJc59K1tB2y6bH9Gg4CuFQs7N2IV003Dj5XO7xLymT2OBj8fwdEf0DT0aTLHaQ vwxhx98cyTAjFaO05RlVkA+MamFXhJYFQIMyzsGIhznNO7YcsrKV/DMKrXMDvyA1 j4JrMLd40xC6iICU7VOCyaFScZVpqVjcnsBWk8UQhKJVXmfLUvOfz8m/2r56ga0= =yFPh -----END PGP SIGNATURE----- --9jHkwA2TBA/ec6v+--