| Message ID | 20190317011756.abjylw3nzfnc6gnh@decadent.org.uk (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8bc086899816214fbc6047c9c7e15fcab49552bf |
| Headers | show |
| Series | powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (de3c83c2fd2b87cf68214eda76dfa66989d78cb6) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
On Mär 22 2019, Michael Ellerman <patch-notifications@ellerman.id.au> wrote: > On Sun, 2019-03-17 at 01:17:56 UTC, Ben Hutchings wrote: >> MAX_PHYSMEM_BITS only needs to be defined if CONFIG_SPARSEMEM is >> enabled, and that was the case before commit 4ffe713b7587 >> ("powerpc/mm: Increase the max addressable memory to 2PB"). >> >> On 32-bit systems, where CONFIG_SPARSEMEM is not enabled, we now >> define it as 46. That is larger than the real number of physical >> address bits, and breaks calculations in zsmalloc: >> >> mm/zsmalloc.c:130:49: warning: right shift count is negative [-Wshift-count-negative] >> MAX(32, (ZS_MAX_PAGES_PER_ZSPAGE << PAGE_SHIFT >> OBJ_INDEX_BITS)) >> ^~ >> ... >> mm/zsmalloc.c:253:21: error: variably modified 'size_class' at file scope >> struct size_class *size_class[ZS_SIZE_CLASSES]; >> ^~~~~~~~~~ >> >> Fixes: 4ffe713b7587 ("powerpc/mm: Increase the max addressable memory to 2PB") >> Cc: stable@vger.kernel.org >> Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > > Applied to powerpc fixes, thanks. > > https://git.kernel.org/powerpc/c/8bc086899816214fbc6047c9c7e15fca In file included from ./arch/powerpc/include/asm/book3s/64/mmu.h:39:0, from ./arch/powerpc/include/asm/mmu.h:360, from ./arch/powerpc/include/asm/lppaca.h:36, from ./arch/powerpc/include/asm/paca.h:21, from ./arch/powerpc/include/asm/current.h:16, from ./include/linux/thread_info.h:21, from ./include/asm-generic/preempt.h:5, from ./arch/powerpc/include/generated/asm/preempt.h:1, from ./include/linux/preempt.h:78, from ./include/linux/spinlock.h:51, from ./include/linux/mmzone.h:8, from ./include/linux/gfp.h:6, from ./include/linux/slab.h:15, from ./include/linux/crypto.h:24, from ./include/crypto/algapi.h:15, from ./include/crypto/internal/hash.h:16, from arch/powerpc/crypto/md5-glue.c:15: ./arch/powerpc/include/asm/book3s/64/mmu-hash.h:584:6: error: "MAX_PHYSMEM_BITS" is not defined [-Werror=undef] #if (MAX_PHYSMEM_BITS > MAX_EA_BITS_PER_CONTEXT) ^ Andreas.
On Sun, 2019-03-24 at 23:18 +0100, Andreas Schwab wrote: > On Mär 22 2019, Michael Ellerman <patch-notifications@ellerman.id.au> wrote: > > > On Sun, 2019-03-17 at 01:17:56 UTC, Ben Hutchings wrote: > > > MAX_PHYSMEM_BITS only needs to be defined if CONFIG_SPARSEMEM is > > > enabled, and that was the case before commit 4ffe713b7587 > > > ("powerpc/mm: Increase the max addressable memory to 2PB"). > > > > > > On 32-bit systems, where CONFIG_SPARSEMEM is not enabled, we now > > > define it as 46. That is larger than the real number of physical > > > address bits, and breaks calculations in zsmalloc: > > > > > > mm/zsmalloc.c:130:49: warning: right shift count is negative [-Wshift-count-negative] > > > MAX(32, (ZS_MAX_PAGES_PER_ZSPAGE << PAGE_SHIFT >> OBJ_INDEX_BITS)) > > > ^~ > > > ... > > > mm/zsmalloc.c:253:21: error: variably modified 'size_class' at file scope > > > struct size_class *size_class[ZS_SIZE_CLASSES]; > > > ^~~~~~~~~~ > > > > > > Fixes: 4ffe713b7587 ("powerpc/mm: Increase the max addressable memory to 2PB") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > > > > Applied to powerpc fixes, thanks. > > > > https://git.kernel.org/powerpc/c/8bc086899816214fbc6047c9c7e15fca > > In file included from ./arch/powerpc/include/asm/book3s/64/mmu.h:39:0, > from ./arch/powerpc/include/asm/mmu.h:360, > from ./arch/powerpc/include/asm/lppaca.h:36, > from ./arch/powerpc/include/asm/paca.h:21, > from ./arch/powerpc/include/asm/current.h:16, > from ./include/linux/thread_info.h:21, > from ./include/asm-generic/preempt.h:5, > from ./arch/powerpc/include/generated/asm/preempt.h:1, > from ./include/linux/preempt.h:78, > from ./include/linux/spinlock.h:51, > from ./include/linux/mmzone.h:8, > from ./include/linux/gfp.h:6, > from ./include/linux/slab.h:15, > from ./include/linux/crypto.h:24, > from ./include/crypto/algapi.h:15, > from ./include/crypto/internal/hash.h:16, > from arch/powerpc/crypto/md5-glue.c:15: > ./arch/powerpc/include/asm/book3s/64/mmu-hash.h:584:6: error: "MAX_PHYSMEM_BITS" is not defined [-Werror=undef] > #if (MAX_PHYSMEM_BITS > MAX_EA_BITS_PER_CONTEXT) > ^ Presumably you have CONFIG_PPC_BOOK3S_64 enabled and CONFIG_SPARSEMEM disabled? Was this configuration actually usable? I think this would fix the build failure: --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h @@ -341,7 +341,7 @@ static inline u16 get_mm_addr_key(struct mm_struct *mm, unsigned long address) #if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) && \ defined (CONFIG_PPC_64K_PAGES) #define MAX_PHYSMEM_BITS 51 -#elif defined(CONFIG_SPARSEMEM) +#elif defined(CONFIG_PPC64) #define MAX_PHYSMEM_BITS 46 #endif --- END --- but whether the result will actually *work*, I don't know. Ben.
On Mär 24 2019, Ben Hutchings <ben@decadent.org.uk> wrote: > Presumably you have CONFIG_PPC_BOOK3S_64 enabled and CONFIG_SPARSEMEM > disabled? Was this configuration actually usable? Why not? Andreas.
On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote: > On Mär 24 2019, Ben Hutchings <ben@decadent.org.uk> wrote: > > > Presumably you have CONFIG_PPC_BOOK3S_64 enabled and > > CONFIG_SPARSEMEM > > disabled? Was this configuration actually usable? > > Why not? I assume that CONFIG_SPARSEMEM is the default for a good reason. What I don't know is how strong that reason is (I am not a Power expert at all). Looking a bit further, it seems to be related to CONFIG_NUMA in that you can enable CONFIG_FLATMEM if and only if that's disabled. So I suppose the configuration you used works for non-NUMA systems. Anyway, does the patch I sent work for you? Ben.
Ben Hutchings <ben@decadent.org.uk> writes: > On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote: >> On Mär 24 2019, Ben Hutchings <ben@decadent.org.uk> wrote: >> >> > Presumably you have CONFIG_PPC_BOOK3S_64 enabled and >> > CONFIG_SPARSEMEM >> > disabled? Was this configuration actually usable? >> >> Why not? > > I assume that CONFIG_SPARSEMEM is the default for a good reason. > What I don't know is how strong that reason is (I am not a Power expert > at all). Looking a bit further, it seems to be related to CONFIG_NUMA > in that you can enable CONFIG_FLATMEM if and only if that's disabled. > So I suppose the configuration you used works for non-NUMA systems. Aneesh pointed out this fix would break FLATMEM after I'd merged it, but it didn't break any of our defconfigs so I wondered if anyone would notice. I checked today and a G5 will boot with FLATMEM, which I assume is what Andreas is using. I guess we should fix this build break for now. Even some G5's have discontiguous memory, so FLATMEM is not clearly a good choice even for all G5's, and actually a fresh g5_defconfig uses SPARSEMEM. So I'm inclined to just switch to always using SPARSEMEM on 64-bit Book3S, because that's what's well tested and we hardly need more code paths to test. Unless anyone has a strong objection, I haven't actually benchmarked FLATMEM vs SPARSEMEM on a G5. cheers
Le 25/03/2019 à 12:35, Michael Ellerman a écrit : > Ben Hutchings <ben@decadent.org.uk> writes: >> On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote: >>> On Mär 24 2019, Ben Hutchings <ben@decadent.org.uk> wrote: >>> >>>> Presumably you have CONFIG_PPC_BOOK3S_64 enabled and >>>> CONFIG_SPARSEMEM >>>> disabled? Was this configuration actually usable? >>> >>> Why not? >> >> I assume that CONFIG_SPARSEMEM is the default for a good reason. >> What I don't know is how strong that reason is (I am not a Power expert >> at all). Looking a bit further, it seems to be related to CONFIG_NUMA >> in that you can enable CONFIG_FLATMEM if and only if that's disabled. >> So I suppose the configuration you used works for non-NUMA systems. > > Aneesh pointed out this fix would break FLATMEM after I'd merged it, but > it didn't break any of our defconfigs so I wondered if anyone would > notice. > > I checked today and a G5 will boot with FLATMEM, which I assume is what > Andreas is using. > > I guess we should fix this build break for now. > > Even some G5's have discontiguous memory, so FLATMEM is not clearly a > good choice even for all G5's, and actually a fresh g5_defconfig uses > SPARSEMEM. > > So I'm inclined to just switch to always using SPARSEMEM on 64-bit > Book3S, because that's what's well tested and we hardly need more code > paths to test. Unless anyone has a strong objection, I haven't actually > benchmarked FLATMEM vs SPARSEMEM on a G5. > Christian Zigotzky reported a build failure with this patch. Christophe
On Mär 25 2019, Michael Ellerman <mpe@ellerman.id.au> wrote: > So I'm inclined to just switch to always using SPARSEMEM on 64-bit > Book3S, because that's what's well tested and we hardly need more code > paths to test. Unless anyone has a strong objection, I haven't actually > benchmarked FLATMEM vs SPARSEMEM on a G5. Configuring with SPARSEMEM saves about 32Mb of memory. Andreas.
Michael Ellerman <mpe@ellerman.id.au> writes: > Ben Hutchings <ben@decadent.org.uk> writes: >> On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote: >>> On Mär 24 2019, Ben Hutchings <ben@decadent.org.uk> wrote: >>> >>> > Presumably you have CONFIG_PPC_BOOK3S_64 enabled and >>> > CONFIG_SPARSEMEM >>> > disabled? Was this configuration actually usable? >>> >>> Why not? >> >> I assume that CONFIG_SPARSEMEM is the default for a good reason. >> What I don't know is how strong that reason is (I am not a Power expert >> at all). Looking a bit further, it seems to be related to CONFIG_NUMA >> in that you can enable CONFIG_FLATMEM if and only if that's disabled. >> So I suppose the configuration you used works for non-NUMA systems. > > Aneesh pointed out this fix would break FLATMEM after I'd merged it, but > it didn't break any of our defconfigs so I wondered if anyone would > notice. > > I checked today and a G5 will boot with FLATMEM, which I assume is what > Andreas is using. > > I guess we should fix this build break for now. > > Even some G5's have discontiguous memory, so FLATMEM is not clearly a > good choice even for all G5's, and actually a fresh g5_defconfig uses > SPARSEMEM. > > So I'm inclined to just switch to always using SPARSEMEM on 64-bit > Book3S, because that's what's well tested and we hardly need more code > paths to test. Unless anyone has a strong objection, I haven't actually > benchmarked FLATMEM vs SPARSEMEM on a G5. > How about From 207fb0036065d8db44853e63bb858c4fd9952106 Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> Date: Mon, 1 Apr 2019 17:51:17 +0530 Subject: [PATCH] powerpc/mm: Fix build error The current value of MAX_PHYSMEM_BITS cannot work with 32 bit configs. We used to have MAX_PHYSMEM_BITS not defined without SPARSEMEM and 32 bit configs never expected a value to be set for MAX_PHYSMEM_BITS. Dependent code such as zsmalloc derived the right values based on other fields. Instead of finding a value that works with different configs, use new values only for book3s_64. For 64 bit booke, use the definition of MAX_PHYSMEM_BITS as per commit a7df61a0e2b6 ("[PATCH] ppc64: Increase sparsemem defaults") That change was done in 2005 and hopefully will work with book3e 64. Fixes: 4ffe713b7587 ("powerpc/mm: Increase the max addressable memory to 2PB") Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- arch/powerpc/include/asm/book3s/64/mmu.h | 15 +++++++++++++++ arch/powerpc/include/asm/mmu.h | 15 --------------- arch/powerpc/include/asm/nohash/64/mmu.h | 2 ++ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h index 1ceee000c18d..a809bdd77322 100644 --- a/arch/powerpc/include/asm/book3s/64/mmu.h +++ b/arch/powerpc/include/asm/book3s/64/mmu.h @@ -35,6 +35,21 @@ typedef pte_t *pgtable_t; #endif /* __ASSEMBLY__ */ +/* + * If we store section details in page->flags we can't increase the MAX_PHYSMEM_BITS + * if we increase SECTIONS_WIDTH we will not store node details in page->flags and + * page_to_nid does a page->section->node lookup + * Hence only increase for VMEMMAP. Further depending on SPARSEMEM_EXTREME reduce + * memory requirements with large number of sections. + * 51 bits is the max physical real address on POWER9 + */ +#if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) && \ + defined(CONFIG_PPC_64K_PAGES) +#define MAX_PHYSMEM_BITS 51 +#else +#define MAX_PHYSMEM_BITS 46 +#endif + /* 64-bit classic hash table MMU */ #include <asm/book3s/64/mmu-hash.h> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index 598cdcdd1355..78d53c4396ac 100644 --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h @@ -341,21 +341,6 @@ static inline bool strict_kernel_rwx_enabled(void) */ #define MMU_PAGE_COUNT 16 -/* - * If we store section details in page->flags we can't increase the MAX_PHYSMEM_BITS - * if we increase SECTIONS_WIDTH we will not store node details in page->flags and - * page_to_nid does a page->section->node lookup - * Hence only increase for VMEMMAP. Further depending on SPARSEMEM_EXTREME reduce - * memory requirements with large number of sections. - * 51 bits is the max physical real address on POWER9 - */ -#if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) && \ - defined (CONFIG_PPC_64K_PAGES) -#define MAX_PHYSMEM_BITS 51 -#elif defined(CONFIG_SPARSEMEM) -#define MAX_PHYSMEM_BITS 46 -#endif - #ifdef CONFIG_PPC_BOOK3S_64 #include <asm/book3s/64/mmu.h> #else /* CONFIG_PPC_BOOK3S_64 */ diff --git a/arch/powerpc/include/asm/nohash/64/mmu.h b/arch/powerpc/include/asm/nohash/64/mmu.h index e6585480dfc4..81cf30c370e5 100644 --- a/arch/powerpc/include/asm/nohash/64/mmu.h +++ b/arch/powerpc/include/asm/nohash/64/mmu.h @@ -2,6 +2,8 @@ #ifndef _ASM_POWERPC_NOHASH_64_MMU_H_ #define _ASM_POWERPC_NOHASH_64_MMU_H_ +#define MAX_PHYSMEM_BITS 44 + /* Freescale Book-E software loaded TLB or Book-3e (ISA 2.06+) MMU */ #include <asm/nohash/mmu-book3e.h>
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index 25607604a7a5..2bc8c3f04f8a 100644 --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h @@ -341,7 +341,7 @@ static inline u16 get_mm_addr_key(struct mm_struct *mm, unsigned long address) #if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) && \ defined (CONFIG_PPC_64K_PAGES) #define MAX_PHYSMEM_BITS 51 -#else +#elif defined(CONFIG_SPARSEMEM) #define MAX_PHYSMEM_BITS 46 #endif
MAX_PHYSMEM_BITS only needs to be defined if CONFIG_SPARSEMEM is enabled, and that was the case before commit 4ffe713b7587 ("powerpc/mm: Increase the max addressable memory to 2PB"). On 32-bit systems, where CONFIG_SPARSEMEM is not enabled, we now define it as 46. That is larger than the real number of physical address bits, and breaks calculations in zsmalloc: mm/zsmalloc.c:130:49: warning: right shift count is negative [-Wshift-count-negative] MAX(32, (ZS_MAX_PAGES_PER_ZSPAGE << PAGE_SHIFT >> OBJ_INDEX_BITS)) ^~ ... mm/zsmalloc.c:253:21: error: variably modified 'size_class' at file scope struct size_class *size_class[ZS_SIZE_CLASSES]; ^~~~~~~~~~ Fixes: 4ffe713b7587 ("powerpc/mm: Increase the max addressable memory to 2PB") Cc: stable@vger.kernel.org Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- arch/powerpc/include/asm/mmu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)