Nick Child [off-list ref] writes:

Hi Nathan,
Patches 1 and 3 LGTM

thanks.

Regarding this patch, dlpar_memory_remove_by_count() calls 
dlpar_add_lmb() and does not free drc on add error.
dlpar_add_lmb() is called here in error recovery so probably
not a big deal.

This is all new code to me but it looks like if the requested
number of lmbs cannot be removed then it attempts to add back
the ones that were successfully removed. So if you cannot add
an lmb that WAS successfully removed, it seems sane to also
release the drc.

Maybe I'll drop this one for now and turn my attention to removing all
the high-level rollback logic in this code. There's no reliable way to
accomplish what it's trying to do (i.e. restore the original quantity of
LMBs) and it just complicates things.

On 11/14/23 11:01, Nathan Lynch via B4 Relay wrote:

quoted

From: Nathan Lynch <redacted>

Callers of dlpar_add_lmb() are responsible for first acquiring the DRC
and releasing it if dlpar_add_lmb() fails.

However, dlpar_add_lmb() performs a dlpar_release_drc() in one error
branch.  There is no corresponding dlpar_acquire_drc() in the
function, nor is there any stated justification. None of the other
error paths in dlpar_add_lmb() release the DRC.

This is a potential source of redundant attempts to release DRCs,
which is likely benign, but is confusing and inconsistent. Remove it.

Signed-off-by: Nathan Lynch <redacted>
---
  arch/powerpc/platforms/pseries/hotplug-memory.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 6f2eebae7bee..ba883c1b9f6d 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c

@@ -575,7 +575,6 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
  
  	rc = update_lmb_associativity_index(lmb);
  	if (rc) {
-		dlpar_release_drc(lmb->drc_index);
  		return rc;
  	}