[Qemu-devel] [PATCH] migration: fix ram decompression race deadlock

[Maxim Nestratov]

The way how decompress_data_with_multi_threads communicates with
do_data_decompress is incorrect. Imagine the following scenario.
The function do_data_decompress just finished decompression and
released param->mutex and got preempted. In parallel, the function
decompress_data_with_multi_threads called start_decompression and
then it starts to loop infinitely waiting for decompression to
complete, which will never happend because decomp_param[idx].start
is true and do_data_decompress will never enter while loop again.
The patch fixes this problem by correcting while loop where we
wait for condition only and other actions are moved out of it.

Signed-off-by: Maxim Nestratov <address@hidden>
---
 migration/ram.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 3f05738..579bfc0 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2193,18 +2193,18 @@ static void *do_data_decompress(void *opaque)
         qemu_mutex_lock(&param->mutex);
         while (!param->start && !quit_decomp_thread) {
             qemu_cond_wait(&param->cond, &param->mutex);
-            pagesize = TARGET_PAGE_SIZE;
-            if (!quit_decomp_thread) {
-                /* uncompress() will return failed in some case, especially
-                 * when the page is dirted when doing the compression, it's
-                 * not a problem because the dirty page will be retransferred
-                 * and uncompress() won't break the data in other pages.
-                 */
-                uncompress((Bytef *)param->des, &pagesize,
-                           (const Bytef *)param->compbuf, param->len);
-            }
-            param->start = false;
         }
+        pagesize = TARGET_PAGE_SIZE;
+        if (!quit_decomp_thread) {
+           /* uncompress() will return failed in some case, especially
+            * when the page is dirted when doing the compression, it's
+            * not a problem because the dirty page will be retransferred
+            * and uncompress() won't break the data in other pages.
+            */
+            uncompress((Bytef *)param->des, &pagesize,
+                       (const Bytef *)param->compbuf, param->len);
+        }
+        param->start = false;
         qemu_mutex_unlock(&param->mutex);
     }
 
-- 
1.8.3.1