[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 02/74] cpu: rename cpu->work_mutex to cpu->lock
From: |
Alex Bennée |
Subject: |
Re: [PATCH v8 02/74] cpu: rename cpu->work_mutex to cpu->lock |
Date: |
Mon, 11 May 2020 15:48:44 +0100 |
User-agent: |
mu4e 1.4.5; emacs 28.0.50 |
Robert Foley <address@hidden> writes:
> From: "Emilio G. Cota" <address@hidden>
>
> This lock will soon protect more fields of the struct. Give
> it a more appropriate name.
Hmm while bisecting to find another problem I found this commit:
/home/alex/lsrc/qemu.git/hw/core/cpu.c: In function ‘cpu_common_finalize’:
/home/alex/lsrc/qemu.git/hw/core/cpu.c:383:27: error: incompatible type for
argument 1 of ‘qemu_mutex_destroy’
qemu_mutex_destroy(cpu->lock);
~~~^~~~~~
In file included from /home/alex/lsrc/qemu.git/include/hw/core/cpu.h:31,
from /home/alex/lsrc/qemu.git/hw/core/cpu.c:23:
/home/alex/lsrc/qemu.git/include/qemu/thread.h:26:36: note: expected
‘QemuMutex *’ {aka ‘struct QemuMutex *’} but argument is of type ‘QemuMutex’
{aka ‘struct QemuMutex’}
void qemu_mutex_destroy(QemuMutex *mutex);
~~~~~~~~~~~^~~~~
make: *** [/home/alex/lsrc/qemu.git/rules.mak:69: hw/core/cpu.o] Error 1
which works fine in the final product so I suspect something has slipped
between commits somewhere.
>
> Reviewed-by: Richard Henderson <address@hidden>
> Reviewed-by: Alex Bennée <address@hidden>
> Signed-off-by: Emilio G. Cota <address@hidden>
> Signed-off-by: Robert Foley <address@hidden>
> ---
> cpus-common.c | 14 +++++++-------
> cpus.c | 4 ++--
> hw/core/cpu.c | 4 ++--
> include/hw/core/cpu.h | 6 ++++--
> 4 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/cpus-common.c b/cpus-common.c
> index 3d659df464..f75cae23d9 100644
> --- a/cpus-common.c
> +++ b/cpus-common.c
> @@ -107,10 +107,10 @@ struct qemu_work_item {
>
> static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
> {
> - qemu_mutex_lock(&cpu->work_mutex);
> + qemu_mutex_lock(&cpu->lock);
> QSIMPLEQ_INSERT_TAIL(&cpu->work_list, wi, node);
> wi->done = false;
> - qemu_mutex_unlock(&cpu->work_mutex);
> + qemu_mutex_unlock(&cpu->lock);
>
> qemu_cpu_kick(cpu);
> }
> @@ -304,15 +304,15 @@ void process_queued_cpu_work(CPUState *cpu)
> {
> struct qemu_work_item *wi;
>
> - qemu_mutex_lock(&cpu->work_mutex);
> + qemu_mutex_lock(&cpu->lock);
> if (QSIMPLEQ_EMPTY(&cpu->work_list)) {
> - qemu_mutex_unlock(&cpu->work_mutex);
> + qemu_mutex_unlock(&cpu->lock);
> return;
> }
> while (!QSIMPLEQ_EMPTY(&cpu->work_list)) {
> wi = QSIMPLEQ_FIRST(&cpu->work_list);
> QSIMPLEQ_REMOVE_HEAD(&cpu->work_list, node);
> - qemu_mutex_unlock(&cpu->work_mutex);
> + qemu_mutex_unlock(&cpu->lock);
> if (wi->exclusive) {
> /* Running work items outside the BQL avoids the following
> deadlock:
> * 1) start_exclusive() is called with the BQL taken while
> another
> @@ -328,13 +328,13 @@ void process_queued_cpu_work(CPUState *cpu)
> } else {
> wi->func(cpu, wi->data);
> }
> - qemu_mutex_lock(&cpu->work_mutex);
> + qemu_mutex_lock(&cpu->lock);
> if (wi->free) {
> g_free(wi);
> } else {
> atomic_mb_set(&wi->done, true);
> }
> }
> - qemu_mutex_unlock(&cpu->work_mutex);
> + qemu_mutex_unlock(&cpu->lock);
> qemu_cond_broadcast(&qemu_work_cond);
> }
> diff --git a/cpus.c b/cpus.c
> index 151abbc04c..71bd2f5d55 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -100,9 +100,9 @@ static inline bool cpu_work_list_empty(CPUState *cpu)
> {
> bool ret;
>
> - qemu_mutex_lock(&cpu->work_mutex);
> + qemu_mutex_lock(&cpu->lock);
> ret = QSIMPLEQ_EMPTY(&cpu->work_list);
> - qemu_mutex_unlock(&cpu->work_mutex);
> + qemu_mutex_unlock(&cpu->lock);
> return ret;
> }
>
> diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> index 2fc6aa2159..bc0416829a 100644
> --- a/hw/core/cpu.c
> +++ b/hw/core/cpu.c
> @@ -367,7 +367,7 @@ static void cpu_common_initfn(Object *obj)
> cpu->nr_cores = 1;
> cpu->nr_threads = 1;
>
> - qemu_mutex_init(&cpu->work_mutex);
> + qemu_mutex_init(&cpu->lock);
> QSIMPLEQ_INIT(&cpu->work_list);
> QTAILQ_INIT(&cpu->breakpoints);
> QTAILQ_INIT(&cpu->watchpoints);
> @@ -379,7 +379,7 @@ static void cpu_common_finalize(Object *obj)
> {
> CPUState *cpu = CPU(obj);
>
> - qemu_mutex_destroy(&cpu->work_mutex);
> + qemu_mutex_destroy(cpu->lock);
> }
>
> static int64_t cpu_common_get_arch_id(CPUState *cpu)
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 398b65159e..0b75fdb093 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -331,7 +331,8 @@ struct qemu_work_item;
> * @opaque: User data.
> * @mem_io_pc: Host Program Counter at which the memory was accessed.
> * @kvm_fd: vCPU file descriptor for KVM.
> - * @work_mutex: Lock to prevent multiple access to @work_list.
> + * @lock: Lock to prevent multiple access to per-CPU fields. Must be acquired
> + * after the BQL.
> * @work_list: List of pending asynchronous work.
> * @trace_dstate_delayed: Delayed changes to trace_dstate (includes all
> changes
> * to @trace_dstate).
> @@ -375,7 +376,8 @@ struct CPUState {
> uint64_t random_seed;
> sigjmp_buf jmp_env;
>
> - QemuMutex work_mutex;
> + QemuMutex lock;
> + /* fields below protected by @lock */
> QSIMPLEQ_HEAD(, qemu_work_item) work_list;
>
> CPUAddressSpace *cpu_ases;
--
Alex Bennée
- Re: [PATCH v8 02/74] cpu: rename cpu->work_mutex to cpu->lock,
Alex Bennée <=