Re: [Qemu-devel] [RFC PATCH v2.1 12/12] spapr: CPU hot unplug support

[Bharata B Rao <bharata@linux.vnet.ibm.com>]

On Mon, Apr 04, 2016 at 02:27:24PM +1000, David Gibson wrote:
> On Thu, Mar 31, 2016 at 02:09:21PM +0530, Bharata B Rao wrote:
> > Remove the CPU core device by removing the underlying CPU thread devices.
> > Hot removal of CPU for sPAPR guests is achieved by sending the hot unplug
> > notification to the guest. Release the vCPU object after CPU hot unplug so
> > that vCPU fd can be parked and reused.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr.c                  | 16 ++++++++
> >  hw/ppc/spapr_cpu_core.c         | 86 +++++++++++++++++++++++++++++++++++++++++
> >  include/hw/ppc/spapr.h          |  1 +
> >  include/hw/ppc/spapr_cpu_core.h | 11 ++++++
> >  4 files changed, 114 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 1a5dbd9..74cdcf2 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2348,11 +2348,27 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >      }
> >  }
> >  
> > +void spapr_cpu_destroy(PowerPCCPU *cpu)
> > +{
> > +    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> > +
> > +    xics_cpu_destroy(spapr->icp, cpu);
> > +    qemu_unregister_reset(spapr_cpu_reset, cpu);
> > +}
> > +
> >  static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
> >                                        DeviceState *dev, Error **errp)
> >  {
> > +    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
> > +
> >      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >          error_setg(errp, "Memory hot unplug not supported by sPAPR");
> > +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> > +        if (!smc->dr_cpu_enabled) {
> > +            error_setg(errp, "CPU hot unplug not supported on this machine");
> > +            return;
> > +        }
> > +        spapr_core_unplug(hotplug_dev, dev, errp);
> >      }
> >  }
> >  
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index a9ba843..09a592e 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -119,6 +119,92 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> >      }
> >  }
> >  
> > +static void spapr_cpu_core_cleanup(struct sPAPRCPUUnplugList *unplug_list)
> > +{
> > +    sPAPRCPUUnplug *unplug, *next;
> > +    Object *cpu;
> > +
> > +    QLIST_FOREACH_SAFE(unplug, unplug_list, node, next) {
> > +        cpu = unplug->cpu;
> > +        object_unparent(cpu);
> 
> Is there any danger in the fact that the cpu object is still in the
> QOM tree until unparented here?  My usual expectation would be that
> you'd remove the object from the tree immediately, but defer the
> actual free.  But I'm a bit unclear on how QOM removals are supposed
> to work.

As per my understanding, object_unparent() removes the object from
its parent and finalizes the child too.

The reason I defer unparenting of each CPU thread object like this is because
from the parent core object's detach_cb routine (spapr_core_release), we are
still walking the parent core's child list and can't immediately unparent the
child thread objects.

Regards,
Bharata.