2014-01-10 19:06:00

by Prarit Bhargava

[permalink] [raw]
Subject: [PATCH 0/2] Add option to disable ACPI Memory Hotplug [v2]

This patchset adds the ability for the user to disable ACPI Memory Hotplug
by adding "acpi_no_memhotplug" as a kernel paramaeter, and disables
ACPI Memory Hotplug by default when the memmap=exactmap and mem=X parameters
are used.

Signed-off-by: Prarit Bhargava <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Cc: Len Brown <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Linn Crosetto <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Toshi Kani <[email protected]>
Cc: Tang Chen <[email protected]>
Cc: Wen Congyang <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Toshi Kani <[email protected]>
Cc: [email protected]
Cc: [email protected]


Prarit Bhargava (2):
acpi memory hotplug, add parameter to disable memory hotplug [v2]
x86, e820 disable ACPI Memory Hotplug if memory mapping is specified
by user [v2]

Documentation/kernel-parameters.txt | 3 +++
arch/x86/kernel/e820.c | 10 +++++++++-
drivers/acpi/acpi_memhotplug.c | 17 +++++++++++++++++
include/linux/memory_hotplug.h | 3 +++
4 files changed, 32 insertions(+), 1 deletion(-)

--
1.7.9.3


2014-01-10 19:05:37

by Prarit Bhargava

[permalink] [raw]
Subject: [PATCH 1/2] acpi memory hotplug, add parameter to disable memory hotplug [v2]

When booting a kexec/kdump kernel on a system that has specific memory hotplug
regions the boot will fail with warnings like:

[ 2.939467] swapper/0: page allocation failure: order:9, mode:0x84d0
[ 2.946564] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
3.10.0-65.el7.x86_64 #1
[ 2.954532] Hardware name: QCI QSSC-S4R/QSSC-S4R, BIOS
QSSC-S4R.QCI.01.00.S013.032920111005 03/29/2011
[ 2.964926] 0000000000000000 ffff8800341bd8c8 ffffffff815bcc67
ffff8800341bd950
[ 2.973224] ffffffff8113b1a0 ffff880036339b00 0000000000000009
00000000000084d0
[ 2.981523] ffff8800341bd950 ffffffff815b87ee 0000000000000000
0000000000000200
[ 2.989821] Call Trace:
[ 2.992560] [<ffffffff815bcc67>] dump_stack+0x19/0x1b
[ 2.998300] [<ffffffff8113b1a0>] warn_alloc_failed+0xf0/0x160
[ 3.004817] [<ffffffff815b87ee>] ?
__alloc_pages_direct_compact+0xac/0x196
[ 3.012594] [<ffffffff8113f14f>] __alloc_pages_nodemask+0x7ff/0xa00
[ 3.019692] [<ffffffff815b417c>] vmemmap_alloc_block+0x62/0xba
[ 3.026303] [<ffffffff815b41e9>] vmemmap_alloc_block_buf+0x15/0x3b
[ 3.033302] [<ffffffff815b1ff6>] vmemmap_populate+0xb4/0x21b
[ 3.039718] [<ffffffff815b461d>] sparse_mem_map_populate+0x27/0x35
[ 3.046717] [<ffffffff815b400f>] sparse_add_one_section+0x7a/0x185
[ 3.053720] [<ffffffff815a1e9f>] __add_pages+0xaf/0x240
[ 3.059656] [<ffffffff81047359>] arch_add_memory+0x59/0xd0
[ 3.065877] [<ffffffff815a21d9>] add_memory+0xb9/0x1b0
[ 3.071713] [<ffffffff81333b9c>] acpi_memory_device_add+0x18d/0x26d
[ 3.078813] [<ffffffff81309a01>] acpi_bus_device_attach+0x7d/0xcd
[ 3.085719] [<ffffffff8132379d>] acpi_ns_walk_namespace+0xc8/0x17f
[ 3.092716] [<ffffffff81309984>] ? acpi_bus_type_and_status+0x90/0x90
[ 3.100004] [<ffffffff81309984>] ? acpi_bus_type_and_status+0x90/0x90
[ 3.107293] [<ffffffff81323c8c>] acpi_walk_namespace+0x95/0xc5
[ 3.113904] [<ffffffff8130a6d6>] acpi_bus_scan+0x8b/0x9d
[ 3.119933] [<ffffffff81a2019a>] acpi_scan_init+0x63/0x160
[ 3.126153] [<ffffffff81a1ffb5>] acpi_init+0x25d/0x2a6
[ 3.131987] [<ffffffff81a1fd58>] ? acpi_sleep_proc_init+0x2a/0x2a
[ 3.138889] [<ffffffff810020e2>] do_one_initcall+0xe2/0x190
[ 3.145210] [<ffffffff819e20c4>] kernel_init_freeable+0x17c/0x207
[ 3.152111] [<ffffffff819e18d0>] ? do_early_param+0x88/0x88
[ 3.158430] [<ffffffff8159fea0>] ? rest_init+0x80/0x80
[ 3.164264] [<ffffffff8159feae>] kernel_init+0xe/0x180
[ 3.170097] [<ffffffff815cca2c>] ret_from_fork+0x7c/0xb0
[ 3.176123] [<ffffffff8159fea0>] ? rest_init+0x80/0x80
[ 3.181956] Mem-Info:
[ 3.184490] Node 0 DMA per-cpu:
[ 3.188007] CPU 0: hi: 0, btch: 1 usd: 0
[ 3.193353] Node 0 DMA32 per-cpu:
[ 3.197060] CPU 0: hi: 42, btch: 7 usd: 0
[ 3.202410] active_anon:0 inactive_anon:0 isolated_anon:0
[ 3.202410] active_file:0 inactive_file:0 isolated_file:0
[ 3.202410] unevictable:0 dirty:0 writeback:0 unstable:0
[ 3.202410] free:872 slab_reclaimable:13 slab_unreclaimable:1880
[ 3.202410] mapped:0 shmem:0 pagetables:0 bounce:0
[ 3.202410] free_cma:0

because the system has run out of memory at boot time. This occurs
because of the following sequence in the boot:

Main kernel boots and sets E820 map. The second kernel is booted with a
map generated by the kdump service using memmap= and memmap=exactmap.
These parameters are added to the kernel parameters of the kexec/kdump
kernel. The kexec/kdump kernel has limited memory resources so as not
to severely impact the main kernel.

The system then panics and the kdump/kexec kernel boots (which is a
completely new kernel boot). During this boot ACPI is initialized and the
kernel (as can be seen above) traverses the ACPI namespace and finds an
entry for a memory device to be hotadded.

ie)

[ 3.053720] [<ffffffff815a1e9f>] __add_pages+0xaf/0x240
[ 3.059656] [<ffffffff81047359>] arch_add_memory+0x59/0xd0
[ 3.065877] [<ffffffff815a21d9>] add_memory+0xb9/0x1b0
[ 3.071713] [<ffffffff81333b9c>] acpi_memory_device_add+0x18d/0x26d
[ 3.078813] [<ffffffff81309a01>] acpi_bus_device_attach+0x7d/0xcd
[ 3.085719] [<ffffffff8132379d>] acpi_ns_walk_namespace+0xc8/0x17f
[ 3.092716] [<ffffffff81309984>] ? acpi_bus_type_and_status+0x90/0x90
[ 3.100004] [<ffffffff81309984>] ? acpi_bus_type_and_status+0x90/0x90
[ 3.107293] [<ffffffff81323c8c>] acpi_walk_namespace+0x95/0xc5
[ 3.113904] [<ffffffff8130a6d6>] acpi_bus_scan+0x8b/0x9d
[ 3.119933] [<ffffffff81a2019a>] acpi_scan_init+0x63/0x160
[ 3.126153] [<ffffffff81a1ffb5>] acpi_init+0x25d/0x2a6

At this point the kernel adds page table information and the the kexec/kdump
kernel runs out of memory.

This can also be reproduced by using the memmap=exactmap and mem=X
parameters on the main kernel and booting.

This patchset resolves the problem by adding a kernel parameter,
acpi_no_memhotplug, to disable ACPI memory hotplug.

[v2]: changed acpi_no_memhotplug to bool

Signed-off-by: Prarit Bhargava <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Cc: Len Brown <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Linn Crosetto <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Toshi Kani <[email protected]>
Cc: Tang Chen <[email protected]>
Cc: Wen Congyang <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Toshi Kani <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
Documentation/kernel-parameters.txt | 3 +++
drivers/acpi/acpi_memhotplug.c | 12 ++++++++++++
2 files changed, 15 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index b9e9bd8..41374f9 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2117,6 +2117,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

nomce [X86-32] Machine Check Exception

+ acpi_no_memhotplug [ACPI] Disable memory hotplug. Useful for kexec
+ and kdump kernels.
+
nomfgpt [X86-32] Disable Multi-Function General Purpose
Timer usage (for AMD Geode machines).

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 551dad7..4a0fa94 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -361,7 +361,19 @@ static void acpi_memory_device_remove(struct acpi_device *device)
acpi_memory_device_free(mem_device);
}

+static bool acpi_no_memhotplug;
+
void __init acpi_memory_hotplug_init(void)
{
+ if (acpi_no_memhotplug)
+ return;
+
acpi_scan_add_handler_with_hotplug(&memory_device_handler, "memory");
}
+
+static int __init disable_acpi_memory_hotplug(char *str)
+{
+ acpi_no_memhotplug = true;
+ return 1;
+}
+__setup("acpi_no_memhotplug", disable_acpi_memory_hotplug);
--
1.7.9.3

2014-01-10 19:05:35

by Prarit Bhargava

[permalink] [raw]
Subject: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

kdump uses memmap=exactmap and mem=X values to configure the memory
mapping for the kdump kernel. If memory is hotadded during the boot of
the kdump kernel it is possible that the page tables for the new memory
cause the kdump kernel to run out of memory.

Since the user has specified a specific mapping ACPI Memory Hotplug should be
disabled in this case.

[v2]: really add mem=

Signed-off-by: Prarit Bhargava <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Cc: Len Brown <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Linn Crosetto <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Toshi Kani <[email protected]>
Cc: Tang Chen <[email protected]>
Cc: Wen Congyang <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Toshi Kani <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/x86/kernel/e820.c | 10 +++++++++-
drivers/acpi/acpi_memhotplug.c | 7 ++++++-
include/linux/memory_hotplug.h | 3 +++
3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 174da5f..747f36a 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -20,6 +20,7 @@
#include <linux/firmware-map.h>
#include <linux/memblock.h>
#include <linux/sort.h>
+#include <linux/memory_hotplug.h>

#include <asm/e820.h>
#include <asm/proto.h>
@@ -834,6 +835,8 @@ static int __init parse_memopt(char *p)
return -EINVAL;
e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);

+ set_acpi_no_memhotplug();
+
return 0;
}
early_param("mem", parse_memopt);
@@ -880,15 +883,20 @@ static int __init parse_memmap_one(char *p)

return *p == '\0' ? 0 : -EINVAL;
}
+
static int __init parse_memmap_opt(char *str)
{
+ int ret;
+
while (str) {
char *k = strchr(str, ',');

if (k)
*k++ = 0;

- parse_memmap_one(str);
+ ret = parse_memmap_one(str);
+ if (!ret)
+ set_acpi_no_memhotplug();
str = k;
}

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 4a0fa94..48b9267 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -363,6 +363,11 @@ static void acpi_memory_device_remove(struct acpi_device *device)

static bool acpi_no_memhotplug;

+void set_acpi_no_memhotplug(void)
+{
+ acpi_no_memhotplug = true;
+}
+
void __init acpi_memory_hotplug_init(void)
{
if (acpi_no_memhotplug)
@@ -373,7 +378,7 @@ void __init acpi_memory_hotplug_init(void)

static int __init disable_acpi_memory_hotplug(char *str)
{
- acpi_no_memhotplug = true;
+ set_acpi_no_memhotplug();
return 1;
}
__setup("acpi_no_memhotplug", disable_acpi_memory_hotplug);
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 4ca3d95..80f5a23 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -12,6 +12,9 @@ struct pglist_data;
struct mem_section;
struct memory_block;

+/* set flag to disable ACPI memory hotplug */
+extern void set_acpi_no_memhotplug(void);
+
#ifdef CONFIG_MEMORY_HOTPLUG

/*
--
1.7.9.3

2014-01-10 21:12:24

by Vivek Goyal

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On Fri, Jan 10, 2014 at 02:04:58PM -0500, Prarit Bhargava wrote:

> kdump uses memmap=exactmap and mem=X values

Minor nit. Kdump only uses memmap=exactmap and not mem=X. mem=X is there
for debugging. So lets fix the changelog.

[..]
> static int __init parse_memmap_opt(char *str)
> {
> + int ret;
> +
> while (str) {
> char *k = strchr(str, ',');
>
> if (k)
> *k++ = 0;
>
> - parse_memmap_one(str);
> + ret = parse_memmap_one(str);
> + if (!ret)
> + set_acpi_no_memhotplug();

We want to call this only in case of memmap=exactmap and not other memmap=
options. So I think instead of here, call it inside parse_memmap_one()
where exactmap check is done.

if (!strncmp(p, "exactmap", 8)) {
set_acpi_no_memhotplug();
}

Thanks
Vivek

2014-01-10 21:40:01

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH 1/2] acpi memory hotplug, add parameter to disable memory hotplug [v2]

On Fri, 2014-01-10 at 14:04 -0500, Prarit Bhargava wrote:
:
> ---
> Documentation/kernel-parameters.txt | 3 +++
> drivers/acpi/acpi_memhotplug.c | 12 ++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index b9e9bd8..41374f9 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2117,6 +2117,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> nomce [X86-32] Machine Check Exception
>
> + acpi_no_memhotplug [ACPI] Disable memory hotplug. Useful for kexec
> + and kdump kernels.
> +

Please move it to where other acpi_xxx are described.

For kdump kernel, this option will be used when memmap=exactmap is
deprecated. IOW, it is not useful yet. Not sure what you mean by kexec
kernel. Memory hotplug does not need to be disabled for kexec reboot.

Thanks,
-Toshi


2014-01-10 21:40:47

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On Fri, 2014-01-10 at 14:04 -0500, Prarit Bhargava wrote:
:
> arch/x86/kernel/e820.c | 10 +++++++++-
> drivers/acpi/acpi_memhotplug.c | 7 ++++++-
> include/linux/memory_hotplug.h | 3 +++
> 3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 174da5f..747f36a 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -20,6 +20,7 @@
> #include <linux/firmware-map.h>
> #include <linux/memblock.h>
> #include <linux/sort.h>
> +#include <linux/memory_hotplug.h>
>
> #include <asm/e820.h>
> #include <asm/proto.h>
> @@ -834,6 +835,8 @@ static int __init parse_memopt(char *p)
> return -EINVAL;
> e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
>
> + set_acpi_no_memhotplug();
> +

It won't build when CONFIG_ACPI_HOTPLUG_MEMORY is not defined.

Thanks,
-Toshi



2014-01-11 16:35:13

by Bodo Eggert

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]



On Fri, 10 Jan 2014, Prarit Bhargava wrote:

> kdump uses memmap=exactmap and mem=X values to configure the memory
> mapping for the kdump kernel. If memory is hotadded during the boot of
> the kdump kernel it is possible that the page tables for the new memory
> cause the kdump kernel to run out of memory.
>
> Since the user has specified a specific mapping ACPI Memory Hotplug should be
> disabled in this case.

I'll ask just in case: Is it possible to want memory hotplug in spite of
using memmap=exactmap or mem=X?

2014-01-12 23:47:14

by Prarit Bhargava

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]



On 01/11/2014 11:35 AM, [email protected] wrote:
>
>
> On Fri, 10 Jan 2014, Prarit Bhargava wrote:
>
>> kdump uses memmap=exactmap and mem=X values to configure the memory
>> mapping for the kdump kernel. If memory is hotadded during the boot of
>> the kdump kernel it is possible that the page tables for the new memory
>> cause the kdump kernel to run out of memory.
>>
>> Since the user has specified a specific mapping ACPI Memory Hotplug should be
>> disabled in this case.
>
> I'll ask just in case: Is it possible to want memory hotplug in spite of
> using memmap=exactmap or mem=X?

Good question -- I can't think of a case. When a user specifies "memmap" or
"mem" IMO they are asking for a very specific memory configuration. Having
extra memory added above what the user has specified seems to defeat the purpose
of "memmap" and "mem".

P.

2014-01-13 20:31:29

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On Sun, Jan 12, 2014 at 6:46 PM, Prarit Bhargava <[email protected]> wrote:
>
>
> On 01/11/2014 11:35 AM, [email protected] wrote:
>>
>>
>> On Fri, 10 Jan 2014, Prarit Bhargava wrote:
>>
>>> kdump uses memmap=exactmap and mem=X values to configure the memory
>>> mapping for the kdump kernel. If memory is hotadded during the boot of
>>> the kdump kernel it is possible that the page tables for the new memory
>>> cause the kdump kernel to run out of memory.
>>>
>>> Since the user has specified a specific mapping ACPI Memory Hotplug should be
>>> disabled in this case.
>>
>> I'll ask just in case: Is it possible to want memory hotplug in spite of
>> using memmap=exactmap or mem=X?
>
> Good question -- I can't think of a case. When a user specifies "memmap" or
> "mem" IMO they are asking for a very specific memory configuration. Having
> extra memory added above what the user has specified seems to defeat the purpose
> of "memmap" and "mem".

May be yes, may be no.

They are often used for a wrokaround to avoid broken firmware issue.
If we have no way
to explicitly enable hotplug. We will lose a workaround.

Perhaps, there is no matter. Today, memory hotplug is only used on
high-end machine
and their firmware is carefully developped and don't have a serious
issue almostly. Though.

2014-01-13 23:40:19

by Prarit Bhargava

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]



On 01/13/2014 03:31 PM, KOSAKI Motohiro wrote:
> On Sun, Jan 12, 2014 at 6:46 PM, Prarit Bhargava <[email protected]> wrote:
>>
>>
>> On 01/11/2014 11:35 AM, [email protected] wrote:
>>>
>>>
>>> On Fri, 10 Jan 2014, Prarit Bhargava wrote:
>>>
>>>> kdump uses memmap=exactmap and mem=X values to configure the memory
>>>> mapping for the kdump kernel. If memory is hotadded during the boot of
>>>> the kdump kernel it is possible that the page tables for the new memory
>>>> cause the kdump kernel to run out of memory.
>>>>
>>>> Since the user has specified a specific mapping ACPI Memory Hotplug should be
>>>> disabled in this case.
>>>
>>> I'll ask just in case: Is it possible to want memory hotplug in spite of
>>> using memmap=exactmap or mem=X?
>>
>> Good question -- I can't think of a case. When a user specifies "memmap" or
>> "mem" IMO they are asking for a very specific memory configuration. Having
>> extra memory added above what the user has specified seems to defeat the purpose
>> of "memmap" and "mem".
>
> May be yes, may be no.
>
> They are often used for a wrokaround to avoid broken firmware issue.
> If we have no way
> to explicitly enable hotplug. We will lose a workaround.
>
> Perhaps, there is no matter. Today, memory hotplug is only used on
> high-end machine
> and their firmware is carefully developped and don't have a serious
> issue almostly. Though.

Oof -- sorry Kosaki :( I didn't see this until just now (and your subsequent
ACK on the updated patch).

I just remembered that we did have a processor vendor's whitebox that would not
boot unless we specified a specific memmap and we did specify memmap=exactmap to
boot the system correctly and the system had hotplug memory.

So it means that I should not key off of "memmap=exactmap".

I will self-NAK the updated patch and submit a new one.

P.

2014-01-14 00:39:58

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On Mon, 2014-01-13 at 18:39 -0500, Prarit Bhargava wrote:
>
> On 01/13/2014 03:31 PM, KOSAKI Motohiro wrote:
> > On Sun, Jan 12, 2014 at 6:46 PM, Prarit Bhargava <[email protected]> wrote:
> >>
> >>
> >> On 01/11/2014 11:35 AM, [email protected] wrote:
> >>>
> >>>
> >>> On Fri, 10 Jan 2014, Prarit Bhargava wrote:
> >>>
> >>>> kdump uses memmap=exactmap and mem=X values to configure the memory
> >>>> mapping for the kdump kernel. If memory is hotadded during the boot of
> >>>> the kdump kernel it is possible that the page tables for the new memory
> >>>> cause the kdump kernel to run out of memory.
> >>>>
> >>>> Since the user has specified a specific mapping ACPI Memory Hotplug should be
> >>>> disabled in this case.
> >>>
> >>> I'll ask just in case: Is it possible to want memory hotplug in spite of
> >>> using memmap=exactmap or mem=X?
> >>
> >> Good question -- I can't think of a case. When a user specifies "memmap" or
> >> "mem" IMO they are asking for a very specific memory configuration. Having
> >> extra memory added above what the user has specified seems to defeat the purpose
> >> of "memmap" and "mem".
> >
> > May be yes, may be no.
> >
> > They are often used for a wrokaround to avoid broken firmware issue.
> > If we have no way
> > to explicitly enable hotplug. We will lose a workaround.
> >
> > Perhaps, there is no matter. Today, memory hotplug is only used on
> > high-end machine
> > and their firmware is carefully developped and don't have a serious
> > issue almostly. Though.
>
> Oof -- sorry Kosaki :( I didn't see this until just now (and your subsequent
> ACK on the updated patch).
>
> I just remembered that we did have a processor vendor's whitebox that would not
> boot unless we specified a specific memmap and we did specify memmap=exactmap to
> boot the system correctly and the system had hotplug memory.
>
> So it means that I should not key off of "memmap=exactmap".

I do not think it makes sense. You needed memmap=exactmap as a
workaround because the kernel did not boot with the firmware's memory
info. So, it's broken, and you requested the kernel to ignore the
firmware info.

Why do you think memory hotplug needs to be supported under such
condition, which has to use the broken firmware info?

Thanks,
-Toshi


2014-01-14 00:54:32

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On 01/13/2014 04:33 PM, Toshi Kani wrote:
>
> I do not think it makes sense. You needed memmap=exactmap as a
> workaround because the kernel did not boot with the firmware's memory
> info. So, it's broken, and you requested the kernel to ignore the
> firmware info.
>
> Why do you think memory hotplug needs to be supported under such
> condition, which has to use the broken firmware info?
>

Even more than memory hotplug: what do we do with NUMA? Since we have
already told the kernel "the firmware is bogus" it would seem that any
NUMA optimizations would be a bit ... cantankerous at best, no?

-hpa

2014-01-14 01:15:11

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On Mon, 2014-01-13 at 16:53 -0800, H. Peter Anvin wrote:
> On 01/13/2014 04:33 PM, Toshi Kani wrote:
> >
> > I do not think it makes sense. You needed memmap=exactmap as a
> > workaround because the kernel did not boot with the firmware's memory
> > info. So, it's broken, and you requested the kernel to ignore the
> > firmware info.
> >
> > Why do you think memory hotplug needs to be supported under such
> > condition, which has to use the broken firmware info?
> >
>
> Even more than memory hotplug: what do we do with NUMA? Since we have
> already told the kernel "the firmware is bogus" it would seem that any
> NUMA optimizations would be a bit ... cantankerous at best, no?

Agreed that NUMA info can be bogus in this case, but is probably not
critical.

In majority of the cases, memmap=exactmap is used for kdump and the
firmware info is sane. So, I think we should keep NUMA enabled since it
could be useful when multiple CPUs are enabled for kdump.

Thanks,
-Toshi

2014-01-14 01:30:24

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On 01/13/2014 05:09 PM, Toshi Kani wrote:
>
> In majority of the cases, memmap=exactmap is used for kdump and the
> firmware info is sane. So, I think we should keep NUMA enabled since it
> could be useful when multiple CPUs are enabled for kdump.
>

Rather unlikely since all of the kdump memory is likely to sit in a
single node.

-hpa

2014-01-14 01:46:19

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On Mon, 2014-01-13 at 17:29 -0800, H. Peter Anvin wrote:
> On 01/13/2014 05:09 PM, Toshi Kani wrote:
> >
> > In majority of the cases, memmap=exactmap is used for kdump and the
> > firmware info is sane. So, I think we should keep NUMA enabled since it
> > could be useful when multiple CPUs are enabled for kdump.
> >
>
> Rather unlikely since all of the kdump memory is likely to sit in a
> single node.

Right, but CPUs are distributed into multiple nodes, which dump the 1st
kernel's memory. So, these CPUs should dump their local memory ranges.

Thanks,
-Toshi

2014-01-14 01:51:57

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On 01/13/14 at 06:09pm, Toshi Kani wrote:
> On Mon, 2014-01-13 at 16:53 -0800, H. Peter Anvin wrote:
> > On 01/13/2014 04:33 PM, Toshi Kani wrote:
> > >
> > > I do not think it makes sense. You needed memmap=exactmap as a
> > > workaround because the kernel did not boot with the firmware's memory
> > > info. So, it's broken, and you requested the kernel to ignore the
> > > firmware info.
> > >
> > > Why do you think memory hotplug needs to be supported under such
> > > condition, which has to use the broken firmware info?
> > >
> >
> > Even more than memory hotplug: what do we do with NUMA? Since we have
> > already told the kernel "the firmware is bogus" it would seem that any
> > NUMA optimizations would be a bit ... cantankerous at best, no?
>
> Agreed that NUMA info can be bogus in this case, but is probably not
> critical.
>
> In majority of the cases, memmap=exactmap is used for kdump and the
> firmware info is sane. So, I think we should keep NUMA enabled since it
> could be useful when multiple CPUs are enabled for kdump.

In Fedora kdump, we by default add numa=off to 2nd kernel cmdline because
enabling numa will use a lot more memory, at the same time we have only 128M
reserved by default..

Thanks
Dave

2014-01-14 01:53:39

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]

On Tue, 2014-01-14 at 09:52 +0800, Dave Young wrote:
> On 01/13/14 at 06:09pm, Toshi Kani wrote:
> > On Mon, 2014-01-13 at 16:53 -0800, H. Peter Anvin wrote:
> > > On 01/13/2014 04:33 PM, Toshi Kani wrote:
> > > >
> > > > I do not think it makes sense. You needed memmap=exactmap as a
> > > > workaround because the kernel did not boot with the firmware's memory
> > > > info. So, it's broken, and you requested the kernel to ignore the
> > > > firmware info.
> > > >
> > > > Why do you think memory hotplug needs to be supported under such
> > > > condition, which has to use the broken firmware info?
> > > >
> > >
> > > Even more than memory hotplug: what do we do with NUMA? Since we have
> > > already told the kernel "the firmware is bogus" it would seem that any
> > > NUMA optimizations would be a bit ... cantankerous at best, no?
> >
> > Agreed that NUMA info can be bogus in this case, but is probably not
> > critical.
> >
> > In majority of the cases, memmap=exactmap is used for kdump and the
> > firmware info is sane. So, I think we should keep NUMA enabled since it
> > could be useful when multiple CPUs are enabled for kdump.
>
> In Fedora kdump, we by default add numa=off to 2nd kernel cmdline because
> enabling numa will use a lot more memory, at the same time we have only 128M
> reserved by default..

That quite makes sense as we only enable a single CPU today.

Thanks,
-Toshi

2014-01-14 11:02:41

by Prarit Bhargava

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86, e820 disable ACPI Memory Hotplug if memory mapping is specified by user [v2]



On 01/13/2014 07:33 PM, Toshi Kani wrote:
> On Mon, 2014-01-13 at 18:39 -0500, Prarit Bhargava wrote:
>>
>> On 01/13/2014 03:31 PM, KOSAKI Motohiro wrote:
>>> On Sun, Jan 12, 2014 at 6:46 PM, Prarit Bhargava <[email protected]> wrote:
>>>>
>>>>
>>>> On 01/11/2014 11:35 AM, [email protected] wrote:
>>>>>
>>>>>
>>>>> On Fri, 10 Jan 2014, Prarit Bhargava wrote:
>>>>>
>>>>>> kdump uses memmap=exactmap and mem=X values to configure the memory
>>>>>> mapping for the kdump kernel. If memory is hotadded during the boot of
>>>>>> the kdump kernel it is possible that the page tables for the new memory
>>>>>> cause the kdump kernel to run out of memory.
>>>>>>
>>>>>> Since the user has specified a specific mapping ACPI Memory Hotplug should be
>>>>>> disabled in this case.
>>>>>
>>>>> I'll ask just in case: Is it possible to want memory hotplug in spite of
>>>>> using memmap=exactmap or mem=X?
>>>>
>>>> Good question -- I can't think of a case. When a user specifies "memmap" or
>>>> "mem" IMO they are asking for a very specific memory configuration. Having
>>>> extra memory added above what the user has specified seems to defeat the purpose
>>>> of "memmap" and "mem".
>>>
>>> May be yes, may be no.
>>>
>>> They are often used for a wrokaround to avoid broken firmware issue.
>>> If we have no way
>>> to explicitly enable hotplug. We will lose a workaround.
>>>
>>> Perhaps, there is no matter. Today, memory hotplug is only used on
>>> high-end machine
>>> and their firmware is carefully developped and don't have a serious
>>> issue almostly. Though.
>>
>> Oof -- sorry Kosaki :( I didn't see this until just now (and your subsequent
>> ACK on the updated patch).
>>
>> I just remembered that we did have a processor vendor's whitebox that would not
>> boot unless we specified a specific memmap and we did specify memmap=exactmap to
>> boot the system correctly and the system had hotplug memory.
>>
>> So it means that I should not key off of "memmap=exactmap".
>
> I do not think it makes sense. You needed memmap=exactmap as a
> workaround because the kernel did not boot with the firmware's memory
> info. So, it's broken, and you requested the kernel to ignore the
> firmware info.
>
> Why do you think memory hotplug needs to be supported under such
> condition, which has to use the broken firmware info?

There was a memory address region that was not in the e820 map that had to be
reserved for a specific device's use. It was not so we used memmap=exactmap and
other memmap entries to "rewrite" the e820 map to see if the system would boot;
then I filed a bug against the hardware vendor ;)

So, yes, in that case I did want memory hotplug & memmap=exactmap. Admittedly
this was a rare corner case, and I certainly could have recompiled the kernel.

P.

>
> Thanks,
> -Toshi
>
>
>