Re: [Qemu-devel] [PATCH 1/2] rcu: completely disable pthread_atfork callbacks as soon as possible

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> Because of -daemonize, system mode QEMU sometimes needs to fork() and
> keep RCU enabled in the child.  However, there is a possible deadlock
> with synchronize_rcu:
> 
> - the CPU thread is inside a RCU critical section and wants to take
>   the BQL in order to do MMIO
> 
> - the monitor thread, which is owning the BQL, calls rcu_init_lock
>   which tries to take the rcu_sync_lock
> 
> - the call_rcu thread has taken rcu_sync_lock in synchronize_rcu, but
>   synchronize_rcu needs the CPU thread to end the critical section
>   before returning.
> 
> This cannot happen for user-mode emulation, because it does not have
> a BQL.
> 
> To fix it, assume that system mode QEMU only forks in preparation for
> exec (except when daemonizing) and disable pthread_atfork as soon as
> the double fork has happened.
> 
> Reported-by: David Gilbert <dgilbert@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

That seems to fix that issue (not tested it any more than that though),
so for fixing that issue:

Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  include/qemu/rcu.h |  6 ++++++
>  util/rcu.c         | 20 ++++++++++++++++++++
>  vl.c               |  1 +
>  3 files changed, 27 insertions(+)
> 
> diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
> index 83ae2808be..c0da9907e8 100644
> --- a/include/qemu/rcu.h
> +++ b/include/qemu/rcu.h
> @@ -105,6 +105,12 @@ extern void synchronize_rcu(void);
>   */
>  extern void rcu_register_thread(void);
>  extern void rcu_unregister_thread(void);
> +
> +/*
> + * Support for fork().  fork() support is enabled at startup.
> + */
> +extern void rcu_enable_atfork(void);
> +extern void rcu_disable_atfork(void);
>  extern void rcu_after_fork(void);
>  
>  struct rcu_head;
> diff --git a/util/rcu.c b/util/rcu.c
> index 9adc5e4a36..2142ddd93b 100644
> --- a/util/rcu.c
> +++ b/util/rcu.c
> @@ -318,15 +318,35 @@ static void rcu_init_complete(void)
>      rcu_register_thread();
>  }
>  
> +static int atfork_depth = 1;
> +
> +void rcu_enable_atfork(void)
> +{
> +    atfork_depth++;
> +}
> +
> +void rcu_disable_atfork(void)
> +{
> +    atfork_depth--;
> +}
> +
>  #ifdef CONFIG_POSIX
>  static void rcu_init_lock(void)
>  {
> +    if (atfork_depth < 1) {
> +        return;
> +    }
> +
>      qemu_mutex_lock(&rcu_sync_lock);
>      qemu_mutex_lock(&rcu_registry_lock);
>  }
>  
>  static void rcu_init_unlock(void)
>  {
> +    if (atfork_depth < 1) {
> +        return;
> +    }
> +
>      qemu_mutex_unlock(&rcu_registry_lock);
>      qemu_mutex_unlock(&rcu_sync_lock);
>  }
> diff --git a/vl.c b/vl.c
> index 99fcfa0442..8967115514 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4121,6 +4121,7 @@ int main(int argc, char **argv, char **envp)
>      set_memory_options(&ram_slots, &maxram_size, machine_class);
>  
>      os_daemonize();
> +    rcu_disable_atfork();
>  
>      if (pid_file && qemu_create_pidfile(pid_file) != 0) {
>          error_report("could not acquire pid file: %s", strerror(errno));
> -- 
> 2.13.3
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK