From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48859) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZiH1-0007Cs-Ur for qemu-devel@nongnu.org; Thu, 02 Oct 2014 11:26:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XZiGs-0007v8-TH for qemu-devel@nongnu.org; Thu, 02 Oct 2014 11:26:31 -0400 Received: from mail-wi0-x22d.google.com ([2a00:1450:400c:c05::22d]:42092) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZiGs-0007uw-MS for qemu-devel@nongnu.org; Thu, 02 Oct 2014 11:26:22 -0400 Received: by mail-wi0-f173.google.com with SMTP id bs8so4391507wib.6 for ; Thu, 02 Oct 2014 08:26:21 -0700 (PDT) Date: Thu, 2 Oct 2014 16:26:18 +0100 From: Stefan Hajnoczi Message-ID: <20141002152618.GH6250@stefanha-thinkpad.redhat.com> References: <1412048118-24834-1-git-send-email-famz@redhat.com> <1412048118-24834-2-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="svZFHVx8/dhPCe52" Content-Disposition: inline In-Reply-To: <1412048118-24834-2-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 1/2] main-loop: Pass AioContext into qemu_poll_ns List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: kwolf@redhat.com, Paolo Bonzini , qemu-devel@nongnu.org, Stefan Hajnoczi --svZFHVx8/dhPCe52 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 30, 2014 at 11:35:17AM +0800, Fam Zheng wrote: > diff --git a/main-loop.c b/main-loop.c > index d2e64f1..4641ef4 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -234,7 +234,8 @@ static int os_host_main_loop_wait(int64_t timeout) > spin_counter++; > } > =20 > - ret =3D qemu_poll_ns((GPollFD *)gpollfds->data, gpollfds->len, timeo= ut); > + ret =3D qemu_poll_ns(qemu_aio_context, (GPollFD *)gpollfds->data, > + gpollfds->len, timeout); > =20 > if (timeout) { > qemu_mutex_lock_iothread(); > @@ -439,7 +440,8 @@ static int os_host_main_loop_wait(int64_t timeout) > poll_timeout_ns =3D qemu_soonest_timeout(poll_timeout_ns, timeout); > =20 > qemu_mutex_unlock_iothread(); > - g_poll_ret =3D qemu_poll_ns(poll_fds, n_poll_fds + w->num, poll_time= out_ns); > + g_poll_ret =3D qemu_poll_ns(qemu_aio_context, > + poll_fds, n_poll_fds + w->num, poll_timeou= t_ns); > =20 > qemu_mutex_lock_iothread(); > if (g_poll_ret > 0) { This is a hack. The immediate problem is that we're not holding the QEMU global mutex but are accessing qemu_aio_context. What if other threads touch qemu_aio_context while they hold the QEMU global mutex? You didn't indicate what thread-safety rules need to be obeyed so I wonder whether you looked into this. I'm also concerned that this is somewhat abusing AioContext. You want to stash fields in there but this poll call is about more than just AioContext, it also includes non-AioContext file descriptors. So it seems like a kludge to put the fields into AioContext. Can you put the state into a separate struct so it's easy to understand its thread-safety characteristics? Stefan --svZFHVx8/dhPCe52 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJULW6aAAoJEJykq7OBq3PId8gH/A6RjKu+uX4J7gnsGynRnc8W Re0+3/Bk0GdVK2d3R5KI2bouyUsTQ/ePrhi9d1NApjSH0tWe6a0sqdKBAFZcTrDJ 4+MLBl3q0xICCuhg6lBDiRKep2JuhLZk6u2u+AXGCCv8KJNoNCOe1DJo00atgdRu HQmkb1hhrFNEM3ad4vBQ4/SbzvEcxXXYe1HYiq9CAmDS/7YB0h+et1woAMYVsScA SsKzSL9jlnkUZ08NifRA+bUjtnYCGb8nHi4OX7s024Hz2J8x8OkpqeE0AsYK2Dp9 oMjYbWbodUwbU8cyfIdD1Cx9rs3XSyMYfXMbW5+eFG0BL+98AdRoSz0BaVUqGHE= =X0sT -----END PGP SIGNATURE----- --svZFHVx8/dhPCe52--