QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Robert Foley <robert.foley@linaro.org>
Cc: peter.puhov@linaro.org, "Emilio G. Cota" <cota@braap.org>,
	richard.henderson@linaro.org, qemu-devel@nongnu.org
Subject: Re: [PATCH v8 02/74] cpu: rename cpu->work_mutex to cpu->lock
Date: Mon, 11 May 2020 15:48:44 +0100
Message-ID: <87r1vqfrxv.fsf@linaro.org> (raw)
In-Reply-To: <20200326193156.4322-3-robert.foley@linaro.org>


Robert Foley <robert.foley@linaro.org> writes:

> From: "Emilio G. Cota" <cota@braap.org>
>
> 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 <richard.henderson@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> ---
>  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


  reply index

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26 19:30 [PATCH v8 00/74] per-CPU locks Robert Foley
2020-03-26 19:30 ` [PATCH v8 01/74] cpu: convert queued work to a QSIMPLEQ Robert Foley
2020-03-26 19:30 ` [PATCH v8 02/74] cpu: rename cpu->work_mutex to cpu->lock Robert Foley
2020-05-11 14:48   ` Alex Bennée [this message]
2020-05-11 16:33     ` Robert Foley
2020-03-26 19:30 ` [PATCH v8 03/74] cpu: introduce cpu_mutex_lock/unlock Robert Foley
2020-05-11 10:24   ` Alex Bennée
2020-05-11 16:09     ` Robert Foley
2020-03-26 19:30 ` [PATCH v8 04/74] cpu: make qemu_work_cond per-cpu Robert Foley
2020-03-26 19:30 ` [PATCH v8 05/74] cpu: move run_on_cpu to cpus-common Robert Foley
2020-03-26 19:30 ` [PATCH v8 06/74] cpu: introduce process_queued_cpu_work_locked Robert Foley
2020-03-26 19:30 ` [PATCH v8 07/74] cpu: make per-CPU locks an alias of the BQL in TCG rr mode Robert Foley
2020-03-26 19:30 ` [PATCH v8 08/74] tcg-runtime: define helper_cpu_halted_set Robert Foley
2020-03-26 19:30 ` [PATCH v8 09/74] ppc: convert to helper_cpu_halted_set Robert Foley
2020-03-26 19:30 ` [PATCH v8 10/74] cris: " Robert Foley
2020-03-26 19:30 ` [PATCH v8 11/74] hppa: " Robert Foley
2020-03-26 19:30 ` [PATCH v8 12/74] m68k: " Robert Foley
2020-03-26 19:30 ` [PATCH v8 13/74] alpha: " Robert Foley
2020-03-26 19:30 ` [PATCH v8 14/74] microblaze: " Robert Foley
2020-03-26 19:30 ` [PATCH v8 15/74] cpu: define cpu_halted helpers Robert Foley
2020-03-26 19:30 ` [PATCH v8 16/74] tcg-runtime: convert to cpu_halted_set Robert Foley
2020-03-26 19:30 ` [PATCH v8 17/74] hw/semihosting: " Robert Foley
2020-05-11 10:25   ` Alex Bennée
2020-03-26 19:31 ` [PATCH v8 18/74] arm: convert to cpu_halted Robert Foley
2020-03-26 19:31 ` [PATCH v8 19/74] ppc: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 20/74] sh4: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 21/74] i386: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 22/74] lm32: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 23/74] m68k: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 24/74] mips: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 25/74] riscv: " Robert Foley
2020-05-11 10:40   ` Alex Bennée
2020-05-11 16:13     ` Robert Foley
2020-03-26 19:31 ` [PATCH v8 26/74] s390x: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 27/74] sparc: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 28/74] xtensa: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 29/74] gdbstub: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 30/74] openrisc: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 31/74] cpu-exec: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 32/74] cpu: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 33/74] cpu: define cpu_interrupt_request helpers Robert Foley
2020-03-26 19:31 ` [PATCH v8 34/74] ppc: use cpu_reset_interrupt Robert Foley
2020-03-26 19:31 ` [PATCH v8 35/74] exec: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 36/74] i386: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 37/74] s390x: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 38/74] openrisc: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 39/74] arm: convert to cpu_interrupt_request Robert Foley
2020-03-26 19:31 ` [PATCH v8 40/74] i386: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 41/74] i386/kvm: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 42/74] i386/hax-all: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 43/74] i386/whpx-all: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 44/74] i386/hvf: convert to cpu_request_interrupt Robert Foley
2020-03-26 19:31 ` [PATCH v8 45/74] ppc: convert to cpu_interrupt_request Robert Foley
2020-03-26 19:31 ` [PATCH v8 46/74] sh4: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 47/74] cris: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 48/74] hppa: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 49/74] lm32: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 50/74] m68k: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 51/74] mips: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 52/74] nios: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 53/74] s390x: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 54/74] alpha: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 55/74] moxie: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 56/74] sparc: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 57/74] openrisc: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 58/74] unicore32: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 59/74] microblaze: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 60/74] accel/tcg: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 61/74] cpu: convert to interrupt_request Robert Foley
2020-03-26 19:31 ` [PATCH v8 62/74] cpu: call .cpu_has_work with the CPU lock held Robert Foley
2020-03-26 19:31 ` [PATCH v8 63/74] cpu: introduce cpu_has_work_with_iothread_lock Robert Foley
2020-03-26 19:31 ` [PATCH v8 64/74] ppc: convert to cpu_has_work_with_iothread_lock Robert Foley
2020-03-26 19:31 ` [PATCH v8 65/74] mips: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 66/74] s390x: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 67/74] riscv: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 68/74] sparc: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 69/74] xtensa: " Robert Foley
2020-03-26 19:31 ` [PATCH v8 70/74] cpu: rename all_cpu_threads_idle to qemu_tcg_rr_all_cpu_threads_idle Robert Foley
2020-03-26 19:31 ` [PATCH v8 71/74] cpu: protect CPU state with cpu->lock instead of the BQL Robert Foley
2020-03-26 19:31 ` [PATCH v8 72/74] cpus-common: release BQL earlier in run_on_cpu Robert Foley
2020-03-26 19:31 ` [PATCH v8 73/74] cpu: add async_run_on_cpu_no_bql Robert Foley
2020-03-26 19:31 ` [PATCH v8 74/74] cputlb: queue async flush jobs without the BQL Robert Foley
2020-05-12 16:27   ` Alex Bennée
2020-05-12 19:26     ` Robert Foley
2020-05-18 13:46       ` Robert Foley
2020-05-20  4:46         ` Emilio G. Cota
2020-05-20 15:01           ` Robert Foley
2020-05-21 14:17             ` Robert Foley
2020-05-12 18:38   ` Alex Bennée
2020-03-26 22:58 ` [PATCH v8 00/74] per-CPU locks Aleksandar Markovic
2020-03-27  9:39   ` Alex Bennée
2020-03-27  9:50     ` Aleksandar Markovic
2020-03-27 10:24       ` Aleksandar Markovic
2020-03-27 17:21         ` Robert Foley
2020-03-27  5:14 ` Emilio G. Cota
2020-03-27 10:59   ` Philippe Mathieu-Daudé
2020-03-30  8:57     ` Stefan Hajnoczi
2020-03-27 18:23   ` Alex Bennée
2020-03-27 18:30   ` Robert Foley
2020-05-12 16:29 ` Alex Bennée

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r1vqfrxv.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=cota@braap.org \
    --cc=peter.puhov@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=robert.foley@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git