Re: [RFC PATCH] powerpc: Disabling MEMORY_HOTPLUG_DEFAULT_ONLINE option for PPC64 arch

[Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>]

On 08/01/2017 11:05 AM, Nathan Fontenot wrote:
> On 08/01/2017 04:59 AM, Michael Ellerman wrote:
>> Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> writes:
>>
>>> Commit 943db62c316c ("powerpc/pseries: Revert 'Auto-online
>>> hotplugged memory'") reverted the auto-online feature for pseries due
>>> to problems with LMB removals not updating the device struct properly.
>>> Among other things, this commit made the following change in
>>> arch/powerpc/configs/pseries_defconfig:
>>>
>>> @@ -58,7 +58,6 @@ CONFIG_KEXEC_FILE=y
>>>   CONFIG_IRQ_ALL_CPUS=y
>>>   CONFIG_MEMORY_HOTPLUG=y
>>>   CONFIG_MEMORY_HOTREMOVE=y
>>> -CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE=y
>>>   CONFIG_KSM=y
>>>
>>> The intent was to disable the option in the defconfig of pseries, since
>>> after that the code doesn't have this support anymore.
>> It's always polite to Cc the author of a commit you're referring to, so
>> I added Nathan.
Noted. Thanks for adding Nathan in the CC.

>>
>> The intention when we merged that fix was that the auto-online code
>> would be "fixed" to mark the device online. I say "fixed" because it
>> wasn't entirely clear if that was the correct behaviour, though it
>> definitely seemed like it should be.
>>
>> I've lost track of where/if the discussion got to on whether the
>> auto-online code should do that or not. Did anything get resolved?
> I think, though I should go back and test to be sure, that everything
> works in the latest mainline code. The issue causing this to be a problem
> was in the original implementation of auto_online support. If you wanted
> to auto online memory, the code was calling memory_block_change_state().
> This worked but did not update the device struct for each of the memory
> block that was online'ed such that dev->offline == true even after the
> memory was online.
>
> I sent a patch earlier this year (commit dc18d706a436) that corrected
> this to call device_online() instead of memory_block_change_state().
> With this fix (appears to have gone into the 4.11 kernel) it should be
> possible to use auto_online on power systems.

Commit dc18d706a436 was present in the 4.11 kernels that experiences this
issue (Fedora 26 and Ubuntu 17.10 in my tests). So I am not entirely sure
that we can use auto_online on power systems, at least in those kernels.


>
> At this point I don't think we need this patch to disable auto online
> for ppc64. I would be curious if this is still broken with the latest
> mainline code though.

If the auto_online feature is already working in upstream 4.13 kernel 
then I don't see
a reason to apply this patch either. We can leave it as a FYI/reminder 
of a problem
that was happening in 4.11 and got solved later on.


Thanks,


Daniel

>
> -Nathan
>    
>>> However, this change
>>> alone isn't enough to prevent situations such as [1], where
>>> distros can enable the option unaware of the consequences of
>>> doing it (e.g. breaking LMB hotplug altogether).
>>>
>>> Instead of relying on all distros knowing that pseries can't handle
>>> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE=y after 943db62c316c, this patch
>>> changes mm/Kconfig to make the MEMORY_HOTPLUG_DEFAULT_ONLINE config
>>> unavailable for the PPC64 arch.
>>>
>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1476380
>>>
>>> Fixes: 943db62c316c ("powerpc/pseries: Revert 'Auto-online hotplugged memory'")
>>> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
>>> ---
>>>   mm/Kconfig | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> I don't own that file, so we at least need an Ack from the mm folks.
>>
>> cheers
>>
>>> diff --git a/mm/Kconfig b/mm/Kconfig
>>> index 48b1af4..a342c77 100644
>>> --- a/mm/Kconfig
>>> +++ b/mm/Kconfig
>>> @@ -169,7 +169,7 @@ config MEMORY_HOTPLUG_SPARSE
>>>   config MEMORY_HOTPLUG_DEFAULT_ONLINE
>>>           bool "Online the newly added memory blocks by default"
>>>           default n
>>> -        depends on MEMORY_HOTPLUG
>>> +        depends on MEMORY_HOTPLUG && !PPC64
>>>           help
>>>   	  This option sets the default policy setting for memory hotplug
>>>   	  onlining policy (/sys/devices/system/memory/auto_online_blocks) which
>>> -- 
>>> 2.9.4