On Wed, May 06, 2015 at 12:07:57PM +0530, Bharata B Rao wrote: > 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 <guz.f...@cn.fujitsu.com> > > > > > > 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/kvm@vger.kernel.org/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 <chen.fan.f...@cn.fujitsu.com> > > > Signed-off-by: Gu Zheng <guz.f...@cn.fujitsu.com> > > > Signed-off-by: Zhu Guihua <zhugh.f...@cn.fujitsu.com> > > > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com> > > > [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.
Why can't the core unplug routine propagte the unplug down to the
threads, let that complete, then do the per-core unplug stuff and
remove itself?
Is there an asynchronous callback in here somewhere?
> 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.
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpDZmbfxH4LU.pgp
Description: PGP signature
