On Mon, 19 Oct 2020 10:49:01 +0200
Greg Kurz <gr...@kaod.org> wrote:
> As recommended in "qapi/error.h", add a bool return value to
> spapr_add_lmbs() and spapr_add_nvdimm(), and use them instead
> of local_err in spapr_memory_plug().
>
> This allows to get rid of the error propagation overhead.
>
> Signed-off-by: Greg Kurz <gr...@kaod.org>
you won't need this patch if check from spapr_drc_attach() were
moved to _pre_plug() time.
> ---
> hw/ppc/spapr.c | 23 ++++++++++-------------
> hw/ppc/spapr_nvdimm.c | 5 +++--
> include/hw/ppc/spapr_nvdimm.h | 2 +-
> 3 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 62f217a6b914..0cc19b5863a4 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3382,7 +3382,7 @@ int spapr_lmb_dt_populate(SpaprDrc *drc,
> SpaprMachineState *spapr,
> return 0;
> }
>
> -static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t
> size,
> +static bool spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t
> size,
> bool dedicated_hp_event_source, Error **errp)
> {
> SpaprDrc *drc;
> @@ -3403,7 +3403,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
> addr_start, uint64_t size,
> addr / SPAPR_MEMORY_BLOCK_SIZE);
> spapr_drc_detach(drc);
> }
> - return;
> + return false;
> }
> if (!hotplugged) {
> spapr_drc_reset(drc);
> @@ -3425,12 +3425,12 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
> addr_start, uint64_t size,
> nr_lmbs);
> }
> }
> + return true;
> }
>
> static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp)
> {
> - Error *local_err = NULL;
> SpaprMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> PCDIMMDevice *dimm = PC_DIMM(dev);
> uint64_t size, addr;
> @@ -3444,27 +3444,24 @@ static void spapr_memory_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> if (!is_nvdimm) {
> addr = object_property_get_uint(OBJECT(dimm),
> PC_DIMM_ADDR_PROP, &error_abort);
> - spapr_add_lmbs(dev, addr, size,
> - spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
> - &local_err);
> + if (!spapr_add_lmbs(dev, addr, size,
> + spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT), errp))
> {
> + goto out_unplug;
> + }
> } else {
> slot = object_property_get_int(OBJECT(dimm),
> PC_DIMM_SLOT_PROP, &error_abort);
> /* We should have valid slot number at this point */
> g_assert(slot >= 0);
> - spapr_add_nvdimm(dev, slot, &local_err);
> - }
> -
> - if (local_err) {
> - goto out_unplug;
> + if (!spapr_add_nvdimm(dev, slot, errp)) {
> + goto out_unplug;
> + }
> }
>
> return;
>
> out_unplug:
> pc_dimm_unplug(dimm, MACHINE(ms));
> -out:
> - error_propagate(errp, local_err);
> }
>
> static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
> *dev,
> diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
> index 9e3d94071fe1..a833a63b5ed3 100644
> --- a/hw/ppc/spapr_nvdimm.c
> +++ b/hw/ppc/spapr_nvdimm.c
> @@ -89,7 +89,7 @@ bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev,
> NVDIMMDevice *nvdimm,
> }
>
>
> -void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp)
> +bool spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp)
> {
> SpaprDrc *drc;
> bool hotplugged = spapr_drc_hotplugged(dev);
> @@ -98,12 +98,13 @@ void spapr_add_nvdimm(DeviceState *dev, uint64_t slot,
> Error **errp)
> g_assert(drc);
>
> if (!spapr_drc_attach(drc, dev, errp)) {
> - return;
> + return false;
> }
>
> if (hotplugged) {
> spapr_hotplug_req_add_by_index(drc);
> }
> + return true;
> }
>
> static int spapr_dt_nvdimm(SpaprMachineState *spapr, void *fdt,
> diff --git a/include/hw/ppc/spapr_nvdimm.h b/include/hw/ppc/spapr_nvdimm.h
> index 490b19a009f4..344582d2f5f7 100644
> --- a/include/hw/ppc/spapr_nvdimm.h
> +++ b/include/hw/ppc/spapr_nvdimm.h
> @@ -30,6 +30,6 @@ int spapr_pmem_dt_populate(SpaprDrc *drc, SpaprMachineState
> *spapr,
> void spapr_dt_persistent_memory(SpaprMachineState *spapr, void *fdt);
> bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
> uint64_t size, Error **errp);
> -void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp);
> +bool spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp);
>
> #endif
>
>
