Re: [Qemu-devel] [RFC v4 20/28] cpus: tweak sleeping and safe_work rules for MTTCG

[Paolo Bonzini]

On 11/08/2016 17:24, Alex Bennée wrote:
> Once TCG gains the ability to sleep individual threads we need to make
> sure they don't sleep when safe work is pending as all threads need to
> go through the process_queued_work function. Also if we have multiple
> threads wait_for_safe_work can now sleep without deadlocking.
> 
> Signed-off-by: Alex Bennée <address@hidden>
> 
> ---
> v4
>   - new for v4, ontop of async-safe-work-v5
> ---
>  cpu-exec-common.c | 15 ++++++++++++++-
>  cpus.c            |  2 +-
>  include/qom/cpu.h |  8 ++++++++
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/cpu-exec-common.c b/cpu-exec-common.c
> index e29cf6d..84cb789 100644
> --- a/cpu-exec-common.c
> +++ b/cpu-exec-common.c
> @@ -83,6 +83,18 @@ QemuCond qemu_exclusive_cond;
>  
>  static int safe_work_pending;
>  
> +/* No vCPUs can sleep while there is safe work pending as we need
> + * everything to finish up in process_cpu_work.
> + */
> +bool cpu_has_queued_work(CPUState *cpu)
> +{
> +    if (cpu->queued_work || atomic_mb_read(&safe_work_pending) > 0) {
> +        return true;
> +    } else {
> +        return false;
> +    }
> +}

This should not be needed anymore with the new version.  If a safe work
item is pending, its CPU will have cpu->queued_work != NULL.  The work
item will then run as soon as the CPU is scheduled because no other CPUs
can be inside TCG.

Paolo

>  #ifdef CONFIG_USER_ONLY
>  #define can_wait_for_safe() (1)
>  #else
> @@ -91,7 +103,8 @@ static int safe_work_pending;
>   * all vCPUs are in the same thread. This will change for MTTCG
>   * however.
>   */
> -#define can_wait_for_safe() (0)
> +extern int smp_cpus;
> +#define can_wait_for_safe() (mttcg_enabled && smp_cpus > 1)
>  #endif
>  
>  void wait_safe_cpu_work(void)
> diff --git a/cpus.c b/cpus.c
> index 8a40a08..4fc5e4c 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -88,7 +88,7 @@ bool cpu_is_stopped(CPUState *cpu)
>  
>  static bool cpu_thread_is_idle(CPUState *cpu)
>  {
> -    if (cpu->stop || cpu->queued_work) {
> +    if (cpu->stop || cpu_has_queued_work(cpu)) {
>          return false;
>      }
>      if (cpu_is_stopped(cpu)) {
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 5ecbd29..d0db846 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -661,6 +661,14 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func 
> func, void *data);
>  void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
>  
>  /**
> + * cpu_has_queued_work:
> + * @cpu: The vCPU to check
> + *
> + * Returns true if there is *_run_on_cpu work to be done.
> + */
> +bool cpu_has_queued_work(CPUState *cpu);
> +
> +/**
>   * qemu_get_cpu:
>   * @index: The address@hidden value of the CPU to obtain.
>   *
>