Message ID | 20210222194531.62717-3-danielhb413@gmail.com |
---|---|
State | New |
Headers | show |
Series | CPU unplug timeout/LMB unplug cleanup in DRC reconfiguration | expand |
On Mon, Feb 22, 2021 at 04:45:28PM -0300, Daniel Henrique Barboza wrote: > spapr_drc_detach() is not the best name for what the function does. The > function does not detach the DRC, it makes an uncommited attempt to do > it. It'll mark the DRC as pending unplug, via the 'unplug_request' > flag, and only if the DRC state is drck->empty_state it will detach the > DRC, via spapr_drc_release(). > > This is a contrast with its pair spapr_drc_attach(), where the function > is indeed creating the DRC QOM object. If you know what > spapr_drc_attach() does, you can be misled into thinking that > spapr_drc_detach() is removing the DRC from QEMU internal state, which > isn't true. > > The current role of this function is better described as a request for > detach, since there's no guarantee that we're going to detach the DRC in > the end. Rename the function to spapr_drc_unplug_request to reflect > what is is doing. > > The initial idea was to change the name to spapr_drc_detach_request(), > and later on change the unplug_request flag to detach_request. However, > unplug_request is a migratable boolean for a long time now and renaming > it is not worth the trouble. spapr_drc_unplug_request() setting > drc->unplug_request is more natural than spapr_drc_detach_request > setting drc->unplug_request. > > Reviewed-by: Greg Kurz <groug@kaod.org> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Applied to ppc-for-6.0, thanks. > --- > hw/ppc/spapr.c | 6 +++--- > hw/ppc/spapr_drc.c | 4 ++-- > hw/ppc/spapr_pci.c | 4 ++-- > hw/ppc/trace-events | 2 +- > include/hw/ppc/spapr_drc.h | 2 +- > 5 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 85fe65f894..b066df68cb 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3654,7 +3654,7 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, > addr / SPAPR_MEMORY_BLOCK_SIZE); > g_assert(drc); > > - spapr_drc_detach(drc); > + spapr_drc_unplug_request(drc); > addr += SPAPR_MEMORY_BLOCK_SIZE; > } > > @@ -3722,7 +3722,7 @@ void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, > g_assert(drc); > > if (!spapr_drc_unplug_requested(drc)) { > - spapr_drc_detach(drc); > + spapr_drc_unplug_request(drc); > spapr_hotplug_req_remove_by_index(drc); > } > } > @@ -3985,7 +3985,7 @@ static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev, > assert(drc); > > if (!spapr_drc_unplug_requested(drc)) { > - spapr_drc_detach(drc); > + spapr_drc_unplug_request(drc); > spapr_hotplug_req_remove_by_index(drc); > } > } > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > index 555a25517d..67041fb212 100644 > --- a/hw/ppc/spapr_drc.c > +++ b/hw/ppc/spapr_drc.c > @@ -386,11 +386,11 @@ void spapr_drc_attach(SpaprDrc *drc, DeviceState *d) > NULL, 0); > } > > -void spapr_drc_detach(SpaprDrc *drc) > +void spapr_drc_unplug_request(SpaprDrc *drc) > { > SpaprDrcClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); > > - trace_spapr_drc_detach(spapr_drc_index(drc)); > + trace_spapr_drc_unplug_request(spapr_drc_index(drc)); > > g_assert(drc->dev); > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index f1c7479816..b00e9609ae 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -1723,12 +1723,12 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler, > * functions, even if their unplug weren't requested > * beforehand. > */ > - spapr_drc_detach(func_drc); > + spapr_drc_unplug_request(func_drc); > } > } > } > > - spapr_drc_detach(drc); > + spapr_drc_unplug_request(drc); > > /* if this isn't func 0, defer unplug event. otherwise signal removal > * for all present functions > diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events > index 1e91984526..b4bbfbb013 100644 > --- a/hw/ppc/trace-events > +++ b/hw/ppc/trace-events > @@ -50,7 +50,7 @@ spapr_drc_set_allocation_state(uint32_t index, int state) "drc: 0x%"PRIx32", sta > spapr_drc_set_allocation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32 > spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32 > spapr_drc_attach(uint32_t index) "drc: 0x%"PRIx32 > -spapr_drc_detach(uint32_t index) "drc: 0x%"PRIx32 > +spapr_drc_unplug_request(uint32_t index) "drc: 0x%"PRIx32 > spapr_drc_awaiting_quiesce(uint32_t index) "drc: 0x%"PRIx32 > spapr_drc_reset(uint32_t index) "drc: 0x%"PRIx32 > spapr_drc_realize(uint32_t index) "drc: 0x%"PRIx32 > diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h > index 8982927d5c..02a63b3666 100644 > --- a/include/hw/ppc/spapr_drc.h > +++ b/include/hw/ppc/spapr_drc.h > @@ -243,7 +243,7 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask); > * beforehand (eg. check drc->dev at pre-plug). > */ > void spapr_drc_attach(SpaprDrc *drc, DeviceState *d); > -void spapr_drc_detach(SpaprDrc *drc); > +void spapr_drc_unplug_request(SpaprDrc *drc); > > /* > * Reset all DRCs, causing pending hot-plug/unplug requests to complete.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 85fe65f894..b066df68cb 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3654,7 +3654,7 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, addr / SPAPR_MEMORY_BLOCK_SIZE); g_assert(drc); - spapr_drc_detach(drc); + spapr_drc_unplug_request(drc); addr += SPAPR_MEMORY_BLOCK_SIZE; } @@ -3722,7 +3722,7 @@ void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, g_assert(drc); if (!spapr_drc_unplug_requested(drc)) { - spapr_drc_detach(drc); + spapr_drc_unplug_request(drc); spapr_hotplug_req_remove_by_index(drc); } } @@ -3985,7 +3985,7 @@ static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev, assert(drc); if (!spapr_drc_unplug_requested(drc)) { - spapr_drc_detach(drc); + spapr_drc_unplug_request(drc); spapr_hotplug_req_remove_by_index(drc); } } diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 555a25517d..67041fb212 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -386,11 +386,11 @@ void spapr_drc_attach(SpaprDrc *drc, DeviceState *d) NULL, 0); } -void spapr_drc_detach(SpaprDrc *drc) +void spapr_drc_unplug_request(SpaprDrc *drc) { SpaprDrcClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - trace_spapr_drc_detach(spapr_drc_index(drc)); + trace_spapr_drc_unplug_request(spapr_drc_index(drc)); g_assert(drc->dev); diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index f1c7479816..b00e9609ae 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1723,12 +1723,12 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler, * functions, even if their unplug weren't requested * beforehand. */ - spapr_drc_detach(func_drc); + spapr_drc_unplug_request(func_drc); } } } - spapr_drc_detach(drc); + spapr_drc_unplug_request(drc); /* if this isn't func 0, defer unplug event. otherwise signal removal * for all present functions diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events index 1e91984526..b4bbfbb013 100644 --- a/hw/ppc/trace-events +++ b/hw/ppc/trace-events @@ -50,7 +50,7 @@ spapr_drc_set_allocation_state(uint32_t index, int state) "drc: 0x%"PRIx32", sta spapr_drc_set_allocation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_attach(uint32_t index) "drc: 0x%"PRIx32 -spapr_drc_detach(uint32_t index) "drc: 0x%"PRIx32 +spapr_drc_unplug_request(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_awaiting_quiesce(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_reset(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_realize(uint32_t index) "drc: 0x%"PRIx32 diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 8982927d5c..02a63b3666 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -243,7 +243,7 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask); * beforehand (eg. check drc->dev at pre-plug). */ void spapr_drc_attach(SpaprDrc *drc, DeviceState *d); -void spapr_drc_detach(SpaprDrc *drc); +void spapr_drc_unplug_request(SpaprDrc *drc); /* * Reset all DRCs, causing pending hot-plug/unplug requests to complete.