Re: [PATCH] powerpc/pseries: Auto online hotplugged memory

[Nathan Fontenot <nfont@linux.vnet.ibm.com>]

On 06/24/2016 12:35 AM, Michael Ellerman wrote:
> On Mon, 2016-06-20 at 21:14 -0500, Nathan Fontenot wrote:
>> On 06/20/2016 07:57 PM, Michael Ellerman wrote:
>>> On Mon, 2016-06-20 at 08:51 -0500, Nathan Fontenot wrote:
>>>
>>>> Auto online hotplugged memory
>>>>
>>>> A recent update (commit id 31bc3858ea3) to the core mm hotplug code
>>>> introduced the memhp_auto_online variable to allow for automatically
>>>> onlining memory that is added.
>>>>
>>>> This patch update the pseries memory hotplug code to enable this so that
>>>> any memory DLPAR added to the system is automatically onlined. The code
>>>> to add the memory block for memory added from add_memory() is removed as
>>>> this is not needed, the memory_add code does this.
>>>
>>> Is this a bug fix, or just a cleanup?
>>
>> Hmmm.. some cleanup and some new feature. The removal of the memblock_add()
>> call is a cleanup and the setting of the memhp_auto_online variable is
>> taking advantage of a feature I was not previously aware of.
> 
> OK. Looking at usage of memhp_auto_online it's not clear to me that you're
> supposed to be setting it in arch code.
> 
> eg. if I build my kernel with CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE=n, I will
> expect it to not be onlined by default.
> 
> Similarly if I boot with memhp_default_state=offline on the kernel command line.
> 
> But this patch would then mean it is onlined by default. So that seems kind of
> confusing for users.

Yes, but we already online memory when it is DLPAR added to the system. This has
always been the default behavior for pseries. I was using the memhp_auto_online
setting so that the memory will be added as part of the memory_add call instead
of having to online the memory as a separate step in the pseries code.

In other words, I am not changing the existing behavior of pseries code. 

Perhaps something a bit different? I could save and restore the memhp_auto_online
setting across the call to memory_add, or perhpas ther should be an
add_memory_and_online() variant of the add_memory() call that always onlines memory.

> 
> I think instead we should be merging the bulk of this patch, but without the
> forced assignment to memhp_auto_online?

The bulk of the patch revolves around the setting of the memhp_auto_online setting
and the bits of code we can remove by using this. The other part is removing
the call to memblock_add() which I can send another patch to do once we have
resolved the memhp_auto_online questions.

-Nathan