Re: [PATCH 1/5] spapr.c: assert first DRC LMB earlier in spapr_memory_unplug_request()

[Greg Kurz]

On Fri, 26 Feb 2021 13:32:57 -0300
Daniel Henrique Barboza <danielhb413@gmail.com> wrote:

> We are asserting the existence of the first DRC LMB after sending unplug
> requests to all LMBs of the DIMM, where every DRC is being asserted
> inside the loop. This means that the first DRC is being asserted twice.
> 
> We will use the first DRC to simplify the code a bit in the next patch,
> so instead of removing the duplicated assert, let's do it earlier.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6eaddb12cb..74e046b522 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3664,7 +3664,7 @@ static void spapr_memory_unplug_request(HotplugHandler 
> *hotplug_dev,
>      uint32_t nr_lmbs;
>      uint64_t size, addr_start, addr;
>      int i;
> -    SpaprDrc *drc;
> +    SpaprDrc *drc, *drc_start;
>  
>      if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
>          error_setg(errp, "nvdimm device hot unplug is not supported yet.");
> @@ -3677,6 +3677,10 @@ static void spapr_memory_unplug_request(HotplugHandler 
> *hotplug_dev,
>      addr_start = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
>                                            &error_abort);
>  
> +    drc_start = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
> +                                addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> +    g_assert(drc_start);
> +
>      /*
>       * An existing pending dimm state for this DIMM means that there is an
>       * unplug operation in progress, waiting for the spapr_lmb_release
> @@ -3701,11 +3705,9 @@ static void spapr_memory_unplug_request(HotplugHandler 
> *hotplug_dev,
>          addr += SPAPR_MEMORY_BLOCK_SIZE;
>      }
>  
> -    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
> -                          addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> -    g_assert(drc);
>      spapr_hotplug_req_remove_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
> -                                              nr_lmbs, spapr_drc_index(drc));
> +                                              nr_lmbs,
> +                                              spapr_drc_index(drc_start));
>  }
>  
>  /* Callback to be called during DRC release. */