BQL now is used only for waiting for IO events.
This patch also removes lock/unlock from kvm module.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 accel/kvm/kvm-all.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9628512..d708c7f 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1861,7 +1861,6 @@ int kvm_cpu_exec(CPUState *cpu)
         return EXCP_HLT;
     }
 
-    qemu_mutex_unlock_iothread();
     cpu_exec_start(cpu);
     do {
         MemTxAttrs attrs;
@@ -1992,7 +1991,6 @@ int kvm_cpu_exec(CPUState *cpu)
     } while (ret == 0);
 
     cpu_exec_end(cpu);
-    qemu_mutex_lock_iothread();
 
     if (ret < 0) {
         cpu_dump_state(cpu, stderr, fprintf, CPU_DUMP_CODE);

On 23/01/2018 03:54, Pavel Dovgalyuk wrote:
> @@ -1861,7 +1861,6 @@ int kvm_cpu_exec(CPUState *cpu)
>          return EXCP_HLT;
>      }
>  
> -    qemu_mutex_unlock_iothread();
>      cpu_exec_start(cpu);
>      do {
>          MemTxAttrs attrs;


So this means that kvm_cpu_exec is now called without taking the BQL.
I'll leave aside the bisectability issue (patch 11 breaks kvm_cpu_exec,
because this qemu_mutex_unlock_iothread now has an assertion failure),
since they are easily fixed by squashing patches 11-13 together.

The lines immediately above are

    if (kvm_arch_process_async_events(cpu)) {
        atomic_set(&cpu->exit_request, 0);
        return EXCP_HLT;
    }

So this means that, after patch 11, kvm_arch_process_async_events went
from "called with BQL taken" to "called with BQL not taken".  And that
is completely broken, because it accesses cs->interrupt_request just
like cpu_has_work.  Previous reviews have ascertained that accessing
cs->interrupt_request requires taking the BQL; this is the same, except
worse because now we can even *write* cs->interrupt_request (clear bits)
without taking the lock.  I don't need to explain to you why this is bad.

         .------------------------------------------------.
         | .--------------------------------------------. |
         | | This is not how you are supposed to modify | |
         | |             multi-threaded code.           | |
         | '--------------------------------------------' |
         '------------------------------------------------'

If something can be accessed outside a lock, e.g. with atomics, that has
to be documented.  In addition, if it's not obvious whether a function
is called with a lock or without, you add comments that make it clear.
Take a lock at accel/tcg/translate-all.c or exec.c for examples.

This is the last pass through this series that I make.  I'll pick the
patches that I consider ready, for everything else you'll have to find a
reviewer that is willing to look through the series and vouch for it
with a "Reviewed-by".

Paolo