From: Thomas Huth <thuth@redhat.com>
To: Greg Kurz <gkurz@linux.vnet.ibm.com>,
David Gibson <david@gibson.dropbear.id.au>
Cc: Cedric Le Goater <clg@fr.ibm.com>,
qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
Ladi Prosek <lprosek@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] spapr_rng: fix race with main loop
Date: Mon, 14 Mar 2016 11:51:19 +0100 [thread overview]
Message-ID: <56E697A7.2010209@redhat.com> (raw)
In-Reply-To: <20160311184514.2768.97728.stgit@bahia.huguette.org>
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.
>
> The following path is taken without holding the lock to the main loop mutex:
>
> h_random()
> rng_backend_request_entropy()
> rng_random_request_entropy()
> qemu_set_fd_handler()
>
> The consequence is that entropy_available() may be called before the vCPU
> thread could even queue the request: depending on the scheduling, it may
> happen that entropy_available() does not call random_recv()->qemu_sem_post().
> The vCPU thread will then sleep forever in h_random()->qemu_sem_wait().
>
> This could not happen before 60253ed1e6ec because entropy_available() used
> to call random_recv() unconditionally.
>
> This patch ensures the lock is held to avoid the race.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>
> Thomas,
>
> This is the problem mentioned by Cedric in:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg02526.html
>
> Cheers.
>
> --
> Greg
>
> hw/ppc/spapr_rng.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> 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, sPAPRMachineState *spapr,
> hrdata.val.v64 = 0;
> hrdata.received = 0;
>
> - qemu_mutex_unlock_iothread();
> while (hrdata.received < 8) {
> rng_backend_request_entropy(rngstate->backend, 8 - hrdata.received,
> random_recv, &hrdata);
> + qemu_mutex_unlock_iothread();
> qemu_sem_wait(&hrdata.sem);
> + qemu_mutex_lock_iothread();
> }
> - qemu_mutex_lock_iothread();
>
> qemu_sem_destroy(&hrdata.sem);
> args[0] = hrdata.val.v64;
Reviewed-by: Thomas Huth <thuth@redhat.com>
next prev parent reply other threads:[~2016-03-14 10:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-11 18:48 [Qemu-devel] [PATCH] spapr_rng: fix race with main loop Greg Kurz
2016-03-14 10:03 ` Cédric Le Goater
2016-03-14 10:51 ` Thomas Huth [this message]
2016-03-15 0:30 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56E697A7.2010209@redhat.com \
--to=thuth@redhat.com \
--cc=clg@fr.ibm.com \
--cc=david@gibson.dropbear.id.au \
--cc=gkurz@linux.vnet.ibm.com \
--cc=lprosek@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.