Re: [Qemu-devel] [RFC 24/38] cpu-exec: reset mmap_lock after exiting the CPU loop

[Paolo Bonzini]

On 23/08/2015 17:23, Emilio G. Cota wrote:
> Otherwise after an exception we end up in a deadlock.

Can you explain better the path that exits cpu_exec with the lock taken?

Also, let's remove the recursive locking by introducing "mmap_lock()
already taken" variants of target_mprotect and target_mmap.

Paolo

> Signed-off-by: Emilio G. Cota <address@hidden>
> ---
>  bsd-user/mmap.c         | 12 ++++++++++++
>  cpu-exec.c              |  1 +
>  include/exec/exec-all.h |  2 ++
>  linux-user/mmap.c       |  8 ++++++++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 092bf7f..b37a8f5 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -48,6 +48,14 @@ void mmap_unlock(void)
>      }
>  }
>  
> +void mmap_lock_reset(void)
> +{
> +    while (mmap_lock_count) {
> +        mmap_lock_count--;
> +        pthread_mutex_unlock(&mmap_mutex);
> +    }
> +}
> +
>  /* Grab lock to make sure things are in a consistent state after fork().  */
>  void mmap_fork_start(void)
>  {
> @@ -72,6 +80,10 @@ void mmap_lock(void)
>  void mmap_unlock(void)
>  {
>  }
> +
> +void mmap_lock_reset(void)
> +{
> +}
>  #endif
>  
>  /* NOTE: all the constants are the HOST ones, but addresses are target. */
> diff --git a/cpu-exec.c b/cpu-exec.c
> index a1700ac..f758928 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -630,6 +630,7 @@ int cpu_exec(CPUState *cpu)
>              env = &x86_cpu->env;
>  #endif
>              tb_lock_reset();
> +            mmap_lock_reset();
>              cpu_exit_loop_lock_reset(cpu);
>          }
>      } /* for(;;) */
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index b1934bf..3b8399a 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -334,6 +334,7 @@ void tlb_fill(CPUState *cpu, target_ulong addr, int 
> is_write, int mmu_idx,
>  #if defined(CONFIG_USER_ONLY)
>  void mmap_lock(void);
>  void mmap_unlock(void);
> +void mmap_lock_reset(void);
>  
>  static inline tb_page_addr_t get_page_addr_code(CPUArchState *env1, 
> target_ulong addr)
>  {
> @@ -342,6 +343,7 @@ static inline tb_page_addr_t 
> get_page_addr_code(CPUArchState *env1, target_ulong
>  #else
>  static inline void mmap_lock(void) {}
>  static inline void mmap_unlock(void) {}
> +static inline void mmap_lock_reset(void) {}
>  
>  /* cputlb.c */
>  tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr);
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 78e1b2d..8ee80f5 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -51,6 +51,14 @@ void mmap_unlock(void)
>      }
>  }
>  
> +void mmap_lock_reset(void)
> +{
> +    if (mmap_lock_count) {
> +        mmap_lock_count = 0;
> +        pthread_mutex_unlock(&mmap_mutex);
> +    }
> +}
> +
>  /* Grab lock to make sure things are in a consistent state after fork().  */
>  void mmap_fork_start(void)
>  {
>