Re: [PATCH v2 3/4] powerpc/pseries: break early in dlpar_memory_remove_by_count() loops

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2922 bytes --]

On Wed, May 12, 2021 at 05:28:08PM -0300, Daniel Henrique Barboza wrote:
> After marking the LMBs as reserved depending on dlpar_remove_lmb() rc,
> we evaluate whether we need to add the LMBs back or if we can release
> the LMB DRCs. In both cases, a for_each_drmem_lmb() loop without a break
> condition is used. This means that we're going to cycle through all LMBs
> of the partition even after we're done with what we were going to do.
> 
> This patch adds break conditions in both loops to avoid this. The
> 'lmbs_removed' variable was renamed to 'lmbs_reserved', and it's now
> being decremented each time a lmb reservation is removed, indicating
> that the operation we're doing (adding back LMBs or releasing DRCs) is
> completed.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

The fact that DRCONF_MEM_RESERVED and DRMEM_LMB_RESERVED look so
similar but have totally different meanings doesn't make this easy to
follow :/.

> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c21d9278c1ce..3c7ce5361ce3 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -402,7 +402,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
>  static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
>  {
>  	struct drmem_lmb *lmb;
> -	int lmbs_removed = 0;
> +	int lmbs_reserved = 0;
>  	int lmbs_available = 0;
>  	int rc;
>  
> @@ -436,12 +436,12 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
>  		 */
>  		drmem_mark_lmb_reserved(lmb);
>  
> -		lmbs_removed++;
> -		if (lmbs_removed == lmbs_to_remove)
> +		lmbs_reserved++;
> +		if (lmbs_reserved == lmbs_to_remove)
>  			break;
>  	}
>  
> -	if (lmbs_removed != lmbs_to_remove) {
> +	if (lmbs_reserved != lmbs_to_remove) {
>  		pr_err("Memory hot-remove failed, adding LMB's back\n");
>  
>  		for_each_drmem_lmb(lmb) {
> @@ -454,6 +454,10 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
>  				       lmb->drc_index);
>  
>  			drmem_remove_lmb_reservation(lmb);
> +
> +			lmbs_reserved--;
> +			if (lmbs_reserved == 0)
> +				break;
>  		}
>  
>  		rc = -EINVAL;
> @@ -467,6 +471,10 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
>  				lmb->base_addr);
>  
>  			drmem_remove_lmb_reservation(lmb);
> +
> +			lmbs_reserved--;
> +			if (lmbs_reserved == 0)
> +				break;
>  		}
>  		rc = 0;
>  	}

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]