From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33697) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtIpa-0001dn-LR for qemu-devel@nongnu.org; Tue, 25 Nov 2014 11:19:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtIpQ-0000LH-AN for qemu-devel@nongnu.org; Tue, 25 Nov 2014 11:19:10 -0500 Received: from mail-wg0-x22e.google.com ([2a00:1450:400c:c00::22e]:60804) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtIpP-0000L9-V6 for qemu-devel@nongnu.org; Tue, 25 Nov 2014 11:19:00 -0500 Received: by mail-wg0-f46.google.com with SMTP id x12so1310032wgg.33 for ; Tue, 25 Nov 2014 08:18:59 -0800 (PST) Date: Tue, 25 Nov 2014 16:18:56 +0000 From: Stefan Hajnoczi Message-ID: <20141125161856.GA22421@stefanha-thinkpad.redhat.com> References: <1416900193-3763-1-git-send-email-ming.lei@canonical.com> <1416900193-3763-2-git-send-email-ming.lei@canonical.com> <20141125130804.GE21126@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="W/nzBZO5zC0uMSeA" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v6 1/3] linux-aio: fix submit aio as a batch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei Cc: Kevin Wolf , Paolo Bonzini , qemu-devel , Stefan Hajnoczi --W/nzBZO5zC0uMSeA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 25, 2014 at 10:45:05PM +0800, Ming Lei wrote: > On Tue, Nov 25, 2014 at 9:08 PM, Stefan Hajnoczi wr= ote: > > On Tue, Nov 25, 2014 at 03:23:11PM +0800, Ming Lei wrote: > >> @@ -296,12 +370,14 @@ void laio_detach_aio_context(void *s_, AioContex= t *old_context) > >> > >> aio_set_event_notifier(old_context, &s->e, NULL); > >> qemu_bh_delete(s->completion_bh); > >> + qemu_bh_delete(s->io_q.abort_bh); > >> } > >> > >> void laio_attach_aio_context(void *s_, AioContext *new_context) > >> { > >> struct qemu_laio_state *s =3D s_; > >> > >> + s->io_q.abort_bh =3D aio_bh_new(new_context, ioq_abort_bh, s); > >> s->completion_bh =3D aio_bh_new(new_context, qemu_laio_completion= _bh, s); > >> aio_set_event_notifier(new_context, &s->e, qemu_laio_completion_c= b); > >> } > > > > These functions are incomplete when ->aborting =3D=3D true. I can't th= ink >=20 > Could you explain in a bit when ->aborting is true during attach callback? >=20 > > of a reason why we are guaranteed never to hit that state, and fixing it > > is easy. Just add the following to the end of > > laio_attach_aio_context(): > > > > if (s->aborting) { > > qemu_bh_schedule(s->io_q.abort_bh); > > } >=20 > You mean the abort BH may not have chance to run before its deletion > in the detach callback? Exactly. Any time you schedule a BH you need to be aware of things that may happen before the BH is invoked. > If so, bdrv_drain_all() from bdrv_set_aio_context() should have > handled the pending BH, right? I'm not sure if it's good to make subtle assumptions like that. If the code changes they will break. Since it is very easy to protect against this case (the code I posted before), it seems worthwhile to be on the safe side. Stefan --W/nzBZO5zC0uMSeA Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUdKvwAAoJEJykq7OBq3PIwxoIAKWwYGB0TO2HYcUvvkURAdak gRBBFtncxMGVSGMM/sXHrDat3MvDcqiZADs3Mu61igH/RL8Cn1/iXqOfm2K6+WNC yK5SBd9v5/DpluOUm0SRvrzdUj5l1lz7gZT+R5CKlPtSQQ803zRiM27L4IZOlGMA QuQAnCMX7YfTEKCtuxK3oWZEgTSmyNU2kzVy1Oy/gwJKPki/P/6P6VawpQmTgK0I lpmGJtXUGWFphYSk+4JagAhKsOJgY/0olgMh8pXxzy6KFTN1gbDD1gKVgyoWewT9 MfMUGK8cz9GkjZyi5uW1p+Td0fx3r8ZeBUDDh1vxkAvoDjQtG4JDbaiyYV5Djto= =F57N -----END PGP SIGNATURE----- --W/nzBZO5zC0uMSeA--