Re: [Qemu-devel] [PATCH v6 06/11] cpu: Add a sync version of cpu_remove()

[Bharata B Rao]

On Tue, Jan 12, 2016 at 03:16:15PM +1100, David Gibson wrote:
> On Fri, Jan 08, 2016 at 12:25:14PM +0530, Bharata B Rao wrote:
> > This sync API will be used by the CPU hotplug code to wait for the CPU to
> > completely get removed before flagging the failure to the device_add
> > command.
> > 
> > Sync version of this call is needed to correctly recover from CPU
> > realization failures when ->plug() handler fails.
> > 
> > Signed-off-by: Bharata B Rao <address@hidden>
> > Reviewed-by: David Gibson <address@hidden>
> > ---
> >  cpus.c            | 12 ++++++++++++
> >  include/qom/cpu.h |  8 ++++++++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/cpus.c b/cpus.c
> > index 12374af..c829bd6 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -1067,6 +1067,8 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
> >          qemu_kvm_wait_io_event(cpu);
> >          if (cpu->exit && !cpu_can_run(cpu)) {
> >              qemu_kvm_destroy_vcpu(cpu);
> > +            cpu->created = false;
> > +            qemu_cond_signal(&qemu_cpu_cond);
> 
> I know I reviewed this earlier, but I suspect this needs to be
> qemu_cond_broadcast.  qemu_cpu_cond is global, so there could (at
> least in theory) be multiple things waiting on it, and we don't know
> which one is waiting on *this* cpu, rather than another one (or on any
> other condition handled by this qemu_cpu_cond).

qemu_cpu_cond condition variable is currently handling the CPU creation
condition where qemu_cond_signal() is being used. From what I understand,
only main thread will be waiting on this condition.

In this patch, I am using the same condition variable to track CPU
deletion condition too. After this too, I think only main thread will be
waiting for the deletion to get completed. So using qemu_cond_signal()
should be ok isn't ?

Regards,
Bharata.