From patchwork Fri Jun 17 13:06:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amit Shah X-Patchwork-Id: 637003 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rWLD92BVPz9t1J for ; Fri, 17 Jun 2016 23:08:41 +1000 (AEST) Received: from localhost ([::1]:57366 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDtVn-0000PU-8M for incoming@patchwork.ozlabs.org; Fri, 17 Jun 2016 09:08:39 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55866) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDtUP-0007OL-SS for qemu-devel@nongnu.org; Fri, 17 Jun 2016 09:07:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDtUN-00084d-Ff for qemu-devel@nongnu.org; Fri, 17 Jun 2016 09:07:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40372) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDtUN-00084Y-7U for qemu-devel@nongnu.org; Fri, 17 Jun 2016 09:07:11 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D9C24BB06; Fri, 17 Jun 2016 13:07:10 +0000 (UTC) Received: from localhost (ovpn-116-17.sin2.redhat.com [10.67.116.17]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5HD787b027454; Fri, 17 Jun 2016 09:07:09 -0400 From: Amit Shah To: Peter Maydell Date: Fri, 17 Jun 2016 18:36:43 +0530 Message-Id: <73a8912b8aeed1c992e3f9cb4880e9d1edb935de.1466168448.git.amit.shah@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 17 Jun 2016 13:07:10 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 04/13] migration: Fix multi-thread compression bug X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juan Quintela , liang.z.li@intel.com, qemu list , "Dr. David Alan Gilbert" , Amit Shah , den@openvz.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Liang Li Recently, a bug related to multiple thread compression feature for live migration is reported. The destination side will be blocked during live migration if there are heavy workload in host and memory intensive workload in guest, this is most likely to happen when there is one decompression thread. Some parts of the decompression code are incorrect: 1. The main thread receives data from source side will enter a busy loop to wait for a free decompression thread. 2. A lock is needed to protect the decomp_param[idx]->start, because it is checked in the main thread and is updated in the decompression thread. Fix these two issues by following the code pattern for compression. Signed-off-by: Liang Li Reported-by: Daniel P. Berrange Reviewed-by: Daniel P. Berrange Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Tested-by: Daniel P. Berrange Signed-off-by: Liang Li Message-Id: <1462433579-13691-2-git-send-email-liang.z.li@intel.com> Signed-off-by: Amit Shah --- migration/ram.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 42fb8ac..f3fe6c7 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -265,6 +265,7 @@ typedef struct CompressParam CompressParam; struct DecompressParam { bool start; + bool done; QemuMutex mutex; QemuCond cond; void *des; @@ -289,6 +290,8 @@ static bool quit_comp_thread; static bool quit_decomp_thread; static DecompressParam *decomp_param; static QemuThread *decompress_threads; +static QemuMutex decomp_done_lock; +static QemuCond decomp_done_cond; static int do_compress_ram_page(CompressParam *param); @@ -834,6 +837,7 @@ static inline void start_compression(CompressParam *param) static inline void start_decompression(DecompressParam *param) { + param->done = false; qemu_mutex_lock(¶m->mutex); param->start = true; qemu_cond_signal(¶m->cond); @@ -2196,19 +2200,24 @@ static void *do_data_decompress(void *opaque) qemu_mutex_lock(¶m->mutex); while (!param->start && !quit_decomp_thread) { qemu_cond_wait(¶m->cond, ¶m->mutex); + } + if (!quit_decomp_thread) { 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; + /* 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(¶m->mutex); + + qemu_mutex_lock(&decomp_done_lock); + param->done = true; + qemu_cond_signal(&decomp_done_cond); + qemu_mutex_unlock(&decomp_done_lock); } return NULL; @@ -2222,10 +2231,13 @@ void migrate_decompress_threads_create(void) decompress_threads = g_new0(QemuThread, thread_count); decomp_param = g_new0(DecompressParam, thread_count); quit_decomp_thread = false; + qemu_mutex_init(&decomp_done_lock); + qemu_cond_init(&decomp_done_cond); for (i = 0; i < thread_count; i++) { qemu_mutex_init(&decomp_param[i].mutex); qemu_cond_init(&decomp_param[i].cond); decomp_param[i].compbuf = g_malloc0(compressBound(TARGET_PAGE_SIZE)); + decomp_param[i].done = true; qemu_thread_create(decompress_threads + i, "decompress", do_data_decompress, decomp_param + i, QEMU_THREAD_JOINABLE); @@ -2261,9 +2273,10 @@ static void decompress_data_with_multi_threads(QEMUFile *f, int idx, thread_count; thread_count = migrate_decompress_threads(); + qemu_mutex_lock(&decomp_done_lock); while (true) { for (idx = 0; idx < thread_count; idx++) { - if (!decomp_param[idx].start) { + if (decomp_param[idx].done) { qemu_get_buffer(f, decomp_param[idx].compbuf, len); decomp_param[idx].des = host; decomp_param[idx].len = len; @@ -2273,8 +2286,11 @@ static void decompress_data_with_multi_threads(QEMUFile *f, } if (idx < thread_count) { break; + } else { + qemu_cond_wait(&decomp_done_cond, &decomp_done_lock); } } + qemu_mutex_unlock(&decomp_done_lock); } /*