All of lore.kernel.org
 help / color / mirror / Atom feed
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>

  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.