From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43195) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4k9X-00058t-3f for qemu-devel@nongnu.org; Mon, 23 May 2016 03:19:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4k9S-00055B-St for qemu-devel@nongnu.org; Mon, 23 May 2016 03:19:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4k9S-000551-Ky for qemu-devel@nongnu.org; Mon, 23 May 2016 03:19:46 -0400 Date: Mon, 23 May 2016 12:49:42 +0530 From: Amit Shah Message-ID: <20160523071942.GB24417@grmbl.mre> References: <1463124421-16587-1-git-send-email-den@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1463124421-16587-1-git-send-email-den@openvz.org> Subject: Re: [Qemu-devel] [PATCH 1/1] migration: fix ram decompression race deadlock List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: qemu-devel@nongnu.org, Maxim Nestratov , Juan Quintela , Liang Li , "Dr. David Alan Gilbert" Adding Liang Li to CC for his comments as the author of the feature. On (Fri) 13 May 2016 [10:27:01], Denis V. Lunev wrote: > From: Maxim Nestratov > > There is a race in between do_data_decompress and start_decompression. > > do_data_decompress() > while (!quit_decomp_thread) { > qemu_mutex_lock(¶m->mutex); > while (!param->start && !quit_decomp_thread) { > qemu_cond_wait(¶m->cond, ¶m->mutex); > ... > param->start = false; > } > qemu_mutex_unlock(¶m->mutex); > [ preempted here, start_decompression() is executed ] > } > > start_decompression() > { > qemu_mutex_lock(¶m->mutex); > param->start = true; > qemu_cond_signal(¶m->cond); > qemu_mutex_unlock(¶m->mutex); > } > > In this case do_data_decompress will never enter inner loop again and > will eat 100% CPU. 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 > Signed-off-by: Denis V. Lunev > CC: Juan Quintela > CC: Amit Shah > --- > 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(¶m->mutex); > while (!param->start && !quit_decomp_thread) { > qemu_cond_wait(¶m->cond, ¶m->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(¶m->mutex); > } > > -- > 2.1.4 > Amit