linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [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

* [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

* [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 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 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 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

* 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

* 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

* 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

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).