From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33058) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afQ5w-0001mt-43 for qemu-devel@nongnu.org; Mon, 14 Mar 2016 06:51:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afQ5s-0003oI-O1 for qemu-devel@nongnu.org; Mon, 14 Mar 2016 06:51:28 -0400 References: <20160311184514.2768.97728.stgit@bahia.huguette.org> From: Thomas Huth Message-ID: <56E697A7.2010209@redhat.com> Date: Mon, 14 Mar 2016 11:51:19 +0100 MIME-Version: 1.0 In-Reply-To: <20160311184514.2768.97728.stgit@bahia.huguette.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] spapr_rng: fix race with main loop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz , David Gibson Cc: Cedric Le Goater , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Ladi Prosek On 11.03.2016 19:48, Greg Kurz wrote: > Since commit "60253ed1e6ec rng: add request queue support to rng-random= ", > the use of a spapr_rng device may hang vCPU threads. >=20 > The following path is taken without holding the lock to the main loop m= utex: >=20 > h_random() > rng_backend_request_entropy() > rng_random_request_entropy() > qemu_set_fd_handler() >=20 > The consequence is that entropy_available() may be called before the vC= PU > thread could even queue the request: depending on the scheduling, it ma= y > happen that entropy_available() does not call random_recv()->qemu_sem_p= ost(). > The vCPU thread will then sleep forever in h_random()->qemu_sem_wait(). >=20 > This could not happen before 60253ed1e6ec because entropy_available() u= sed > to call random_recv() unconditionally. >=20 > This patch ensures the lock is held to avoid the race. >=20 > Signed-off-by: Greg Kurz > --- >=20 > Thomas, >=20 > This is the problem mentioned by Cedric in: >=20 > https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg02526.html >=20 > Cheers. >=20 > -- > Greg >=20 > hw/ppc/spapr_rng.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c > index a39d472b66fd..02d6be49f58e 100644 > --- a/hw/ppc/spapr_rng.c > +++ b/hw/ppc/spapr_rng.c > @@ -77,13 +77,13 @@ static target_ulong h_random(PowerPCCPU *cpu, sPAPR= MachineState *spapr, > hrdata.val.v64 =3D 0; > hrdata.received =3D 0; > =20 > - qemu_mutex_unlock_iothread(); > while (hrdata.received < 8) { > rng_backend_request_entropy(rngstate->backend, 8 - hrdata.rece= ived, > random_recv, &hrdata); > + qemu_mutex_unlock_iothread(); > qemu_sem_wait(&hrdata.sem); > + qemu_mutex_lock_iothread(); > } > - qemu_mutex_lock_iothread(); > =20 > qemu_sem_destroy(&hrdata.sem); > args[0] =3D hrdata.val.v64; Reviewed-by: Thomas Huth