From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32897) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCPBH-0001PE-4P for qemu-devel@nongnu.org; Mon, 13 Jun 2016 06:33:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCPBC-00057K-Nn for qemu-devel@nongnu.org; Mon, 13 Jun 2016 06:33:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52328) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCPBC-00056v-Ic for qemu-devel@nongnu.org; Mon, 13 Jun 2016 06:33:14 -0400 Date: Mon, 13 Jun 2016 16:03:11 +0530 From: Amit Shah Message-ID: <20160613103311.GA22759@grmbl.mre> References: <1462433579-13691-1-git-send-email-liang.z.li@intel.com> <1462433579-13691-3-git-send-email-liang.z.li@intel.com> <20160610133924.GC6398@grmbl.mre> <20160613043622.GD6398@grmbl.mre> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v2 2/9] migration: Fix a potential issue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Li, Liang Z" Cc: "qemu-devel@nongnu.org" , "quintela@redhat.com" , "dgilbert@redhat.com" , "berrange@redhat.com" On (Mon) 13 Jun 2016 [05:07:39], Li, Liang Z wrote: > > > > > +static void wait_for_decompress_done(void) { > > > > > + int idx, thread_count; > > > > > + > > > > > + if (!migrate_use_compression()) { > > > > > + return; > > > > > + } > > > > > + > > > > > + thread_count = migrate_decompress_threads(); > > > > > + qemu_mutex_lock(&decomp_done_lock); > > > > > + for (idx = 0; idx < thread_count; idx++) { > > > > > + while (!decomp_param[idx].done) { > > > > > + qemu_cond_wait(&decomp_done_cond, > > &decomp_done_lock); > > > > > + } > > > > > + } > > > > > + qemu_mutex_unlock(&decomp_done_lock); > > > > > > > > Not sure how this works: in the previous patch, done is set to false > > > > under the decomp_done_lock. Here, we take the lock, and wait for done > > to turn false. > > > > That can't happen because this thread holds the lock. > > > > My reading is this is going to lead to a deadlock. What am I missing? > > > > > > > > > > This is the typical usage of the QemuCond, actually, in > > > qemu_cond_wait() , decomp_done_lock will be unlocked at first and then > > > locked again before > > > qemu_cond_wait() return. So deadlock won't happen. > > > > In qemu-thread-posix.c, I don't see such unlock/lock. > > > > > > Amit > > I mean in the 'pthread_cond_wait()' which called by qemu_cond_wait(). Yes, OK - makes sense now. Thanks, I'll continue the review. Amit