[RFC] cpus: avoid get stuck in pause_all_vcpus

* [RFC] cpus: avoid get stuck in pause_all_vcpus
@ 2020-03-10  9:14 Longpeng(Mike)
  2020-03-10 10:20 ` no-reply
  2020-03-12 15:28 ` Paolo Bonzini
  0 siblings, 2 replies; 9+ messages in thread
From: Longpeng(Mike) @ 2020-03-10  9:14 UTC (permalink / raw)
  To: pbonzini, rth
  Cc: Peter Maydell, Dr . David Alan Gilbert,
	qemu-devel @ nongnu . org, arei.gonglei, huangzhichao, Longpeng

From: Longpeng <longpeng2@huawei.com>

We find 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.


Note: This patch is just for discuss this issue, I'm looking forward to
      your suggestions, thanks!

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Signed-off-by: Longpeng <longpeng2@huawei.com>
---
 cpus.c | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/cpus.c b/cpus.c
index b4f8b84..15e8b21 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1857,10 +1857,30 @@ static bool all_vcpus_paused(void)
     return true;
 }
 
+static bool all_vcpus_resumed(void)
+{
+    CPUState *cpu;
+
+    CPU_FOREACH(cpu) {
+        if (cpu->stopped) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void pause_all_vcpus(void)
 {
     CPUState *cpu;
 
+    /* We need to drop the replay_lock so any vCPU threads woken up
+     * can finish their replay tasks
+     */
+retry_unlock:
+    replay_mutex_unlock();
+
+retry_pause:
     qemu_clock_enable(QEMU_CLOCK_VIRTUAL, false);
     CPU_FOREACH(cpu) {
         if (qemu_cpu_is_self(cpu)) {
@@ -1871,13 +1891,17 @@ void pause_all_vcpus(void)
         }
     }
 
-    /* We need to drop the replay_lock so any vCPU threads woken up
-     * can finish their replay tasks
-     */
-    replay_mutex_unlock();
-
     while (!all_vcpus_paused()) {
         qemu_cond_wait(&qemu_pause_cond, &qemu_global_mutex);
+        /*
+         * All of the vcpus maybe resumed due to the race with other
+         * threads that doing pause && resume, and we'll stuck as a
+         * result. So we need to request again if the race occurs.
+         */
+        if (all_vcpus_resumed()) {
+            goto retry_pause;
+        }
+
         CPU_FOREACH(cpu) {
             qemu_cpu_kick(cpu);
         }
@@ -1886,6 +1910,13 @@ void pause_all_vcpus(void)
     qemu_mutex_unlock_iothread();
     replay_mutex_lock();
     qemu_mutex_lock_iothread();
+    /*
+     * The vcpus maybe resumed during the mutex is unlocking, we must
+     * make sure all of the vcpus are paused before return.
+     */
+    if (!all_vcpus_paused()) {
+        goto retry_unlock;
+    }
 }
 
 void cpu_resume(CPUState *cpu)
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread