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

[Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com>]

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;
>