summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorLongpeng <longpeng2@huawei.com>2020-03-16 16:37:32 +0800
committerPaolo Bonzini <pbonzini@redhat.com>2020-03-16 23:02:26 +0100
commitf962cac4c24157aeceff59cbf9dac8b5e30c55da (patch)
tree4516763c219a4aebeb6f5e1fe9a93ce44dab8c1d
parent5073b5d3ea303d37f4a8e2ea451d7a2eb1817448 (diff)
downloadfocaccia-qemu-f962cac4c24157aeceff59cbf9dac8b5e30c55da.tar.gz
focaccia-qemu-f962cac4c24157aeceff59cbf9dac8b5e30c55da.zip
cpus: avoid pause_all_vcpus getting stuck due to race
We found an issue when repeat reboot in guest during migration, it cause the
migration thread never be waken up again.

<main loop>                        |<migration_thread>
                                   |
LOCK BQL                           |
...                                |
main_loop_should_exit              |
 pause_all_vcpus                   |
  1. set all cpus ->stop=true      |
     and then kick                 |
  2. return if all cpus is paused  |
     (by '->stopped == true'), else|
  3. qemu_cond_wait [BQL UNLOCK]   |
                                   |LOCK BQL
                                   |...
                                   |do_vm_stop
                                   | pause_all_vcpus
                                   |  (A)set all cpus ->stop=true
                                   |     and then kick
                                   |  (B)return if all cpus is paused
                                   |     (by '->stopped == true'), else
                                   |  (C)qemu_cond_wait [BQL UNLOCK]
  4. be waken up and LOCK BQL      |  (D)be waken up BUT wait for  BQL
  5. goto 2.                       |
 (BQL is still LOCKed)             |
 resume_all_vcpus                  |
  1. set all cpus ->stop=false     |
     and ->stopped=false           |
...                                |
BQL UNLOCK                         |  (E)LOCK BQL
                                   |  (F)goto B. [but stopped is false now!]
                                   |Finally, sleep at step 3 forever.

resume_all_vcpus should notice this race, so we need to move the change
of runstate before pause_all_vcpus in do_vm_stop() and ignore the resume
request if runstate is not running.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Longpeng <longpeng2@huawei.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200316083732.2010-1-longpeng2@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-rw-r--r--cpus.c6
1 files changed, 5 insertions, 1 deletions
diff --git a/cpus.c b/cpus.c
index b4f8b84b61..ef441bdf62 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1026,9 +1026,9 @@ static int do_vm_stop(RunState state, bool send_stop)
     int ret = 0;
 
     if (runstate_is_running()) {
+        runstate_set(state);
         cpu_disable_ticks();
         pause_all_vcpus();
-        runstate_set(state);
         vm_state_notify(0, state);
         if (send_stop) {
             qapi_event_send_stop();
@@ -1899,6 +1899,10 @@ void resume_all_vcpus(void)
 {
     CPUState *cpu;
 
+    if (!runstate_is_running()) {
+        return;
+    }
+
     qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true);
     CPU_FOREACH(cpu) {
         cpu_resume(cpu);