[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/5] spapr: rename spapr_drc_detach() to spapr_drc_unplug_
From: |
David Gibson |
Subject: |
Re: [PATCH v4 2/5] spapr: rename spapr_drc_detach() to spapr_drc_unplug_request() |
Date: |
Tue, 23 Feb 2021 11:13:01 +1100 |
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.
--
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
signature.asc
Description: PGP signature
- [PATCH v4 0/5] CPU unplug timeout/LMB unplug cleanup in DRC reconfiguration, Daniel Henrique Barboza, 2021/02/22
- [PATCH v4 1/5] spapr_drc.c: use spapr_drc_release() in isolate_physical/set_unusable, Daniel Henrique Barboza, 2021/02/22
- [PATCH v4 2/5] spapr: rename spapr_drc_detach() to spapr_drc_unplug_request(), Daniel Henrique Barboza, 2021/02/22
- Re: [PATCH v4 2/5] spapr: rename spapr_drc_detach() to spapr_drc_unplug_request(),
David Gibson <=
- [PATCH v4 3/5] spapr_drc.c: introduce unplug_timeout_timer, Daniel Henrique Barboza, 2021/02/22
- [PATCH v4 4/5] spapr_drc.c: add hotunplug timeout for CPUs, Daniel Henrique Barboza, 2021/02/22
- [PATCH v4 5/5] spapr_drc.c: use DRC reconfiguration to cleanup DIMM unplug state, Daniel Henrique Barboza, 2021/02/22
- Re: [PATCH v4 0/5] CPU unplug timeout/LMB unplug cleanup in DRC reconfiguration, David Gibson, 2021/02/22