[PATCH v2 1/2] powerpc/fadump: exclude memory holes while reserving memory in second kernel

[PATCH v2 1/2] powerpc/fadump: exclude memory holes while reserving memory in second kernel

[Hari Bathini]

From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

The second kernel, during early boot after the crash, reserves rest of
the memory above boot memory size to make sure it does not touch any of the
dump memory area. It uses memblock_reserve() that reserves the specified
memory region irrespective of memory holes present within that region.
There are chances where previous kernel would have hot removed some of
its memory leaving memory holes behind. In such cases fadump kernel reports
incorrect number of reserved pages through arch_reserved_kernel_pages()
hook causing kernel to hang or panic.

Fix this by excluding memory holes while reserving rest of the memory
above boot memory size during second kernel boot after crash.

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---

Changes in v2:
* Split crash dump memory reservation into a separate function.



 arch/powerpc/kernel/fadump.c |   29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 3c2c268..bea8d5f 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -335,6 +335,26 @@ static unsigned long get_fadump_area_size(void)
 	return size;
 }
 
+static void __init fadump_reserve_crash_area(unsigned long base,
+					     unsigned long size)
+{
+	struct memblock_region *reg;
+	unsigned long mstart, mend, msize;
+
+	for_each_memblock(memory, reg) {
+		mstart = max_t(unsigned long, base, reg->base);
+		mend = reg->base + reg->size;
+		mend = min(base + size, mend);
+
+		if (mstart < mend) {
+			msize = mend - mstart;
+			memblock_reserve(mstart, msize);
+			pr_info("Reserved %ldMB of memory at %#016lx for saving crash dump\n",
+				(msize >> 20), mstart);
+		}
+	}
+}
+
 int __init fadump_reserve_mem(void)
 {
 	unsigned long base, size, memory_boundary;
@@ -380,7 +400,8 @@ int __init fadump_reserve_mem(void)
 		memory_boundary = memblock_end_of_DRAM();
 
 	if (fw_dump.dump_active) {
-		printk(KERN_INFO "Firmware-assisted dump is active.\n");
+		pr_info("Firmware-assisted dump is active.\n");
+
 		/*
 		 * If last boot has crashed then reserve all the memory
 		 * above boot_memory_size so that we don't touch it until
@@ -389,11 +410,7 @@ int __init fadump_reserve_mem(void)
 		 */
 		base = fw_dump.boot_memory_size;
 		size = memory_boundary - base;
-		memblock_reserve(base, size);
-		printk(KERN_INFO "Reserved %ldMB of memory at %ldMB "
-				"for saving crash dump\n",
-				(unsigned long)(size >> 20),
-				(unsigned long)(base >> 20));
+		fadump_reserve_crash_area(base, size);
 
 		fw_dump.fadumphdr_addr =
 				be64_to_cpu(fdm_active->rmr_region.destination_address) +

[PATCH v2 2/2] powerpc/fadump: Do not use hugepages when fadump is active

[Hari Bathini]

FADump capture kernel boots in restricted memory environment preserving
the context of previous kernel to save vmcore. Supporting hugepages in
such environment makes things unnecessarily complicated, as hugepages
need memory set aside for them. This means most of the capture kernel's
memory is used in supporting hugepages. In most cases, this results in
out-of-memory issues while booting FADump capture kernel. But hugepages
are not of much use in capture kernel whose only job is to save vmcore.
So, disabling hugepages support, when fadump is active, is a reliable
solution for the out of memory issues. Introducing a flag variable to
disable HugeTLB support when fadump is active.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---

Changes in v2:
* Introduce a hugetlb_disabled flag to enable/disable hugepage support &
  use that flag to disable hugepage support when fadump is active.


 arch/powerpc/include/asm/page.h |    1 +
 arch/powerpc/kernel/fadump.c    |    8 ++++++++
 arch/powerpc/mm/hash_utils_64.c |    6 ++++--
 arch/powerpc/mm/hugetlbpage.c   |    7 +++++++
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 8da5d4c..40aee93 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -39,6 +39,7 @@
 
 #ifndef __ASSEMBLY__
 #ifdef CONFIG_HUGETLB_PAGE
+extern bool hugetlb_disabled;
 extern unsigned int HPAGE_SHIFT;
 #else
 #define HPAGE_SHIFT PAGE_SHIFT
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index bea8d5f..8ceabef4 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -402,6 +402,14 @@ int __init fadump_reserve_mem(void)
 	if (fw_dump.dump_active) {
 		pr_info("Firmware-assisted dump is active.\n");
 
+#ifdef CONFIG_HUGETLB_PAGE
+		/*
+		 * FADump capture kernel doesn't care much about hugepages.
+		 * In fact, handling hugepages in capture kernel is asking for
+		 * trouble. So, disable HugeTLB support when fadump is active.
+		 */
+		hugetlb_disabled = true;
+#endif
 		/*
 		 * If last boot has crashed then reserve all the memory
 		 * above boot_memory_size so that we don't touch it until
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index cf290d41..eab8f1d 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -571,8 +571,10 @@ static void __init htab_scan_page_sizes(void)
 	}
 
 #ifdef CONFIG_HUGETLB_PAGE
-	/* Reserve 16G huge page memory sections for huge pages */
-	of_scan_flat_dt(htab_dt_scan_hugepage_blocks, NULL);
+	if (!hugetlb_disabled) {
+		/* Reserve 16G huge page memory sections for huge pages */
+		of_scan_flat_dt(htab_dt_scan_hugepage_blocks, NULL);
+	}
 #endif /* CONFIG_HUGETLB_PAGE */
 }
 
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 876da2b..18c080a 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -35,6 +35,8 @@
 #define PAGE_SHIFT_16M	24
 #define PAGE_SHIFT_16G	34
 
+bool hugetlb_disabled = false;
+
 unsigned int HPAGE_SHIFT;
 EXPORT_SYMBOL(HPAGE_SHIFT);
 
@@ -653,6 +655,11 @@ static int __init hugetlbpage_init(void)
 {
 	int psize;
 
+	if (hugetlb_disabled) {
+		pr_info("HugeTLB support is disabled!\n");
+		return 0;
+	}
+
 #if !defined(CONFIG_PPC_FSL_BOOK3E) && !defined(CONFIG_PPC_8xx)
 	if (!radix_enabled() && !mmu_has_feature(MMU_FTR_16M_PAGE))
 		return -ENODEV;

Re: [PATCH v2 2/2] powerpc/fadump: Do not use hugepages when fadump is active

[Mahesh Jagannath Salgaonkar]

On 04/10/2018 07:11 PM, Hari Bathini wrote:
> FADump capture kernel boots in restricted memory environment preserving
> the context of previous kernel to save vmcore. Supporting hugepages in
> such environment makes things unnecessarily complicated, as hugepages
> need memory set aside for them. This means most of the capture kernel's
> memory is used in supporting hugepages. In most cases, this results in
> out-of-memory issues while booting FADump capture kernel. But hugepages
> are not of much use in capture kernel whose only job is to save vmcore.
> So, disabling hugepages support, when fadump is active, is a reliable
> solution for the out of memory issues. Introducing a flag variable to
> disable HugeTLB support when fadump is active.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> ---
> 
> Changes in v2:
> * Introduce a hugetlb_disabled flag to enable/disable hugepage support &
>   use that flag to disable hugepage support when fadump is active.

Looks good to me.

Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

> 
> 
>  arch/powerpc/include/asm/page.h |    1 +
>  arch/powerpc/kernel/fadump.c    |    8 ++++++++
>  arch/powerpc/mm/hash_utils_64.c |    6 ++++--
>  arch/powerpc/mm/hugetlbpage.c   |    7 +++++++
>  4 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 8da5d4c..40aee93 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -39,6 +39,7 @@
> 
>  #ifndef __ASSEMBLY__
>  #ifdef CONFIG_HUGETLB_PAGE
> +extern bool hugetlb_disabled;
>  extern unsigned int HPAGE_SHIFT;
>  #else
>  #define HPAGE_SHIFT PAGE_SHIFT
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index bea8d5f..8ceabef4 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -402,6 +402,14 @@ int __init fadump_reserve_mem(void)
>  	if (fw_dump.dump_active) {
>  		pr_info("Firmware-assisted dump is active.\n");
> 
> +#ifdef CONFIG_HUGETLB_PAGE
> +		/*
> +		 * FADump capture kernel doesn't care much about hugepages.
> +		 * In fact, handling hugepages in capture kernel is asking for
> +		 * trouble. So, disable HugeTLB support when fadump is active.
> +		 */
> +		hugetlb_disabled = true;
> +#endif
>  		/*
>  		 * If last boot has crashed then reserve all the memory
>  		 * above boot_memory_size so that we don't touch it until
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index cf290d41..eab8f1d 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -571,8 +571,10 @@ static void __init htab_scan_page_sizes(void)
>  	}
> 
>  #ifdef CONFIG_HUGETLB_PAGE
> -	/* Reserve 16G huge page memory sections for huge pages */
> -	of_scan_flat_dt(htab_dt_scan_hugepage_blocks, NULL);
> +	if (!hugetlb_disabled) {
> +		/* Reserve 16G huge page memory sections for huge pages */
> +		of_scan_flat_dt(htab_dt_scan_hugepage_blocks, NULL);
> +	}
>  #endif /* CONFIG_HUGETLB_PAGE */
>  }
> 
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 876da2b..18c080a 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -35,6 +35,8 @@
>  #define PAGE_SHIFT_16M	24
>  #define PAGE_SHIFT_16G	34
> 
> +bool hugetlb_disabled = false;
> +
>  unsigned int HPAGE_SHIFT;
>  EXPORT_SYMBOL(HPAGE_SHIFT);
> 
> @@ -653,6 +655,11 @@ static int __init hugetlbpage_init(void)
>  {
>  	int psize;
> 
> +	if (hugetlb_disabled) {
> +		pr_info("HugeTLB support is disabled!\n");
> +		return 0;
> +	}
> +
>  #if !defined(CONFIG_PPC_FSL_BOOK3E) && !defined(CONFIG_PPC_8xx)
>  	if (!radix_enabled() && !mmu_has_feature(MMU_FTR_16M_PAGE))
>  		return -ENODEV;
> 

Re: [v2, 1/2] powerpc/fadump: exclude memory holes while reserving memory in second kernel

[Michael Ellerman]

On Tue, 2018-04-10 at 13:41:16 UTC, Hari Bathini wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> The second kernel, during early boot after the crash, reserves rest of
> the memory above boot memory size to make sure it does not touch any of the
> dump memory area. It uses memblock_reserve() that reserves the specified
> memory region irrespective of memory holes present within that region.
> There are chances where previous kernel would have hot removed some of
> its memory leaving memory holes behind. In such cases fadump kernel reports
> incorrect number of reserved pages through arch_reserved_kernel_pages()
> hook causing kernel to hang or panic.
> 
> Fix this by excluding memory holes while reserving rest of the memory
> above boot memory size during second kernel boot after crash.
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/b71a693d3db3abd1ddf7d29be967a1

cheers