* [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() @ 2019-01-17 12:13 Michael Ellerman 2019-01-17 12:13 ` [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX Michael Ellerman ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: Michael Ellerman @ 2019-01-17 12:13 UTC (permalink / raw) To: linuxppc-dev; +Cc: npiggin It's easier to reason about the code if we only set mmu_slb_size in one place, so convert open-coded assignments to use slb_set_size(). Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/kernel/prom.c | 2 +- arch/powerpc/mm/pgtable-radix.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 4181ec715f88..14693f8ccb80 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -238,7 +238,7 @@ static void __init init_mmu_slb_size(unsigned long node) of_get_flat_dt_prop(node, "ibm,slb-size", NULL); if (slb_size_ptr) - mmu_slb_size = be32_to_cpup(slb_size_ptr); + slb_set_size(be32_to_cpup(slb_size_ptr)); } #else #define init_mmu_slb_size(node) do { } while(0) diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c index 931156069a81..949fbc96b237 100644 --- a/arch/powerpc/mm/pgtable-radix.c +++ b/arch/powerpc/mm/pgtable-radix.c @@ -328,7 +328,8 @@ void __init radix_init_pgtable(void) struct memblock_region *reg; /* We don't support slb for radix */ - mmu_slb_size = 0; + slb_set_size(0); + /* * Create the linear mapping, using standard page size for now */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX 2019-01-17 12:13 [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Michael Ellerman @ 2019-01-17 12:13 ` Michael Ellerman 2019-01-17 16:30 ` Segher Boessenkool 2019-01-23 9:02 ` Aneesh Kumar K.V 2019-01-17 12:13 ` [PATCH 3/4] powerpc/64s: Move SLB init into hash_utils_64.c Michael Ellerman ` (2 subsequent siblings) 3 siblings, 2 replies; 14+ messages in thread From: Michael Ellerman @ 2019-01-17 12:13 UTC (permalink / raw) To: linuxppc-dev; +Cc: npiggin The recent rewrite of the SLB code into C included the assumption that all CPUs we run on have at least 32 SLB entries. This is currently true but a bit fragile as the SLB size is actually defined by the device tree and so could theoretically change at any time. The assumption is encoded in the fact that we use U32_MAX as the value for a full SLB bitmap. Instead, calculate what the full bitmap would be based on the SLB size we're given and store it. This still requires the SLB size to be a power of 2. Fixes: 126b11b294d1 ("powerpc/64s/hash: Add SLB allocation status bitmaps") Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/mm/slb.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c index bc3914d54e26..61450a9cf30d 100644 --- a/arch/powerpc/mm/slb.c +++ b/arch/powerpc/mm/slb.c @@ -506,9 +506,16 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm) asm volatile("isync" : : : "memory"); } +static u32 slb_full_bitmap; + void slb_set_size(u16 size) { mmu_slb_size = size; + + if (size >= 32) + slb_full_bitmap = U32_MAX; + else + slb_full_bitmap = (1ul << size) - 1; } void slb_initialize(void) @@ -611,7 +618,7 @@ static enum slb_index alloc_slb_index(bool kernel) * POWER7/8/9 have 32 SLB entries, this could be expanded if a * future CPU has more. */ - if (local_paca->slb_used_bitmap != U32_MAX) { + if (local_paca->slb_used_bitmap != slb_full_bitmap) { index = ffz(local_paca->slb_used_bitmap); local_paca->slb_used_bitmap |= 1U << index; if (kernel) -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX 2019-01-17 12:13 ` [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX Michael Ellerman @ 2019-01-17 16:30 ` Segher Boessenkool 2019-01-18 12:28 ` Michael Ellerman 2019-01-23 9:02 ` Aneesh Kumar K.V 1 sibling, 1 reply; 14+ messages in thread From: Segher Boessenkool @ 2019-01-17 16:30 UTC (permalink / raw) To: Michael Ellerman; +Cc: linuxppc-dev, npiggin On Thu, Jan 17, 2019 at 11:13:26PM +1100, Michael Ellerman wrote: > The recent rewrite of the SLB code into C included the assumption that > all CPUs we run on have at least 32 SLB entries. This is currently > true but a bit fragile as the SLB size is actually defined by the > device tree and so could theoretically change at any time. It also is guaranteed by the architecture, since at least 2.02, FWIW. Segher ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX 2019-01-17 16:30 ` Segher Boessenkool @ 2019-01-18 12:28 ` Michael Ellerman 2019-01-18 23:12 ` Segher Boessenkool 0 siblings, 1 reply; 14+ messages in thread From: Michael Ellerman @ 2019-01-18 12:28 UTC (permalink / raw) To: Segher Boessenkool; +Cc: linuxppc-dev, npiggin Segher Boessenkool <segher@kernel.crashing.org> writes: > On Thu, Jan 17, 2019 at 11:13:26PM +1100, Michael Ellerman wrote: >> The recent rewrite of the SLB code into C included the assumption that >> all CPUs we run on have at least 32 SLB entries. This is currently >> true but a bit fragile as the SLB size is actually defined by the >> device tree and so could theoretically change at any time. > > It also is guaranteed by the architecture, since at least 2.02, FWIW. True. Actually 2.00 says at least 32. Unfortunately we don't live in a world where "the architecture guarantees it" has any bearing on reality :) But given it *should* always be at least 32 maybe I should optimise for that case. We could use a static key to skip the U32_MAX comparison and go down the else path. cheers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX 2019-01-18 12:28 ` Michael Ellerman @ 2019-01-18 23:12 ` Segher Boessenkool 0 siblings, 0 replies; 14+ messages in thread From: Segher Boessenkool @ 2019-01-18 23:12 UTC (permalink / raw) To: Michael Ellerman; +Cc: linuxppc-dev, npiggin On Fri, Jan 18, 2019 at 11:28:24PM +1100, Michael Ellerman wrote: > Segher Boessenkool <segher@kernel.crashing.org> writes: > > > On Thu, Jan 17, 2019 at 11:13:26PM +1100, Michael Ellerman wrote: > >> The recent rewrite of the SLB code into C included the assumption that > >> all CPUs we run on have at least 32 SLB entries. This is currently > >> true but a bit fragile as the SLB size is actually defined by the > >> device tree and so could theoretically change at any time. > > > > It also is guaranteed by the architecture, since at least 2.02, FWIW. > > True. Actually 2.00 says at least 32. > > Unfortunately we don't live in a world where "the architecture > guarantees it" has any bearing on reality :) It's a pretty strong hint. I don't remember any hardware where it is not true, either. (That might be selective memory ;-) ) > But given it *should* always be at least 32 maybe I should optimise for > that case. We could use a static key to skip the U32_MAX comparison and > go down the else path. Ah that sounds like a good idea :-) Segher ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX 2019-01-17 12:13 ` [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX Michael Ellerman 2019-01-17 16:30 ` Segher Boessenkool @ 2019-01-23 9:02 ` Aneesh Kumar K.V 1 sibling, 0 replies; 14+ messages in thread From: Aneesh Kumar K.V @ 2019-01-23 9:02 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: npiggin Michael Ellerman <mpe@ellerman.id.au> writes: > The recent rewrite of the SLB code into C included the assumption that > all CPUs we run on have at least 32 SLB entries. This is currently > true but a bit fragile as the SLB size is actually defined by the > device tree and so could theoretically change at any time. > > The assumption is encoded in the fact that we use U32_MAX as the value > for a full SLB bitmap. Instead, calculate what the full bitmap would > be based on the SLB size we're given and store it. This still requires > the SLB size to be a power of 2. So if it is less than 32 we want to make sure we don't allocate an index above 32. Is this the reason for that radom assert_slb_presence? Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > > Fixes: 126b11b294d1 ("powerpc/64s/hash: Add SLB allocation status bitmaps") > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/mm/slb.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c > index bc3914d54e26..61450a9cf30d 100644 > --- a/arch/powerpc/mm/slb.c > +++ b/arch/powerpc/mm/slb.c > @@ -506,9 +506,16 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm) > asm volatile("isync" : : : "memory"); > } > > +static u32 slb_full_bitmap; > + > void slb_set_size(u16 size) > { > mmu_slb_size = size; > + > + if (size >= 32) > + slb_full_bitmap = U32_MAX; > + else > + slb_full_bitmap = (1ul << size) - 1; > } > > void slb_initialize(void) > @@ -611,7 +618,7 @@ static enum slb_index alloc_slb_index(bool kernel) > * POWER7/8/9 have 32 SLB entries, this could be expanded if a > * future CPU has more. > */ > - if (local_paca->slb_used_bitmap != U32_MAX) { > + if (local_paca->slb_used_bitmap != slb_full_bitmap) { > index = ffz(local_paca->slb_used_bitmap); > local_paca->slb_used_bitmap |= 1U << index; > if (kernel) > -- > 2.20.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] powerpc/64s: Move SLB init into hash_utils_64.c 2019-01-17 12:13 [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Michael Ellerman 2019-01-17 12:13 ` [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX Michael Ellerman @ 2019-01-17 12:13 ` Michael Ellerman 2019-01-23 9:04 ` Aneesh Kumar K.V 2019-01-17 12:13 ` [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging Michael Ellerman 2019-01-23 8:59 ` [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Aneesh Kumar K.V 3 siblings, 1 reply; 14+ messages in thread From: Michael Ellerman @ 2019-01-17 12:13 UTC (permalink / raw) To: linuxppc-dev; +Cc: npiggin The SLB initialisation code is spread around a bit between prom.c and hash_utils_64.c. Consolidate it all in hash_utils_64.c. This slightly changes the timing of when mmu_slb_size is initialised, but that should have no effect. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/kernel/prom.c | 16 ---------------- arch/powerpc/mm/hash_utils_64.c | 15 ++++++++++----- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 14693f8ccb80..018ededd1948 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -229,21 +229,6 @@ static void __init check_cpu_pa_features(unsigned long node) ibm_pa_features, ARRAY_SIZE(ibm_pa_features)); } -#ifdef CONFIG_PPC_BOOK3S_64 -static void __init init_mmu_slb_size(unsigned long node) -{ - const __be32 *slb_size_ptr; - - slb_size_ptr = of_get_flat_dt_prop(node, "slb-size", NULL) ? : - of_get_flat_dt_prop(node, "ibm,slb-size", NULL); - - if (slb_size_ptr) - slb_set_size(be32_to_cpup(slb_size_ptr)); -} -#else -#define init_mmu_slb_size(node) do { } while(0) -#endif - static struct feature_property { const char *name; u32 min_value; @@ -379,7 +364,6 @@ static int __init early_init_dt_scan_cpus(unsigned long node, } identical_pvr_fixup(node); - init_mmu_slb_size(node); #ifdef CONFIG_PPC64 if (nthreads == 1) diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c index 4aa0797000f7..33ce76be17de 100644 --- a/arch/powerpc/mm/hash_utils_64.c +++ b/arch/powerpc/mm/hash_utils_64.c @@ -344,9 +344,8 @@ static int __init parse_disable_1tb_segments(char *p) } early_param("disable_1tb_segments", parse_disable_1tb_segments); -static int __init htab_dt_scan_seg_sizes(unsigned long node, - const char *uname, int depth, - void *data) +static int __init htab_dt_scan_slb(unsigned long node, const char *uname, + int depth, void *data) { const char *type = of_get_flat_dt_prop(node, "device_type", NULL); const __be32 *prop; @@ -356,6 +355,12 @@ static int __init htab_dt_scan_seg_sizes(unsigned long node, if (type == NULL || strcmp(type, "cpu") != 0) return 0; + prop = of_get_flat_dt_prop(node, "slb-size", NULL); + if (!prop) + prop = of_get_flat_dt_prop(node, "ibm,slb-size", NULL); + if (prop) + slb_set_size(be32_to_cpup(prop)); + prop = of_get_flat_dt_prop(node, "ibm,processor-segment-sizes", &size); if (prop == NULL) return 0; @@ -954,8 +959,8 @@ static void __init htab_initialize(void) void __init hash__early_init_devtree(void) { - /* Initialize segment sizes */ - of_scan_flat_dt(htab_dt_scan_seg_sizes, NULL); + /* Initialize SLB size and segment sizes */ + of_scan_flat_dt(htab_dt_scan_slb, NULL); /* Initialize page sizes */ htab_scan_page_sizes(); -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] powerpc/64s: Move SLB init into hash_utils_64.c 2019-01-17 12:13 ` [PATCH 3/4] powerpc/64s: Move SLB init into hash_utils_64.c Michael Ellerman @ 2019-01-23 9:04 ` Aneesh Kumar K.V 0 siblings, 0 replies; 14+ messages in thread From: Aneesh Kumar K.V @ 2019-01-23 9:04 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: npiggin Michael Ellerman <mpe@ellerman.id.au> writes: > The SLB initialisation code is spread around a bit between prom.c and > hash_utils_64.c. Consolidate it all in hash_utils_64.c. > > This slightly changes the timing of when mmu_slb_size is initialised, > but that should have no effect. Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/kernel/prom.c | 16 ---------------- > arch/powerpc/mm/hash_utils_64.c | 15 ++++++++++----- > 2 files changed, 10 insertions(+), 21 deletions(-) > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 14693f8ccb80..018ededd1948 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -229,21 +229,6 @@ static void __init check_cpu_pa_features(unsigned long node) > ibm_pa_features, ARRAY_SIZE(ibm_pa_features)); > } > > -#ifdef CONFIG_PPC_BOOK3S_64 > -static void __init init_mmu_slb_size(unsigned long node) > -{ > - const __be32 *slb_size_ptr; > - > - slb_size_ptr = of_get_flat_dt_prop(node, "slb-size", NULL) ? : > - of_get_flat_dt_prop(node, "ibm,slb-size", NULL); > - > - if (slb_size_ptr) > - slb_set_size(be32_to_cpup(slb_size_ptr)); > -} > -#else > -#define init_mmu_slb_size(node) do { } while(0) > -#endif > - > static struct feature_property { > const char *name; > u32 min_value; > @@ -379,7 +364,6 @@ static int __init early_init_dt_scan_cpus(unsigned long node, > } > > identical_pvr_fixup(node); > - init_mmu_slb_size(node); > > #ifdef CONFIG_PPC64 > if (nthreads == 1) > diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c > index 4aa0797000f7..33ce76be17de 100644 > --- a/arch/powerpc/mm/hash_utils_64.c > +++ b/arch/powerpc/mm/hash_utils_64.c > @@ -344,9 +344,8 @@ static int __init parse_disable_1tb_segments(char *p) > } > early_param("disable_1tb_segments", parse_disable_1tb_segments); > > -static int __init htab_dt_scan_seg_sizes(unsigned long node, > - const char *uname, int depth, > - void *data) > +static int __init htab_dt_scan_slb(unsigned long node, const char *uname, > + int depth, void *data) > { > const char *type = of_get_flat_dt_prop(node, "device_type", NULL); > const __be32 *prop; > @@ -356,6 +355,12 @@ static int __init htab_dt_scan_seg_sizes(unsigned long node, > if (type == NULL || strcmp(type, "cpu") != 0) > return 0; > > + prop = of_get_flat_dt_prop(node, "slb-size", NULL); > + if (!prop) > + prop = of_get_flat_dt_prop(node, "ibm,slb-size", NULL); > + if (prop) > + slb_set_size(be32_to_cpup(prop)); > + > prop = of_get_flat_dt_prop(node, "ibm,processor-segment-sizes", &size); > if (prop == NULL) > return 0; > @@ -954,8 +959,8 @@ static void __init htab_initialize(void) > > void __init hash__early_init_devtree(void) > { > - /* Initialize segment sizes */ > - of_scan_flat_dt(htab_dt_scan_seg_sizes, NULL); > + /* Initialize SLB size and segment sizes */ > + of_scan_flat_dt(htab_dt_scan_slb, NULL); > > /* Initialize page sizes */ > htab_scan_page_sizes(); > -- > 2.20.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging 2019-01-17 12:13 [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Michael Ellerman 2019-01-17 12:13 ` [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX Michael Ellerman 2019-01-17 12:13 ` [PATCH 3/4] powerpc/64s: Move SLB init into hash_utils_64.c Michael Ellerman @ 2019-01-17 12:13 ` Michael Ellerman 2019-01-19 0:13 ` Michal Suchánek 2019-01-23 9:10 ` Aneesh Kumar K.V 2019-01-23 8:59 ` [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Aneesh Kumar K.V 3 siblings, 2 replies; 14+ messages in thread From: Michael Ellerman @ 2019-01-17 12:13 UTC (permalink / raw) To: linuxppc-dev; +Cc: npiggin On machines with 1TB segments and a 32-entry SLB it's quite hard to cause sufficient SLB pressure to trigger bugs caused due to badly timed SLB faults. We have seen this in the past and a few years ago added the disable_1tb_segments command line option to force the use of 256MB segments. However even this allows some bugs to slip through testing if the SLB entry in question was recently accessed. So add a new command line parameter for debugging which shrinks the SLB to the minimum size we can support. Currently that size is 3, two bolted SLBs and 1 for dynamic use. This creates the maximal SLB pressure while still allowing the kernel to operate. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/mm/slb.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c index 61450a9cf30d..0f33e28f97da 100644 --- a/arch/powerpc/mm/slb.c +++ b/arch/powerpc/mm/slb.c @@ -506,10 +506,24 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm) asm volatile("isync" : : : "memory"); } +static bool shrink_slb = false; + +static int __init parse_shrink_slb(char *p) +{ + shrink_slb = true; + slb_set_size(0); + + return 0; +} +early_param("shrink_slb", parse_shrink_slb); + static u32 slb_full_bitmap; void slb_set_size(u16 size) { + if (shrink_slb) + size = SLB_NUM_BOLTED + 1; + mmu_slb_size = size; if (size >= 32) -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging 2019-01-17 12:13 ` [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging Michael Ellerman @ 2019-01-19 0:13 ` Michal Suchánek 2019-01-19 10:27 ` Michael Ellerman 2019-01-23 9:10 ` Aneesh Kumar K.V 1 sibling, 1 reply; 14+ messages in thread From: Michal Suchánek @ 2019-01-19 0:13 UTC (permalink / raw) To: Michael Ellerman; +Cc: linuxppc-dev, npiggin On Thu, 17 Jan 2019 23:13:28 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote: > On machines with 1TB segments and a 32-entry SLB it's quite hard to > cause sufficient SLB pressure to trigger bugs caused due to badly > timed SLB faults. > > We have seen this in the past and a few years ago added the > disable_1tb_segments command line option to force the use of 256MB > segments. However even this allows some bugs to slip through testing > if the SLB entry in question was recently accessed. > > So add a new command line parameter for debugging which shrinks the > SLB to the minimum size we can support. Currently that size is 3, two > bolted SLBs and 1 for dynamic use. This creates the maximal SLB Doesn't this violate the power of 2 requirement stated in 2/4? Thanks Michal > pressure while still allowing the kernel to operate. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/mm/slb.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c > index 61450a9cf30d..0f33e28f97da 100644 > --- a/arch/powerpc/mm/slb.c > +++ b/arch/powerpc/mm/slb.c > @@ -506,10 +506,24 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm) > asm volatile("isync" : : : "memory"); > } > > +static bool shrink_slb = false; > + > +static int __init parse_shrink_slb(char *p) > +{ > + shrink_slb = true; > + slb_set_size(0); > + > + return 0; > +} > +early_param("shrink_slb", parse_shrink_slb); > + > static u32 slb_full_bitmap; > > void slb_set_size(u16 size) > { > + if (shrink_slb) > + size = SLB_NUM_BOLTED + 1; > + > mmu_slb_size = size; > > if (size >= 32) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging 2019-01-19 0:13 ` Michal Suchánek @ 2019-01-19 10:27 ` Michael Ellerman 2019-01-23 8:40 ` Nicholas Piggin 0 siblings, 1 reply; 14+ messages in thread From: Michael Ellerman @ 2019-01-19 10:27 UTC (permalink / raw) To: Michal Suchánek; +Cc: linuxppc-dev, npiggin Michal Suchánek <msuchanek@suse.de> writes: > On Thu, 17 Jan 2019 23:13:28 +1100 > Michael Ellerman <mpe@ellerman.id.au> wrote: > >> On machines with 1TB segments and a 32-entry SLB it's quite hard to >> cause sufficient SLB pressure to trigger bugs caused due to badly >> timed SLB faults. >> >> We have seen this in the past and a few years ago added the >> disable_1tb_segments command line option to force the use of 256MB >> segments. However even this allows some bugs to slip through testing >> if the SLB entry in question was recently accessed. >> >> So add a new command line parameter for debugging which shrinks the >> SLB to the minimum size we can support. Currently that size is 3, two >> bolted SLBs and 1 for dynamic use. This creates the maximal SLB > > Doesn't this violate the power of 2 requirement stated in 2/4? Yes. Good point. This was originally a hack patch in my tree, back when SLB_NUM_BOLTED was 3 and before Nick even added the slb_used_bitmap, so back then it was a power of 2 but it also didn't matter :) I think I'll rework the slb_full_bitmap patch anyway and remove the power of 2 requirement, so then this patch will be OK. Thanks for the review! cheers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging 2019-01-19 10:27 ` Michael Ellerman @ 2019-01-23 8:40 ` Nicholas Piggin 0 siblings, 0 replies; 14+ messages in thread From: Nicholas Piggin @ 2019-01-23 8:40 UTC (permalink / raw) To: Michael Ellerman, Michal Suchánek; +Cc: linuxppc-dev Michael Ellerman's on January 19, 2019 8:27 pm: > Michal Suchánek <msuchanek@suse.de> writes: > >> On Thu, 17 Jan 2019 23:13:28 +1100 >> Michael Ellerman <mpe@ellerman.id.au> wrote: >> >>> On machines with 1TB segments and a 32-entry SLB it's quite hard to >>> cause sufficient SLB pressure to trigger bugs caused due to badly >>> timed SLB faults. >>> >>> We have seen this in the past and a few years ago added the >>> disable_1tb_segments command line option to force the use of 256MB >>> segments. However even this allows some bugs to slip through testing >>> if the SLB entry in question was recently accessed. >>> >>> So add a new command line parameter for debugging which shrinks the >>> SLB to the minimum size we can support. Currently that size is 3, two >>> bolted SLBs and 1 for dynamic use. This creates the maximal SLB >> >> Doesn't this violate the power of 2 requirement stated in 2/4? > > Yes. Good point. This was originally a hack patch in my tree, back when > SLB_NUM_BOLTED was 3 and before Nick even added the slb_used_bitmap, so > back then it was a power of 2 but it also didn't matter :) > > I think I'll rework the slb_full_bitmap patch anyway and remove the > power of 2 requirement, so then this patch will be OK. Thanks, good patch and really will help testing. Sometimes even if you can get enough pressure with the 256MB segments, you actually want to test 1TB segment code paths too. Thanks, Nick ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging 2019-01-17 12:13 ` [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging Michael Ellerman 2019-01-19 0:13 ` Michal Suchánek @ 2019-01-23 9:10 ` Aneesh Kumar K.V 1 sibling, 0 replies; 14+ messages in thread From: Aneesh Kumar K.V @ 2019-01-23 9:10 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: npiggin Michael Ellerman <mpe@ellerman.id.au> writes: > On machines with 1TB segments and a 32-entry SLB it's quite hard to > cause sufficient SLB pressure to trigger bugs caused due to badly > timed SLB faults. > > We have seen this in the past and a few years ago added the > disable_1tb_segments command line option to force the use of 256MB > segments. However even this allows some bugs to slip through testing > if the SLB entry in question was recently accessed. > > So add a new command line parameter for debugging which shrinks the > SLB to the minimum size we can support. Currently that size is 3, two > bolted SLBs and 1 for dynamic use. This creates the maximal SLB > pressure while still allowing the kernel to operate. > Should we put this within DEBUG_VM? Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/mm/slb.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c > index 61450a9cf30d..0f33e28f97da 100644 > --- a/arch/powerpc/mm/slb.c > +++ b/arch/powerpc/mm/slb.c > @@ -506,10 +506,24 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm) > asm volatile("isync" : : : "memory"); > } > > +static bool shrink_slb = false; > + > +static int __init parse_shrink_slb(char *p) > +{ > + shrink_slb = true; > + slb_set_size(0); Why do we need call slb_set_size(0) here? htab_dt_scan_seg_sizes should find the shirnk_slb = true? > + > + return 0; > +} > +early_param("shrink_slb", parse_shrink_slb); > + > static u32 slb_full_bitmap; > > void slb_set_size(u16 size) > { > + if (shrink_slb) > + size = SLB_NUM_BOLTED + 1; > + > mmu_slb_size = size; > > if (size >= 32) > -- > 2.20.1 -aneesh ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() 2019-01-17 12:13 [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Michael Ellerman ` (2 preceding siblings ...) 2019-01-17 12:13 ` [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging Michael Ellerman @ 2019-01-23 8:59 ` Aneesh Kumar K.V 3 siblings, 0 replies; 14+ messages in thread From: Aneesh Kumar K.V @ 2019-01-23 8:59 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: npiggin Michael Ellerman <mpe@ellerman.id.au> writes: > It's easier to reason about the code if we only set mmu_slb_size in > one place, so convert open-coded assignments to use slb_set_size(). > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/kernel/prom.c | 2 +- > arch/powerpc/mm/pgtable-radix.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 4181ec715f88..14693f8ccb80 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -238,7 +238,7 @@ static void __init init_mmu_slb_size(unsigned long node) > of_get_flat_dt_prop(node, "ibm,slb-size", NULL); > > if (slb_size_ptr) > - mmu_slb_size = be32_to_cpup(slb_size_ptr); > + slb_set_size(be32_to_cpup(slb_size_ptr)); > } > #else > #define init_mmu_slb_size(node) do { } while(0) > diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c > index 931156069a81..949fbc96b237 100644 > --- a/arch/powerpc/mm/pgtable-radix.c > +++ b/arch/powerpc/mm/pgtable-radix.c > @@ -328,7 +328,8 @@ void __init radix_init_pgtable(void) > struct memblock_region *reg; > > /* We don't support slb for radix */ > - mmu_slb_size = 0; > + slb_set_size(0); > + > /* > * Create the linear mapping, using standard page size for now > */ > -- > 2.20.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-01-23 9:11 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-17 12:13 [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Michael Ellerman 2019-01-17 12:13 ` [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX Michael Ellerman 2019-01-17 16:30 ` Segher Boessenkool 2019-01-18 12:28 ` Michael Ellerman 2019-01-18 23:12 ` Segher Boessenkool 2019-01-23 9:02 ` Aneesh Kumar K.V 2019-01-17 12:13 ` [PATCH 3/4] powerpc/64s: Move SLB init into hash_utils_64.c Michael Ellerman 2019-01-23 9:04 ` Aneesh Kumar K.V 2019-01-17 12:13 ` [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging Michael Ellerman 2019-01-19 0:13 ` Michal Suchánek 2019-01-19 10:27 ` Michael Ellerman 2019-01-23 8:40 ` Nicholas Piggin 2019-01-23 9:10 ` Aneesh Kumar K.V 2019-01-23 8:59 ` [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size() Aneesh Kumar K.V
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).