Re: [PATCH] powerpc/book3s64/radix: Fix boot failure with large amount of guest memory
Hi "Aneesh, I love your patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on v5.9-rc1 next-20200818] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Aneesh-Kumar-K-V/powerpc-book3s64-radix-Fix-boot-failure-with-large-amount-of-guest-memory/20200814-002215 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-allnoconfig (attached as .config) compiler: powerpc-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): arch/powerpc/kernel/prom.c: In function 'early_init_devtree': >> arch/powerpc/kernel/prom.c:818:3: error: implicit declaration of function >> 'radix__setup_initial_memory_limit'; did you mean >> 'setup_initial_memory_limit'? [-Werror=implicit-function-declaration] 818 | radix__setup_initial_memory_limit(memstart_addr, first_memblock_size); | ^ | setup_initial_memory_limit cc1: all warnings being treated as errors # https://github.com/0day-ci/linux/commit/082f192bfaabd1eeb28421d82574ce76ae0c4fba git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Aneesh-Kumar-K-V/powerpc-book3s64-radix-Fix-boot-failure-with-large-amount-of-guest-memory/20200814-002215 git checkout 082f192bfaabd1eeb28421d82574ce76ae0c4fba vim +818 arch/powerpc/kernel/prom.c 811 812 mmu_early_init_devtree(); 813 814 /* 815 * Reset ppc64_rma_size and memblock memory limit 816 */ 817 if (early_radix_enabled()) > 818 radix__setup_initial_memory_limit(memstart_addr, > first_memblock_size); 819 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH] powerpc/book3s64/radix: Fix boot failure with large amount of guest memory
On 8/17/20 9:13 PM, Hari Bathini wrote: On 13/08/20 9:50 pm, Aneesh Kumar K.V wrote: If the hypervisor doesn't support hugepages, the kernel ends up allocating a large number of page table pages. The early page table allocation was wrongly setting the max memblock limit to ppc64_rma_size with radix translation which resulted in boot failure as shown below. Kernel panic - not syncing: early_alloc_pgtable: Failed to allocate 16777216 bytes align=0x100 nid=-1 from=0x max_addr=0x CPU: 0 PID: 0 Comm: swapper Not tainted 5.8.0-24.9-default+ #2 Call Trace: [c16f3d00] [c07c6470] dump_stack+0xc4/0x114 (unreliable) [c16f3d40] [c014c78c] panic+0x164/0x418 [c16f3dd0] [c0098890] early_alloc_pgtable+0xe0/0xec [c16f3e60] [c10a5440] radix__early_init_mmu+0x360/0x4b4 [c16f3ef0] [c1099bac] early_init_mmu+0x1c/0x3c [c16f3f10] [c109a320] early_setup+0x134/0x170 This was because the kernel was checking for the radix feature before we enable the feature via mmu_features. This resulted in the kernel using hash restrictions on radix. Rework the early init code such that the kernel boot with memblock restrictions as imposed by hash. At that point, the kernel still hasn't finalized the translation the kernel will end up using. We have three different ways of detecting radix. 1. dt_cpu_ftrs_scan -> used only in case of PowerNV 2. ibm,pa-features -> Used when we don't use cpu_dt_ftr_scan 3. CAS -> Where we negotiate with hypervisor about the supported translation. We look at 1 or 2 early in the boot and after that, we look at the CAS vector to finalize the translation the kernel will use. We also support a kernel command line option (disable_radix) to switch to hash. Update the memblock limit after mmu_early_init_devtree() if the kernel is going to use radix translation. This forces some of the memblock allocations we do before mmu_early_init_devtree() to be within the RMA limit. Minor comments below. Nonetheless... Reviewed-by: Hari Bathini Fixes: 2bfd65e45e87 ("powerpc/mm/radix: Add radix callbacks for early init routines") Reported-by: Shirisha Ganta Signed-off-by: Aneesh Kumar K.V --- arch/powerpc/include/asm/book3s/64/mmu.h | 8 +--- arch/powerpc/kernel/prom.c | 6 ++ arch/powerpc/mm/book3s64/radix_pgtable.c | 2 ++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h index 55442d45c597..4245f99453f5 100644 --- a/arch/powerpc/include/asm/book3s/64/mmu.h +++ b/arch/powerpc/include/asm/book3s/64/mmu.h @@ -244,9 +244,11 @@ extern void radix__setup_initial_memory_limit(phys_addr_t first_memblock_base, static inline void setup_initial_memory_limit(phys_addr_t first_memblock_base, phys_addr_t first_memblock_size) { - if (early_radix_enabled()) - return radix__setup_initial_memory_limit(first_memblock_base, - first_memblock_size); + /* + * Hash has more strict restrictions. At this point we don't + * know which translations we will pick. Hence got with hash :s/got with/go with/ + * restrictions. + */ return hash__setup_initial_memory_limit(first_memblock_base, first_memblock_size); } diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index d8a2fb87ba0c..340900ae95a4 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -811,6 +811,12 @@ void __init early_init_devtree(void *params) mmu_early_init_devtree(); + /* + * Reset ppc64_rma_size and memblock memory limit + */ + if (early_radix_enabled()) + radix__setup_initial_memory_limit(memstart_addr, first_memblock_size); + #ifdef CONFIG_PPC_POWERNV /* Scan and build the list of machine check recoverable ranges */ of_scan_flat_dt(early_init_dt_scan_recoverable_ranges, NULL); diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c index 28c784976bed..094daf16acac 100644 --- a/arch/powerpc/mm/book3s64/radix_pgtable.c +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c @@ -747,6 +747,8 @@ void radix__setup_initial_memory_limit(phys_addr_t first_memblock_base, * Radix mode is not limited by RMA / VRMA addressing. */ ppc64_rma_size = ULONG_MAX; + + memblock_set_current_limit(MEMBLOCK_ALLOC_ANYWHERE); Probably the same thing but I would prefer the below instead: memblock_set_current_limit(ppc64_rma_size); This is not really related to ppc64_rma_size right? On radix what we actually want is memblock alloc from anywhere. Actually what we want is memblock_set_current_limit(memblock_limit_from_rma(ppc64_rma_size)) But that is unnecessary complication? -aneesh
Re: [PATCH] powerpc/book3s64/radix: Fix boot failure with large amount of guest memory
On 13/08/20 9:50 pm, Aneesh Kumar K.V wrote: If the hypervisor doesn't support hugepages, the kernel ends up allocating a large number of page table pages. The early page table allocation was wrongly setting the max memblock limit to ppc64_rma_size with radix translation which resulted in boot failure as shown below. Kernel panic - not syncing: early_alloc_pgtable: Failed to allocate 16777216 bytes align=0x100 nid=-1 from=0x max_addr=0x CPU: 0 PID: 0 Comm: swapper Not tainted 5.8.0-24.9-default+ #2 Call Trace: [c16f3d00] [c07c6470] dump_stack+0xc4/0x114 (unreliable) [c16f3d40] [c014c78c] panic+0x164/0x418 [c16f3dd0] [c0098890] early_alloc_pgtable+0xe0/0xec [c16f3e60] [c10a5440] radix__early_init_mmu+0x360/0x4b4 [c16f3ef0] [c1099bac] early_init_mmu+0x1c/0x3c [c16f3f10] [c109a320] early_setup+0x134/0x170 This was because the kernel was checking for the radix feature before we enable the feature via mmu_features. This resulted in the kernel using hash restrictions on radix. Rework the early init code such that the kernel boot with memblock restrictions as imposed by hash. At that point, the kernel still hasn't finalized the translation the kernel will end up using. We have three different ways of detecting radix. 1. dt_cpu_ftrs_scan -> used only in case of PowerNV 2. ibm,pa-features -> Used when we don't use cpu_dt_ftr_scan 3. CAS -> Where we negotiate with hypervisor about the supported translation. We look at 1 or 2 early in the boot and after that, we look at the CAS vector to finalize the translation the kernel will use. We also support a kernel command line option (disable_radix) to switch to hash. Update the memblock limit after mmu_early_init_devtree() if the kernel is going to use radix translation. This forces some of the memblock allocations we do before mmu_early_init_devtree() to be within the RMA limit. Minor comments below. Nonetheless... Reviewed-by: Hari Bathini Fixes: 2bfd65e45e87 ("powerpc/mm/radix: Add radix callbacks for early init routines") Reported-by: Shirisha Ganta Signed-off-by: Aneesh Kumar K.V --- arch/powerpc/include/asm/book3s/64/mmu.h | 8 +--- arch/powerpc/kernel/prom.c | 6 ++ arch/powerpc/mm/book3s64/radix_pgtable.c | 2 ++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h index 55442d45c597..4245f99453f5 100644 --- a/arch/powerpc/include/asm/book3s/64/mmu.h +++ b/arch/powerpc/include/asm/book3s/64/mmu.h @@ -244,9 +244,11 @@ extern void radix__setup_initial_memory_limit(phys_addr_t first_memblock_base, static inline void setup_initial_memory_limit(phys_addr_t first_memblock_base, phys_addr_t first_memblock_size) { - if (early_radix_enabled()) - return radix__setup_initial_memory_limit(first_memblock_base, - first_memblock_size); + /* +* Hash has more strict restrictions. At this point we don't +* know which translations we will pick. Hence got with hash :s/got with/go with/ +* restrictions. +*/ return hash__setup_initial_memory_limit(first_memblock_base, first_memblock_size); } diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index d8a2fb87ba0c..340900ae95a4 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -811,6 +811,12 @@ void __init early_init_devtree(void *params) mmu_early_init_devtree(); + /* +* Reset ppc64_rma_size and memblock memory limit +*/ + if (early_radix_enabled()) + radix__setup_initial_memory_limit(memstart_addr, first_memblock_size); + #ifdef CONFIG_PPC_POWERNV /* Scan and build the list of machine check recoverable ranges */ of_scan_flat_dt(early_init_dt_scan_recoverable_ranges, NULL); diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c index 28c784976bed..094daf16acac 100644 --- a/arch/powerpc/mm/book3s64/radix_pgtable.c +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c @@ -747,6 +747,8 @@ void radix__setup_initial_memory_limit(phys_addr_t first_memblock_base, * Radix mode is not limited by RMA / VRMA addressing. */ ppc64_rma_size = ULONG_MAX; + + memblock_set_current_limit(MEMBLOCK_ALLOC_ANYWHERE); Probably the same thing but I would prefer the below instead: memblock_set_current_limit(ppc64_rma_size); Thanks Hari
[PATCH] powerpc/book3s64/radix: Fix boot failure with large amount of guest memory
If the hypervisor doesn't support hugepages, the kernel ends up allocating a large number of page table pages. The early page table allocation was wrongly setting the max memblock limit to ppc64_rma_size with radix translation which resulted in boot failure as shown below. Kernel panic - not syncing: early_alloc_pgtable: Failed to allocate 16777216 bytes align=0x100 nid=-1 from=0x max_addr=0x CPU: 0 PID: 0 Comm: swapper Not tainted 5.8.0-24.9-default+ #2 Call Trace: [c16f3d00] [c07c6470] dump_stack+0xc4/0x114 (unreliable) [c16f3d40] [c014c78c] panic+0x164/0x418 [c16f3dd0] [c0098890] early_alloc_pgtable+0xe0/0xec [c16f3e60] [c10a5440] radix__early_init_mmu+0x360/0x4b4 [c16f3ef0] [c1099bac] early_init_mmu+0x1c/0x3c [c16f3f10] [c109a320] early_setup+0x134/0x170 This was because the kernel was checking for the radix feature before we enable the feature via mmu_features. This resulted in the kernel using hash restrictions on radix. Rework the early init code such that the kernel boot with memblock restrictions as imposed by hash. At that point, the kernel still hasn't finalized the translation the kernel will end up using. We have three different ways of detecting radix. 1. dt_cpu_ftrs_scan -> used only in case of PowerNV 2. ibm,pa-features -> Used when we don't use cpu_dt_ftr_scan 3. CAS -> Where we negotiate with hypervisor about the supported translation. We look at 1 or 2 early in the boot and after that, we look at the CAS vector to finalize the translation the kernel will use. We also support a kernel command line option (disable_radix) to switch to hash. Update the memblock limit after mmu_early_init_devtree() if the kernel is going to use radix translation. This forces some of the memblock allocations we do before mmu_early_init_devtree() to be within the RMA limit. Fixes: 2bfd65e45e87 ("powerpc/mm/radix: Add radix callbacks for early init routines") Reported-by: Shirisha Ganta Signed-off-by: Aneesh Kumar K.V --- arch/powerpc/include/asm/book3s/64/mmu.h | 8 +--- arch/powerpc/kernel/prom.c | 6 ++ arch/powerpc/mm/book3s64/radix_pgtable.c | 2 ++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h index 55442d45c597..4245f99453f5 100644 --- a/arch/powerpc/include/asm/book3s/64/mmu.h +++ b/arch/powerpc/include/asm/book3s/64/mmu.h @@ -244,9 +244,11 @@ extern void radix__setup_initial_memory_limit(phys_addr_t first_memblock_base, static inline void setup_initial_memory_limit(phys_addr_t first_memblock_base, phys_addr_t first_memblock_size) { - if (early_radix_enabled()) - return radix__setup_initial_memory_limit(first_memblock_base, - first_memblock_size); + /* +* Hash has more strict restrictions. At this point we don't +* know which translations we will pick. Hence got with hash +* restrictions. +*/ return hash__setup_initial_memory_limit(first_memblock_base, first_memblock_size); } diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index d8a2fb87ba0c..340900ae95a4 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -811,6 +811,12 @@ void __init early_init_devtree(void *params) mmu_early_init_devtree(); + /* +* Reset ppc64_rma_size and memblock memory limit +*/ + if (early_radix_enabled()) + radix__setup_initial_memory_limit(memstart_addr, first_memblock_size); + #ifdef CONFIG_PPC_POWERNV /* Scan and build the list of machine check recoverable ranges */ of_scan_flat_dt(early_init_dt_scan_recoverable_ranges, NULL); diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c index 28c784976bed..094daf16acac 100644 --- a/arch/powerpc/mm/book3s64/radix_pgtable.c +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c @@ -747,6 +747,8 @@ void radix__setup_initial_memory_limit(phys_addr_t first_memblock_base, * Radix mode is not limited by RMA / VRMA addressing. */ ppc64_rma_size = ULONG_MAX; + + memblock_set_current_limit(MEMBLOCK_ALLOC_ANYWHERE); } #ifdef CONFIG_MEMORY_HOTPLUG -- 2.26.2