Re: [Qemu-devel] [PATCH v2 2/9] migration: Fix a potential issue

From: "Li, Liang Z" <liang.z.li@intel.com>
To: Amit Shah <amit.shah@redhat.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"quintela@redhat.com" <quintela@redhat.com>,
	"dgilbert@redhat.com" <dgilbert@redhat.com>,
	"berrange@redhat.com" <berrange@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/9] migration: Fix a potential issue
Date: Mon, 13 Jun 2016 05:07:39 +0000	[thread overview]
Message-ID: <F2CBF3009FA73547804AE4C663CAB28E041EBACC@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20160613043622.GD6398@grmbl.mre>

> > > > +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().

Liang