| Message ID | 1389380698-19361-4-git-send-email-prarit@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/11/2014 11:35 AM, 7eggert@gmx.de 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. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 12, 2014 at 6:46 PM, Prarit Bhargava <prarit@redhat.com> wrote: > > > On 01/11/2014 11:35 AM, 7eggert@gmx.de 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. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/13/2014 03:31 PM, KOSAKI Motohiro wrote: > On Sun, Jan 12, 2014 at 6:46 PM, Prarit Bhargava <prarit@redhat.com> wrote: >> >> >> On 01/11/2014 11:35 AM, 7eggert@gmx.de 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. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <prarit@redhat.com> wrote: > >> > >> > >> On 01/11/2014 11:35 AM, 7eggert@gmx.de 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <prarit@redhat.com> wrote: >>>> >>>> >>>> On 01/11/2014 11:35 AM, 7eggert@gmx.de 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 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 /*
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 <prarit@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: x86@kernel.org Cc: Len Brown <lenb@kernel.org> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Linn Crosetto <linn@hp.com> Cc: Pekka Enberg <penberg@kernel.org> Cc: Yinghai Lu <yinghai@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Toshi Kani <toshi.kani@hp.com> Cc: Tang Chen <tangchen@cn.fujitsu.com> Cc: Wen Congyang <wency@cn.fujitsu.com> Cc: Vivek Goyal <vgoyal@redhat.com> Cc: kosaki.motohiro@gmail.com Cc: dyoung@redhat.com Cc: Toshi Kani <toshi.kani@hp.com> Cc: linux-acpi@vger.kernel.org Cc: linux-mm@kvack.org --- 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(-)