Re: [PATCH 3/3] powerpc/pseries/cpuhp: delete add/remove_by_count code

[Nathan Lynch <nathanl@linux.ibm.com>]

Daniel Henrique Barboza <danielhb413@gmail.com> writes:

> On 9/20/21 10:55, Nathan Lynch wrote:
>> The core DLPAR code supports two actions (add and remove) and three
>> subtypes of action:
>> 
>> * By DRC index: the action is attempted on a single specified resource.
>>    This is the usual case for processors.
>> * By indexed count: the action is attempted on a range of resources
>>    beginning at the specified index. This is implemented only by the memory
>>    DLPAR code.
>> * By count: the lower layer (CPU or memory) is responsible for locating the
>>    specified number of resources to which the action can be applied.
>> 
>> I cannot find any evidence of the "by count" subtype being used by drmgr or
>> qemu for processors. And when I try to exercise this code, the add case
>> does not work:
>
>
> Just to clarify: did you check both CPU and memory cases and found out that the
> 'by count' subtype isn't used with CPUs, but drmgr has some cases in which
> 'by count' is used with LMBs?

Yes, drmgr uses both the 'by count' and the 'by index' methods for
memory currently on PowerVM.

> I'm asking because I worked with a part of the LMB removal code a few months ago,
> and got stuck in a situation in which the 'by count' and 'by indexed count' are
> similar enough to feel repetitive, but distinct enough to not be easily reduced
> into a single function. If drmgr wasn't using the 'by count' subtypes for LMBs
> that would be a good chance for more code redux.

The 'by count' method is definitely used for memory on PowerVM. I was
under the impression that the 'by indexed count' method was used by qemu
for memory sometimes; I'm pretty sure it's not used on PowerVM.

>> Summary:
>> 
>> * This code has not worked reliably since its introduction.
>> * There is no evidence that it is used.
>> * It contains questionable rollback behaviors in error paths which are
>>    difficult to test.
>> 
>> So let's remove it.
>> 
>> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
>> Fixes: ac71380071d1 ("powerpc/pseries: Add CPU dlpar remove functionality")
>> Fixes: 90edf184b9b7 ("powerpc/pseries: Add CPU dlpar add functionality")
>> Fixes: b015f6bc9547 ("powerpc/pseries: Add cpu DLPAR support for drc-info property")
>> ---
>
> Tested with a QEMU pseries guest, no issues found.
>
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Thanks!