On 31/10/2017 12:26, Pavel Dovgalyuk wrote:
> From: Alex Bennée <alex.ben...@linaro.org>
>
> 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 <alex.ben...@linaro.org>
> Tested-by: Pavel Dovgalyuk <pavel.dovga...@ispras.ru>
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)
>
