Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
Michael Ellerman writes:
> Ben Hutchings writes:
>> On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote:
>>> On Mär 24 2019, Ben Hutchings 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"
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
---
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
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_BITS51
-#elif defined(CONFIG_SPARSEMEM)
-#define MAX_PHYSMEM_BITS46
-#endif
-
#ifdef CONFIG_PPC_BOOK3S_64
#include
#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_BITS44
+
/* Freescale Book-E software loaded TLB or Book-3e (ISA 2.06+) MMU */
#include
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
On Mär 25 2019, Michael Ellerman 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. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
Le 25/03/2019 à 12:35, Michael Ellerman a écrit : Ben Hutchings writes: On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote: On Mär 24 2019, Ben Hutchings 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
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
Ben Hutchings writes: > On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote: >> On Mär 24 2019, Ben Hutchings 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
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
Hi, Ben > Presumably you have CONFIG_PPC_BOOK3S_64 enabled and CONFIG_SPARSEMEM > disabled? > Was this configuration actually usable? I just noticed these build warnings too. FWIW, I've been using CONFIG_FLATMEM on my Power Mac G5 for about three years, so I guess the answer is "yes" (for this specific machine, at least).
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote: > On Mär 24 2019, Ben Hutchings 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 I'm not a reverse psychological virus. Please don't copy me into your signature. signature.asc Description: This is a digitally signed message part
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
On Mär 24 2019, Ben Hutchings wrote: > Presumably you have CONFIG_PPC_BOOK3S_64 enabled and CONFIG_SPARSEMEM > disabled? Was this configuration actually usable? Why not? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
On Sun, 2019-03-24 at 23:18 +0100, Andreas Schwab wrote:
> On Mär 22 2019, Michael Ellerman 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: sta...@vger.kernel.org
> > > Signed-off-by: Ben Hutchings
> >
> > 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_BITS51
-#elif defined(CONFIG_SPARSEMEM)
+#elif defined(CONFIG_PPC64)
#define MAX_PHYSMEM_BITS46
#endif
--- END ---
but whether the result will actually *work*, I don't know.
Ben.
--
Ben Hutchings
Larkinson's Law: All laws are basically false.
signature.asc
Description: This is a digitally signed message part
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
On Mär 22 2019, Michael Ellerman 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: sta...@vger.kernel.org
>> Signed-off-by: Ben Hutchings
>
> 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.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
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: sta...@vger.kernel.org
> Signed-off-by: Ben Hutchings
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/8bc086899816214fbc6047c9c7e15fca
cheers
