Re: [Qemu-devel] [PATCH 1/2] CPUArchState: introduce per-cpu lock

[Anthony Liguori <anthony@codemonkey.ws>]

On 06/21/2012 09:49 AM, Liu Ping Fan wrote:
> introduce a lock for per-cpu to protect agaist accesing from
> other vcpu thread.
>
> Signed-off-by: Liu Ping Fan<pingfank@linux.vnet.ibm.com>
> ---
>   cpu-defs.h  |    2 ++
>   cpus.c      |   17 +++++++++++++++++
>   main-loop.h |    3 +++
>   3 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-defs.h b/cpu-defs.h
> index f49e950..7305822 100644
> --- a/cpu-defs.h
> +++ b/cpu-defs.h
> @@ -30,6 +30,7 @@
>   #include "osdep.h"
>   #include "qemu-queue.h"
>   #include "targphys.h"
> +#include "qemu-thread-posix.h"

qemu-thread.h?

I would strongly suspect qemu-thread-posix would break the windows build...

>
>   #ifndef TARGET_LONG_BITS
>   #error TARGET_LONG_BITS must be defined before including this header
> @@ -220,6 +221,7 @@ typedef struct CPUWatchpoint {
>       CPU_COMMON_THREAD                                                   \
>       struct QemuCond *halt_cond;                                         \
>       int thread_kicked;                                                  \
> +    struct QemuMutex *cpu_lock;                                         \
>       struct qemu_work_item *queued_work_first, *queued_work_last;        \
>       const char *cpu_model_str;                                          \
>       struct KVMState *kvm_state;                                         \
> diff --git a/cpus.c b/cpus.c
> index b182b3d..554f7bc 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -735,6 +735,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>       env->thread_id = qemu_get_thread_id();
>       cpu_single_env = env;
>
> +

Please be careful about introducing spurious whitespace.

>       r = kvm_init_vcpu(env);
>       if (r<  0) {
>           fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
> @@ -891,6 +892,20 @@ int qemu_cpu_is_self(void *_env)
>       return qemu_thread_is_self(env->thread);
>   }
>
> +void qemu_mutex_lock_cpu(void *_env)
> +{
> +    CPUArchState *env = _env;
> +
> +    qemu_mutex_lock(env->cpu_lock);
> +}
> +
> +void qemu_mutex_unlock_cpu(void *_env)
> +{
> +    CPUArchState *env = _env;
> +
> +    qemu_mutex_unlock(env->cpu_lock);
> +}
> +

See CODING_STYLE.  _ as the start of a variable name is not allowed.

>   void qemu_mutex_lock_iothread(void)
>   {
>       if (!tcg_enabled()) {
> @@ -1027,6 +1042,8 @@ void qemu_init_vcpu(void *_env)
>       env->nr_cores = smp_cores;
>       env->nr_threads = smp_threads;
>       env->stopped = 1;
> +    env->cpu_lock = g_malloc0(sizeof(QemuMutex));
> +    qemu_mutex_init(env->cpu_lock);
>       if (kvm_enabled()) {
>           qemu_kvm_start_vcpu(env);
>       } else if (tcg_enabled()) {
> diff --git a/main-loop.h b/main-loop.h
> index dce1cd9..d8d44a4 100644
> --- a/main-loop.h
> +++ b/main-loop.h
> @@ -323,6 +323,9 @@ void qemu_bh_delete(QEMUBH *bh);
>   int qemu_add_child_watch(pid_t pid);
>   #endif
>
> +void qemu_mutex_lock_cpu(void *_env);
> +void qemu_mutex_unlock_cpu(void *_env);
> +
>   /**
>    * qemu_mutex_lock_iothread: Lock the main loop mutex.
>    *