On 10/16/2018 02:57 PM, Tyrel Datwyler wrote: > On 10/15/2018 05:39 PM, Michael Ellerman wrote: >> Michael Bringmann <m...@linux.vnet.ibm.com> writes: >>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c >>> b/arch/powerpc/platforms/pseries/hotplug-memory.c >>> index 2b796da..9c76345 100644 >>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c >>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c >>> @@ -541,6 +549,23 @@ static int dlpar_memory_readd_by_index(u32 drc_index) >>> return rc; >>> } >>> >>> +static int dlpar_memory_readd_multiple(void) >>> +{ >>> + struct drmem_lmb *lmb; >>> + int rc; >>> + >>> + pr_info("Attempting to update multiple LMBs\n"); >>> + >>> + for_each_drmem_lmb(lmb) { >>> + if (drmem_lmb_update(lmb)) { >>> + rc = dlpar_memory_readd_helper(lmb); >>> + drmem_remove_lmb_update(lmb); >>> + } >>> + } >>> + >>> + return rc; >>> +} >> >> This leaves rc potentially uninitialised. >> >> What should the result be in that case, -EINVAL ? > > On another note if there are multiple LMBs to update the value of rc only > reflects the final dlpar_memory_readd_helper() call.
Correct. But that is what happens when we compress common code between two disparate uses i.e. updating memory association after a migration event with no reporting mechanism other than the console log, vs re-adding a single LMB by index for the purposes of DLPAR / drmgr. I could discard the return value from dlpar_memory_readd_helper entirely in this function and just return 0, but in my experience, once errors start to occur in memory dlpar ops, they tend to keep on occurring, so I was returning the last one. We could also make the code smart enough to capture and return the first/last non-zero return code. I didn't believe that the frequency of errors for this operation warranted the overhead. > > -Tyrel Michael -- Michael W. Bringmann Linux Technology Center IBM Corporation Tie-Line 363-5196 External: (512) 286-5196 Cell: (512) 466-0650 m...@linux.vnet.ibm.com