[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 13/26] cpus: only take BQL for sleeping thre
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC PATCH 13/26] cpus: only take BQL for sleeping threads |
Date: |
Thu, 2 Nov 2017 12:08:32 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 31/10/2017 12:26, Pavel Dovgalyuk wrote:
> From: Alex Bennée <address@hidden>
>
> Now the only real need to hold the BQL is for when we sleep on the
> cpu->halt conditional. The lock is actually dropped while the thread
> sleeps so the actual window for contention is pretty small. This also
> means we can remove the special case hack for exclusive work and
> simply declare that work no longer has an implicit BQL held. This
> isn't a major problem async work is generally only changing things in
> the context of its own vCPU. If it needs to work across vCPUs it
> should be using the exclusive mechanism or possibly taking the lock
> itself.
>
> Signed-off-by: Alex Bennée <address@hidden>
> Tested-by: Pavel Dovgalyuk <address@hidden>
At least cpu_throttle_thread would fail with this patch.
Also I am not sure if the s390 SIGP handlers are ready for this.
Paolo
>
> ---
> cpus-common.c | 13 +++++--------
> cpus.c | 10 ++++------
> 2 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/cpus-common.c b/cpus-common.c
> index 59f751e..64661c3 100644
> --- a/cpus-common.c
> +++ b/cpus-common.c
> @@ -310,6 +310,11 @@ void async_safe_run_on_cpu(CPUState *cpu,
> run_on_cpu_func func,
> queue_work_on_cpu(cpu, wi);
> }
>
> +/* Work items run outside of the BQL. This is essential for avoiding a
> + * deadlock for exclusive work but also applies to non-exclusive work.
> + * If the work requires cross-vCPU changes then it should use the
> + * exclusive mechanism.
> + */
> void process_queued_cpu_work(CPUState *cpu)
> {
> struct qemu_work_item *wi;
> @@ -327,17 +332,9 @@ void process_queued_cpu_work(CPUState *cpu)
> }
> qemu_mutex_unlock(&cpu->work_mutex);
> if (wi->exclusive) {
> - /* Running work items outside the BQL avoids the following
> deadlock:
> - * 1) start_exclusive() is called with the BQL taken while
> another
> - * CPU is running; 2) cpu_exec in the other CPU tries to takes
> the
> - * BQL, so it goes to sleep; start_exclusive() is sleeping too,
> so
> - * neither CPU can proceed.
> - */
> - qemu_mutex_unlock_iothread();
> start_exclusive();
> wi->func(cpu, wi->data);
> end_exclusive();
> - qemu_mutex_lock_iothread();
> } else {
> wi->func(cpu, wi->data);
> }
> diff --git a/cpus.c b/cpus.c
> index efde5c1..de6dfce 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1127,31 +1127,29 @@ static bool qemu_tcg_should_sleep(CPUState *cpu)
>
> static void qemu_tcg_wait_io_event(CPUState *cpu)
> {
> - qemu_mutex_lock_iothread();
>
> while (qemu_tcg_should_sleep(cpu)) {
> + qemu_mutex_lock_iothread();
> stop_tcg_kick_timer();
> qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
> + qemu_mutex_unlock_iothread();
> }
>
> start_tcg_kick_timer();
>
> qemu_wait_io_event_common(cpu);
> -
> - qemu_mutex_unlock_iothread();
> }
>
> static void qemu_kvm_wait_io_event(CPUState *cpu)
> {
> - qemu_mutex_lock_iothread();
>
> while (cpu_thread_is_idle(cpu)) {
> + qemu_mutex_lock_iothread();
> qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
> + qemu_mutex_unlock_iothread();
> }
>
> qemu_wait_io_event_common(cpu);
> -
> - qemu_mutex_unlock_iothread();
> }
>
> static void *qemu_kvm_cpu_thread_fn(void *arg)
>
- Re: [Qemu-devel] [RFC PATCH 13/26] cpus: only take BQL for sleeping threads,
Paolo Bonzini <=