[RFC] memory_hotplug: disable the functionality for 32b (was: Re: [Bug 206401] kernel panic on Hyper-V after 5 minutes due to) memory hot-add

Commit Message

Michal Hocko Feb. 18, 2020, 10:05 a.m. UTC

On Tue 18-02-20 18:19:00, [e-mail] wrote:
> [e-mail] sed in <[e-mail]>
> 
> >> On Tue 18-02-20 15:24:48, [e-mail] wrote:
> >> [...]
> >> > Tried out the above patch.
> >> > It seems to be working; no panic, total memory has increased and
> >> > the hot-added memory is added as HIGHMEM.
> >> 
> >> I was about to post a patch to mark hotplug broken on 32b but it seems
> >> you do care about this setup. Could you describe your usecase please?
> 
> My usecase is testing out the kernel on Hyper-V before loading it on
> real i686 machine. Hyper-V machine is faster to skim out other bugs.
> So memory hot-add is not a must requirement for me,
> but having hot-add may be handy to see the application memory requirement.
> (as in the anaconda test revealed)

OK, thanks for the clarification. I am not sure that this qualifies
as a sufficient reason to maintain the code though.

> If we're disabling it, we have to announce it somewhere;
> where is appropriate? `modinfo hv_balloon`'s "hot_add" description?

This should behave the same way as when the CONFIG_MEMORY_HOTPLULG is
not enabled. And from a very cursory look hv_balloon.c already checks
for the config.

---
From 562f21abeda508f199c34358e50fbaa518cd5ed8 Mon Sep 17 00:00:00 2001
From: Michal Hocko <[e-mail]>
Date: Tue, 18 Feb 2020 08:04:13 +0100
Subject: [PATCH] memory_hotplug: disable the functionality for 32b

Memory hotlug is broken for 32b systems at least since c6f03e2903c9
("mm, memory_hotplug: remove zone restrictions") which has considerably
reworked how can be memory associated with movable/kernel zones. The
same is not really trivial to achieve in 32b where only lowmem is the
kernel zone. While we can tweak this immediate problem around there are
likely other land mines hidden at other places.

It is also quite dubious that there is a real usecase for the memory
hotplug on 32b in the first place. Low memory is just too small to be
hotplugable (for hot add) and generally unusable for hotremove. Adding
more memory to highmem is also dubious because it would increase the
low mem or vmalloc space pressure for memmaps.

Restrict the functionality to 64b systems. This will help future
development to focus on usecases that have real life application.  We
can remove this restriction in future in presence of a real life usecase
of course but until then make it explicit that hotplug on 32b is broken
and requires a non trivial amount of work to fix.

Signed-off-by: Michal Hocko <[e-mail]>
---
 mm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

David Hildenbrand Feb. 18, 2020, 10:11 a.m. UTC | #1

On 18.02.20 11:05, Michal Hocko wrote:
> On Tue 18-02-20 18:19:00, [e-mail] wrote:
>> [e-mail] sed in <[e-mail]>
>>
>>>> On Tue 18-02-20 15:24:48, [e-mail] wrote:
>>>> [...]
>>>>> Tried out the above patch.
>>>>> It seems to be working; no panic, total memory has increased and
>>>>> the hot-added memory is added as HIGHMEM.
>>>>
>>>> I was about to post a patch to mark hotplug broken on 32b but it seems
>>>> you do care about this setup. Could you describe your usecase please?
>>
>> My usecase is testing out the kernel on Hyper-V before loading it on
>> real i686 machine. Hyper-V machine is faster to skim out other bugs.
>> So memory hot-add is not a must requirement for me,
>> but having hot-add may be handy to see the application memory requirement.
>> (as in the anaconda test revealed)
> 
> OK, thanks for the clarification. I am not sure that this qualifies
> as a sufficient reason to maintain the code though.
> 
>> If we're disabling it, we have to announce it somewhere;
>> where is appropriate? `modinfo hv_balloon`'s "hot_add" description?
> 
> This should behave the same way as when the CONFIG_MEMORY_HOTPLULG is
> not enabled. And from a very cursory look hv_balloon.c already checks
> for the config.
> 
> ---
> From 562f21abeda508f199c34358e50fbaa518cd5ed8 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <[e-mail]>
> Date: Tue, 18 Feb 2020 08:04:13 +0100
> Subject: [PATCH] memory_hotplug: disable the functionality for 32b
> 
> Memory hotlug is broken for 32b systems at least since c6f03e2903c9
> ("mm, memory_hotplug: remove zone restrictions") which has considerably
> reworked how can be memory associated with movable/kernel zones. The
> same is not really trivial to achieve in 32b where only lowmem is the
> kernel zone. While we can tweak this immediate problem around there are
> likely other land mines hidden at other places.
> 
> It is also quite dubious that there is a real usecase for the memory
> hotplug on 32b in the first place. Low memory is just too small to be
> hotplugable (for hot add) and generally unusable for hotremove. Adding
> more memory to highmem is also dubious because it would increase the
> low mem or vmalloc space pressure for memmaps.
> 
> Restrict the functionality to 64b systems. This will help future
> development to focus on usecases that have real life application.  We
> can remove this restriction in future in presence of a real life usecase
> of course but until then make it explicit that hotplug on 32b is broken
> and requires a non trivial amount of work to fix.
> 
> Signed-off-by: Michal Hocko <[e-mail]>
> ---
>  mm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index ab80933be65f..2d5fe9e92969 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -154,6 +154,7 @@ config MEMORY_HOTPLUG
>  	bool "Allow for memory hot-add"
>  	depends on SPARSEMEM || X86_64_ACPI_NUMA
>  	depends on ARCH_ENABLE_MEMORY_HOTPLUG
> +	depends on 64BIT || BROKEN
>  
>  config MEMORY_HOTPLUG_SPARSE
>  	def_bool y
> 

Acked-by: David Hildenbrand <[e-mail]>

Baoquan He Feb. 19, 2020, 3:23 a.m. UTC | #2

On 02/18/20 at 11:05am, Michal Hocko wrote:
> On Tue 18-02-20 18:19:00, [e-mail] wrote:
> > [e-mail] sed in <[e-mail]>
> > 
> > >> On Tue 18-02-20 15:24:48, [e-mail] wrote:
> > >> [...]
> > >> > Tried out the above patch.
> > >> > It seems to be working; no panic, total memory has increased and
> > >> > the hot-added memory is added as HIGHMEM.
> > >> 
> > >> I was about to post a patch to mark hotplug broken on 32b but it seems
> > >> you do care about this setup. Could you describe your usecase please?
> > 
> > My usecase is testing out the kernel on Hyper-V before loading it on
> > real i686 machine. Hyper-V machine is faster to skim out other bugs.
> > So memory hot-add is not a must requirement for me,
> > but having hot-add may be handy to see the application memory requirement.
> > (as in the anaconda test revealed)
> 
> OK, thanks for the clarification. I am not sure that this qualifies
> as a sufficient reason to maintain the code though.
> 
> > If we're disabling it, we have to announce it somewhere;
> > where is appropriate? `modinfo hv_balloon`'s "hot_add" description?
> 
> This should behave the same way as when the CONFIG_MEMORY_HOTPLULG is
> not enabled. And from a very cursory look hv_balloon.c already checks
> for the config.
> 
> ---
> From 562f21abeda508f199c34358e50fbaa518cd5ed8 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <[e-mail]>
> Date: Tue, 18 Feb 2020 08:04:13 +0100
> Subject: [PATCH] memory_hotplug: disable the functionality for 32b
> 
> Memory hotlug is broken for 32b systems at least since c6f03e2903c9
> ("mm, memory_hotplug: remove zone restrictions") which has considerably
> reworked how can be memory associated with movable/kernel zones. The
> same is not really trivial to achieve in 32b where only lowmem is the
> kernel zone. While we can tweak this immediate problem around there are
> likely other land mines hidden at other places.
> 
> It is also quite dubious that there is a real usecase for the memory
> hotplug on 32b in the first place. Low memory is just too small to be
> hotplugable (for hot add) and generally unusable for hotremove. Adding
> more memory to highmem is also dubious because it would increase the
> low mem or vmalloc space pressure for memmaps.
> 
> Restrict the functionality to 64b systems. This will help future
> development to focus on usecases that have real life application.  We
> can remove this restriction in future in presence of a real life usecase
> of course but until then make it explicit that hotplug on 32b is broken
> and requires a non trivial amount of work to fix.
> 
> Signed-off-by: Michal Hocko <[e-mail]>

No objection to this, ack.
Acked-by: Baoquan He <[e-mail]>

At least in our distros, we have taken the i386 off from our ARCH lists
for a very long time, hence I personally haven't followed i386 code for
a long time either. This can save our time when maintain the mem hotplug
code. Thanks for making this patch.

Thanks
Baoquan

> ---
>  mm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index ab80933be65f..2d5fe9e92969 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -154,6 +154,7 @@ config MEMORY_HOTPLUG
>  	bool "Allow for memory hot-add"
>  	depends on SPARSEMEM || X86_64_ACPI_NUMA
>  	depends on ARCH_ENABLE_MEMORY_HOTPLUG
> +	depends on 64BIT || BROKEN
>  
>  config MEMORY_HOTPLUG_SPARSE
>  	def_bool y
> -- 
> 2.24.1
> 
> -- 
> Michal Hocko
> SUSE Labs
>

Andrew Morton Feb. 19, 2020, 9:46 p.m. UTC | #3

On Tue, 18 Feb 2020 11:05:32 +0100 Michal Hocko <[e-mail]> wrote:

> Subject: [PATCH] memory_hotplug: disable the functionality for 32b
> 
> Memory hotlug is broken for 32b systems at least since c6f03e2903c9
> ("mm, memory_hotplug: remove zone restrictions") which has considerably
> reworked how can be memory associated with movable/kernel zones. The
> same is not really trivial to achieve in 32b where only lowmem is the
> kernel zone. While we can tweak this immediate problem around there are
> likely other land mines hidden at other places.
> 
> It is also quite dubious that there is a real usecase for the memory
> hotplug on 32b in the first place. Low memory is just too small to be
> hotplugable (for hot add) and generally unusable for hotremove. Adding
> more memory to highmem is also dubious because it would increase the
> low mem or vmalloc space pressure for memmaps.
> 
> Restrict the functionality to 64b systems. This will help future
> development to focus on usecases that have real life application.  We
> can remove this restriction in future in presence of a real life usecase
> of course but until then make it explicit that hotplug on 32b is broken
> and requires a non trivial amount of work to fix.

(cc linux-arch)

(and linux-arm-kernel, as ARM is a major 32-bit user)

Does anyone see a problem with disabling memory hotplug on 32-bit builds?

Robin Murphy Feb. 19, 2020, 11:07 p.m. UTC | #4

On 2020-02-19 9:46 pm, Andrew Morton wrote:
> On Tue, 18 Feb 2020 11:05:32 +0100 Michal Hocko <[e-mail]> wrote:
> 
>> Subject: [PATCH] memory_hotplug: disable the functionality for 32b
>>
>> Memory hotlug is broken for 32b systems at least since c6f03e2903c9
>> ("mm, memory_hotplug: remove zone restrictions") which has considerably
>> reworked how can be memory associated with movable/kernel zones. The
>> same is not really trivial to achieve in 32b where only lowmem is the
>> kernel zone. While we can tweak this immediate problem around there are
>> likely other land mines hidden at other places.
>>
>> It is also quite dubious that there is a real usecase for the memory
>> hotplug on 32b in the first place. Low memory is just too small to be
>> hotplugable (for hot add) and generally unusable for hotremove. Adding
>> more memory to highmem is also dubious because it would increase the
>> low mem or vmalloc space pressure for memmaps.
>>
>> Restrict the functionality to 64b systems. This will help future
>> development to focus on usecases that have real life application.  We
>> can remove this restriction in future in presence of a real life usecase
>> of course but until then make it explicit that hotplug on 32b is broken
>> and requires a non trivial amount of work to fix.
> 
> (cc linux-arch)
> 
> (and linux-arm-kernel, as ARM is a major 32-bit user)
> 
> Does anyone see a problem with disabling memory hotplug on 32-bit builds?

32-bit Arm doesn't support memory hotplug, and as far as I'm aware 
there's little likelihood of it ever wanting to. FWIW it looks like 
SuperH is the only pure-32-bit architecture to have hotplug support at all.

Robin.

Patch

diff --git a/mm/Kconfig b/mm/Kconfig
index ab80933be65f..2d5fe9e92969 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -154,6 +154,7 @@  config MEMORY_HOTPLUG
 	bool "Allow for memory hot-add"
 	depends on SPARSEMEM || X86_64_ACPI_NUMA
 	depends on ARCH_ENABLE_MEMORY_HOTPLUG
+	depends on 64BIT || BROKEN
 
 config MEMORY_HOTPLUG_SPARSE
 	def_bool y