Re: [Qemu-devel] [RFC PATCH v3 17/24] cpus: Reclaim vCPU objects

[Bharata B Rao]

On Tue, May 05, 2015 at 05:20:04PM +1000, David Gibson wrote:
> On Fri, Apr 24, 2015 at 12:17:39PM +0530, Bharata B Rao wrote:
> > From: Gu Zheng <address@hidden>
> > 
> > In order to deal well with the kvm vcpus (which can not be removed without 
> > any
> > protection), we do not close KVM vcpu fd, just record and mark it as stopped
> > into a list, so that we can reuse it for the appending cpu hot-add request 
> > if
> > possible. It is also the approach that kvm guys suggested:
> > https://www.mail-archive.com/address@hidden/msg102839.html
> > 
> > This patch also adds a QOM API object_has_no_children(Object *obj)
> > that checks whether a given object has any child objects. This API
> > is needed to release CPU core and socket objects when a vCPU is destroyed.
> 
> I'm guessing this commit message needs updating, since you seem to
> have split this out into the previous patch.
> 
> > Signed-off-by: Chen Fan <address@hidden>
> > Signed-off-by: Gu Zheng <address@hidden>
> > Signed-off-by: Zhu Guihua <address@hidden>
> > Signed-off-by: Bharata B Rao <address@hidden>
> >                [Added core and socket removal bits]
> > ---
> >  cpus.c               | 67 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/qom/cpu.h    | 11 +++++++++
> >  include/sysemu/kvm.h |  1 +
> >  kvm-all.c            | 57 +++++++++++++++++++++++++++++++++++++++++++-
> >  kvm-stub.c           |  5 ++++
> >  5 files changed, 140 insertions(+), 1 deletion(-)
> > 
> > diff --git a/cpus.c b/cpus.c
> > index 0fac143..325f8a6 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -858,6 +858,47 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void 
> > *data), void *data)
> >      qemu_cpu_kick(cpu);
> >  }
> >  
> > +static void qemu_destroy_cpu_core(Object *core)
> > +{
> > +    Object *socket = core->parent;
> > +
> > +    object_unparent(core);
> > +    if (socket && object_has_no_children(socket)) {
> > +        object_unparent(socket);
> > +    }
> 
> This seems a bit odd to me.  I thought the general idea of the new
> approaches to cpu hotplug meant that the hotplug sequence started from
> the top (the socket or core) and worked down to the threads.  Rather
> than starting at the thread, and working up to the core and socket
> level.

Yes that's true for hotplug as well as hot unplug curently. Plug or
unplug starts at socket, moves down to cores and threads.

However when the unplug request comes down to the thread, we have to
destroy the vCPU and that's when we end up in this part of the code. Here
the thread (vCPU) unparents itself from the core. The core can't unparent
untill all its threads have unparented themselves. When all threads of a
core are done unparenting, core goes ahead and unparents itself from
its parent socket. Similarly socket can unparent when all cores under
it have unparented themselves from the socket.

This is the code that ensures that the socket device object finally
gets cleared and the id associated with the hot removed socket device
is available for reuse with next hotplug.

> 
> > +}
> > +
> > +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
> > +{
> > +    Object *thread = OBJECT(cpu);
> > +    Object *core = thread->parent;
> > +
> > +    CPU_REMOVE(cpu);
> > +
> > +    if (kvm_destroy_vcpu(cpu) < 0) {
> > +        error_report("kvm_destroy_vcpu failed.\n");
> > +        exit(EXIT_FAILURE);
> > +    }
> > +
> > +    object_unparent(thread);
> > +    if (core && object_has_no_children(core)) {
> > +        qemu_destroy_cpu_core(core);
> > +    }
> > +}
> > +

Regards,
Bharata.