diff mbox series

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

Message ID 20210512202809.95363-4-danielhb413@gmail.com (mailing list archive)
State Accepted
Headers show
Series Unisolate LMBs DRC on removal error + cleanups | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (1fab3666d738e4af3a7450c44441310e4d7a7e53)
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 43 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Daniel Henrique Barboza May 12, 2021, 8:28 p.m. UTC
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>
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

David Gibson May 13, 2021, 5:20 a.m. UTC | #1
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;
>  	}
diff mbox series

Patch

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