From: Hari Bathini <hbathini@linux.ibm.com> Date: 2019-06-27 19:23:11
Sometimes, memory reservation for KDump/FADump can overlap with memory
marked for hugepages. This overlap leads to error, hang in KDump case
and copy error reported by f/w in case of FADump, while trying to
capture dump. Report error while setting up memory for the capture
kernel instead of running into issues while capturing dump, by moving
KDump/FADump reservation below MMU early init and failing gracefully
when hugepages memory overlaps with capture kernel memory.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
arch/powerpc/kernel/prom.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
From: Hari Bathini <hbathini@linux.ibm.com> Date: 2019-06-27 19:25:09
Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
arch/powerpc/kernel/fadump.c | 16 ----------------
arch/powerpc/kernel/machine_kexec.c | 22 +++++++++++-----------
2 files changed, 11 insertions(+), 27 deletions(-)
@@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)#endif}-/*-*Calculatethememoryboundary.-*Ifmemory_limitislessthanactualmemoryboundarythenreserve-*thememoryforfadumpbeyondthememory_limitandadjustthe-*memory_limitaccordingly,sothattherunningkernelcanrunwith-*specifiedmemory_limit.-*/-if(memory_limit&&memory_limit<memblock_end_of_DRAM()){-size=get_fadump_area_size();-if((memory_limit+size)<memblock_end_of_DRAM())-memory_limit+=size;-else-memory_limit=memblock_end_of_DRAM();-printk(KERN_INFO"Adjusted memory_limit for firmware-assisted"-" dump, now %#016llx\n",memory_limit);-}if(memory_limit)memory_boundary=memory_limit;else
@@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)crashk_res.end=crash_base+crash_size-1;}-if(crashk_res.end==crashk_res.start){-crashk_res.start=crashk_res.end=0;-return;-}+if(crashk_res.end==crashk_res.start)+gotoerror_out;/* We might have got these values via the command line or the*devicetree,eitherwaysanitisethemnow.*/
@@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)if(overlaps_crashkernel(__pa(_stext),_end-_stext)){printk(KERN_WARNING"Crash kernel can not overlap current kernel\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}/* Crash kernel trumps memory limit */if(memory_limit&&memory_limit<=crashk_res.end){-memory_limit=crashk_res.end+1;-printk("Adjusted memory limit for crashkernel, now 0x%llx\n",-memory_limit);+pr_err("Crash kernel size can't exceed memory_limit\n");+gotoerror_out;}printk(KERN_INFO"Reserving %ldMB of memory at %ldMB "
@@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)if(!memblock_is_region_memory(crashk_res.start,crash_size)||memblock_reserve(crashk_res.start,crash_size)){pr_err("Failed to reserve memory for crashkernel!\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}++return;+error_out:+crashk_res.start=crashk_res.end=0;+return;}intoverlaps_crashkernel(unsignedlongstart,unsignedlongsize)
From: Michal Suchánek <hidden> Date: 2019-07-22 17:59:06
On Fri, 28 Jun 2019 00:51:19 +0530
Hari Bathini [off-list ref] wrote:
Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.
Can you split out the memory limit adjustment out of memory reservation
so it can still be adjusted?
Thanks
Michal
@@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)#endif}-/*-*Calculatethememoryboundary.-*Ifmemory_limitislessthanactualmemoryboundarythenreserve-*thememoryforfadumpbeyondthememory_limitandadjustthe-*memory_limitaccordingly,sothattherunningkernelcanrunwith-*specifiedmemory_limit.-*/-if(memory_limit&&memory_limit<memblock_end_of_DRAM()){-size=get_fadump_area_size();-if((memory_limit+size)<memblock_end_of_DRAM())-memory_limit+=size;-else-memory_limit=memblock_end_of_DRAM();-printk(KERN_INFO"Adjusted memory_limit for firmware-assisted"-" dump, now %#016llx\n",memory_limit);-}if(memory_limit)memory_boundary=memory_limit;else
@@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)crashk_res.end=crash_base+crash_size-1;}-if(crashk_res.end==crashk_res.start){-crashk_res.start=crashk_res.end=0;-return;-}+if(crashk_res.end==crashk_res.start)+gotoerror_out;/* We might have got these values via the command line or the*devicetree,eitherwaysanitisethemnow.*/
@@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)if(overlaps_crashkernel(__pa(_stext),_end-_stext)){printk(KERN_WARNING"Crash kernel can not overlap current kernel\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}/* Crash kernel trumps memory limit */if(memory_limit&&memory_limit<=crashk_res.end){-memory_limit=crashk_res.end+1;-printk("Adjusted memory limit for crashkernel, now 0x%llx\n",-memory_limit);+pr_err("Crash kernel size can't exceed memory_limit\n");+gotoerror_out;}printk(KERN_INFO"Reserving %ldMB of memory at %ldMB "
@@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)if(!memblock_is_region_memory(crashk_res.start,crash_size)||memblock_reserve(crashk_res.start,crash_size)){pr_err("Failed to reserve memory for crashkernel!\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}++return;+error_out:+crashk_res.start=crashk_res.end=0;+return;}intoverlaps_crashkernel(unsignedlongstart,unsignedlongsize)
On Fri, 28 Jun 2019 00:51:19 +0530
Hari Bathini [off-list ref] wrote:
quoted
Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.
Can you split out the memory limit adjustment out of memory reservation
so it can still be adjusted?
Do you mean adjust the memory limit before we do the actual reservation ?
@@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)#endif}-/*-*Calculatethememoryboundary.-*Ifmemory_limitislessthanactualmemoryboundarythenreserve-*thememoryforfadumpbeyondthememory_limitandadjustthe-*memory_limitaccordingly,sothattherunningkernelcanrunwith-*specifiedmemory_limit.-*/-if(memory_limit&&memory_limit<memblock_end_of_DRAM()){-size=get_fadump_area_size();-if((memory_limit+size)<memblock_end_of_DRAM())-memory_limit+=size;-else-memory_limit=memblock_end_of_DRAM();-printk(KERN_INFO"Adjusted memory_limit for firmware-assisted"-" dump, now %#016llx\n",memory_limit);-}if(memory_limit)memory_boundary=memory_limit;else
@@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)crashk_res.end=crash_base+crash_size-1;}-if(crashk_res.end==crashk_res.start){-crashk_res.start=crashk_res.end=0;-return;-}+if(crashk_res.end==crashk_res.start)+gotoerror_out;/* We might have got these values via the command line or the*devicetree,eitherwaysanitisethemnow.*/
@@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)if(overlaps_crashkernel(__pa(_stext),_end-_stext)){printk(KERN_WARNING"Crash kernel can not overlap current kernel\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}/* Crash kernel trumps memory limit */if(memory_limit&&memory_limit<=crashk_res.end){-memory_limit=crashk_res.end+1;-printk("Adjusted memory limit for crashkernel, now 0x%llx\n",-memory_limit);+pr_err("Crash kernel size can't exceed memory_limit\n");+gotoerror_out;}printk(KERN_INFO"Reserving %ldMB of memory at %ldMB "
@@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)if(!memblock_is_region_memory(crashk_res.start,crash_size)||memblock_reserve(crashk_res.start,crash_size)){pr_err("Failed to reserve memory for crashkernel!\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}++return;+error_out:+crashk_res.start=crashk_res.end=0;+return;}intoverlaps_crashkernel(unsignedlongstart,unsignedlongsize)
From: Michal Suchánek <hidden> Date: 2020-01-06 16:12:38
On Wed, Jul 24, 2019 at 11:26:59AM +0530, Mahesh Jagannath Salgaonkar wrote:
On 7/22/19 11:19 PM, Michal Suchánek wrote:
quoted
On Fri, 28 Jun 2019 00:51:19 +0530
Hari Bathini [off-list ref] wrote:
quoted
Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.
Can you split out the memory limit adjustment out of memory reservation
so it can still be adjusted?
Do you mean adjust the memory limit before we do the actual reservation ?
Yes, without that you get a regression in ability to enable fadump with
limited memory - something like the below patch should fix it. Then
again, there is no code to un-move the memory_limit in case the allocation
fails, and we now have cma allocation which is dubious to allocate
beyond memory_limit. So maybe removing the memory_limit adjustment is a
bugfix removing 'feature' that has bitrotten over time.
Thanks
Michal
From: Michal Suchanek <redacted>
Date: Mon, 6 Jan 2020 14:55:40 +0100
Subject: [PATCH 2/2] powerpc/fadump: adjust memlimit before MMU early init
Moving the farump memory reservation before early MMU init makes the
memlimit adjustment to make room for fadump ineffective.
Move the adjustment back before early MMU init.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/include/asm/fadump.h | 3 +-
arch/powerpc/kernel/fadump.c | 80 +++++++++++++++++++++++--------
arch/powerpc/kernel/prom.c | 3 ++
3 files changed, 66 insertions(+), 20 deletions(-)
@@ -431,19 +431,22 @@ static int __init fadump_get_boot_mem_regions(void)returnret;}-int__initfadump_reserve_mem(void)+staticinlineu64fadump_get_reserve_alignment(void){-u64base,size,mem_boundary,bootmem_min,align=PAGE_SIZE;-boolis_memblock_bottom_up=memblock_bottom_up();-intret=1;+u64align=PAGE_SIZE;-if(!fw_dump.fadump_enabled)-return0;+#ifdef CONFIG_CMA+if(!fw_dump.nocma)+align=FADUMP_CMA_ALIGNMENT;+#endif-if(!fw_dump.fadump_supported){-pr_info("Firmware-Assisted Dump is not supported on this hardware\n");-gotoerror_out;-}+returnalign;+}++staticinlineu64fadump_get_bootmem_min(void)+{+u64bootmem_min=0;+u64align=fadump_get_reserve_alignment();/**Initializebootmemorysize
@@ -455,7 +458,6 @@ int __init fadump_reserve_mem(void)PAGE_ALIGN(fadump_calculate_reserve_size());#ifdef CONFIG_CMAif(!fw_dump.nocma){-align=FADUMP_CMA_ALIGNMENT;fw_dump.boot_memory_size=ALIGN(fw_dump.boot_memory_size,align);}
@@ -472,8 +474,43 @@ int __init fadump_reserve_mem(void)pr_err("Too many holes in boot memory area to enable fadump\n");gotoerror_out;}++}++returnbootmem_min;+error_out:+fw_dump.fadump_enabled=0;+return0;+}++int__initfadump_adjust_memlimit(void)+{+u64size,bootmem_min;++if(!fw_dump.fadump_enabled)+return0;++if(!fw_dump.fadump_supported){+pr_info("Firmware-Assisted Dump is not supported on this hardware\n");+fw_dump.fadump_enabled=0;+return0;}+#ifdef CONFIG_HUGETLB_PAGE+if(fw_dump.dump_active){+/*+*FADumpcapturekerneldoesn'tcaremuchabouthugepages.+*Infact,handlinghugepagesincapturekernelisaskingfor+*trouble.So,disableHugeTLBsupportwhenfadumpisactive.+*/+hugetlb_disabled=true;+}+#endif++bootmem_min=fadump_get_bootmem_min();+if(!bootmem_min)+return0;+/**Calculatethememoryboundary.*Ifmemory_limitislessthanactualmemoryboundarythenreserve
@@ -490,6 +527,19 @@ int __init fadump_reserve_mem(void)printk(KERN_INFO"Adjusted memory_limit for firmware-assisted"" dump, now %#016llx\n",memory_limit);}++return0;+}++int__initfadump_reserve_mem(void)+{+u64base,size,mem_boundary,align=fadump_get_reserve_alignment();+boolis_memblock_bottom_up=memblock_bottom_up();+intret=1;++if(!fw_dump.fadump_enabled)+return0;+if(memory_limit)mem_boundary=memory_limit;else
@@ -501,14 +551,6 @@ int __init fadump_reserve_mem(void)if(fw_dump.dump_active){pr_info("Firmware-assisted dump is active.\n");-#ifdef CONFIG_HUGETLB_PAGE-/*-*FADumpcapturekerneldoesn'tcaremuchabouthugepages.-*Infact,handlinghugepagesincapturekernelisaskingfor-*trouble.So,disableHugeTLBsupportwhenfadumpisactive.-*/-hugetlb_disabled=true;-#endif/**Iflastboothascrashedthenreserveallthememory*abovebootmemorysizesothatwedon'ttouchituntil
From: Michal Suchánek <hidden> Date: 2020-02-18 16:36:55
On Fri, Jun 28, 2019 at 12:51:19AM +0530, Hari Bathini wrote:
Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.
The adjustment of memory limit does not look quite sound
- There is no code to undo the adjustment in case reservation fails
- I don't think reservation is still forced to the end of memory
causing the kernel to use memory it was supposed not to
- The CMA reservation again causes teh reserved memory to be used
- Finally the CMA reservation makes this obsolete because the reserved
memory is can be used by the system
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
@@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)#endif}-/*-*Calculatethememoryboundary.-*Ifmemory_limitislessthanactualmemoryboundarythenreserve-*thememoryforfadumpbeyondthememory_limitandadjustthe-*memory_limitaccordingly,sothattherunningkernelcanrunwith-*specifiedmemory_limit.-*/-if(memory_limit&&memory_limit<memblock_end_of_DRAM()){-size=get_fadump_area_size();-if((memory_limit+size)<memblock_end_of_DRAM())-memory_limit+=size;-else-memory_limit=memblock_end_of_DRAM();-printk(KERN_INFO"Adjusted memory_limit for firmware-assisted"-" dump, now %#016llx\n",memory_limit);-}if(memory_limit)memory_boundary=memory_limit;else
@@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)crashk_res.end=crash_base+crash_size-1;}-if(crashk_res.end==crashk_res.start){-crashk_res.start=crashk_res.end=0;-return;-}+if(crashk_res.end==crashk_res.start)+gotoerror_out;/* We might have got these values via the command line or the*devicetree,eitherwaysanitisethemnow.*/
@@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)if(overlaps_crashkernel(__pa(_stext),_end-_stext)){printk(KERN_WARNING"Crash kernel can not overlap current kernel\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}/* Crash kernel trumps memory limit */if(memory_limit&&memory_limit<=crashk_res.end){-memory_limit=crashk_res.end+1;-printk("Adjusted memory limit for crashkernel, now 0x%llx\n",-memory_limit);+pr_err("Crash kernel size can't exceed memory_limit\n");+gotoerror_out;}printk(KERN_INFO"Reserving %ldMB of memory at %ldMB "
@@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)if(!memblock_is_region_memory(crashk_res.start,crash_size)||memblock_reserve(crashk_res.start,crash_size)){pr_err("Failed to reserve memory for crashkernel!\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}++return;+error_out:+crashk_res.start=crashk_res.end=0;+return;}intoverlaps_crashkernel(unsignedlongstart,unsignedlongsize)
From: Michal Suchanek <hidden> Date: 2020-02-18 16:32:52
From: Hari Bathini <hbathini@linux.ibm.com>
Sometimes, memory reservation for KDump/FADump can overlap with memory
marked for hugepages. This overlap leads to error, hang in KDump case
and copy error reported by f/w in case of FADump, while trying to
capture dump. Report error while setting up memory for the capture
kernel instead of running into issues while capturing dump, by moving
KDump/FADump reservation below MMU early init and failing gracefully
when hugepages memory overlaps with capture kernel memory.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
arch/powerpc/kernel/prom.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
From: Michal Suchanek <hidden> Date: 2020-02-18 16:34:46
From: Hari Bathini <hbathini@linux.ibm.com>
Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Michal Suchanek <redacted>
---
arch/powerpc/kernel/fadump.c | 16 ----------------
arch/powerpc/kexec/core.c | 22 +++++++++++-----------
2 files changed, 11 insertions(+), 27 deletions(-)
@@ -472,22 +472,6 @@ int __init fadump_reserve_mem(void)}}-/*-*Calculatethememoryboundary.-*Ifmemory_limitislessthanactualmemoryboundarythenreserve-*thememoryforfadumpbeyondthememory_limitandadjustthe-*memory_limitaccordingly,sothattherunningkernelcanrunwith-*specifiedmemory_limit.-*/-if(memory_limit&&memory_limit<memblock_end_of_DRAM()){-size=get_fadump_area_size();-if((memory_limit+size)<memblock_end_of_DRAM())-memory_limit+=size;-else-memory_limit=memblock_end_of_DRAM();-printk(KERN_INFO"Adjusted memory_limit for firmware-assisted"-" dump, now %#016llx\n",memory_limit);-}if(memory_limit)mem_boundary=memory_limit;else
@@ -126,10 +126,8 @@ void __init reserve_crashkernel(void)crashk_res.end=crash_base+crash_size-1;}-if(crashk_res.end==crashk_res.start){-crashk_res.start=crashk_res.end=0;-return;-}+if(crashk_res.end==crashk_res.start)+gotoerror_out;/* We might have got these values via the command line or the*devicetree,eitherwaysanitisethemnow.*/
@@ -171,15 +169,13 @@ void __init reserve_crashkernel(void)if(overlaps_crashkernel(__pa(_stext),_end-_stext)){printk(KERN_WARNING"Crash kernel can not overlap current kernel\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}/* Crash kernel trumps memory limit */if(memory_limit&&memory_limit<=crashk_res.end){-memory_limit=crashk_res.end+1;-printk("Adjusted memory limit for crashkernel, now 0x%llx\n",-memory_limit);+pr_err("Crash kernel size can't exceed memory_limit\n");+gotoerror_out;}printk(KERN_INFO"Reserving %ldMB of memory at %ldMB "
@@ -191,9 +187,13 @@ void __init reserve_crashkernel(void)if(!memblock_is_region_memory(crashk_res.start,crash_size)||memblock_reserve(crashk_res.start,crash_size)){pr_err("Failed to reserve memory for crashkernel!\n");-crashk_res.start=crashk_res.end=0;-return;+gotoerror_out;}++return;+error_out:+crashk_res.start=crashk_res.end=0;+return;}intoverlaps_crashkernel(unsignedlongstart,unsignedlongsize)
From: Michal Suchánek <hidden> Date: 2020-03-05 18:44:47
Hello,
This seems to cause crash with kdump reservation 1GB quite reliably.
Thanks
Michal
On Tue, Feb 18, 2020 at 05:28:34PM +0100, Michal Suchanek wrote:
quoted hunk
From: Hari Bathini <hbathini@linux.ibm.com>
Sometimes, memory reservation for KDump/FADump can overlap with memory
marked for hugepages. This overlap leads to error, hang in KDump case
and copy error reported by f/w in case of FADump, while trying to
capture dump. Report error while setting up memory for the capture
kernel instead of running into issues while capturing dump, by moving
KDump/FADump reservation below MMU early init and failing gracefully
when hugepages memory overlaps with capture kernel memory.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
arch/powerpc/kernel/prom.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
Sometimes, memory reservation for KDump/FADump can overlap with memory
marked for hugepages. This overlap leads to error, hang in KDump case
and copy error reported by f/w in case of FADump, while trying to
capture dump. Report error while setting up memory for the capture
kernel instead of running into issues while capturing dump, by moving
KDump/FADump reservation below MMU early init and failing gracefully
when hugepages memory overlaps with capture kernel memory.
This patch doesn't apply, if it's still needed can you please rebase ?
Christophe