Re: [PATCH 2/3] powerpc/pseries/memhp: Remove unbalanced dlpar_release_drc() call

[Nick Child <nnac123@linux.ibm.com>]

Hi Nathan,
Patches 1 and 3 LGTM

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.


On 11/14/23 11:01, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch <nathanl@linux.ibm.com>
> 
> 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 <nathanl@linux.ibm.com>
> ---
>   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;
>   	}
>   
>