[PATCH V2 1/2] powerpc/mm/slice: Move slice_mask struct definition to slice.c

[PATCH V2 1/2] powerpc/mm/slice: Move slice_mask struct definition to slice.c

[Aneesh Kumar K.V]

This structure definition need not be in a header since this is used only by
slice.c file. So move it to slice.c. This also allow us to use SLICE_NUM_HIGH
instead of 512 and also helps in getting rid of one BUILD_BUG_ON().

I also switch the low_slices type to u64 from u16. This doesn't have an impact
on size of struct due to padding added with u16 type. This helps in using
bitmap printing function for printing slice mask.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/page_64.h | 11 -----------
 arch/powerpc/mm/slice.c            | 13 ++++++++++---
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/include/asm/page_64.h b/arch/powerpc/include/asm/page_64.h
index 9b60e9455c6e..3ecfc2734b50 100644
--- a/arch/powerpc/include/asm/page_64.h
+++ b/arch/powerpc/include/asm/page_64.h
@@ -99,17 +99,6 @@ extern u64 ppc64_pft_size;
 #define GET_HIGH_SLICE_INDEX(addr)	((addr) >> SLICE_HIGH_SHIFT)
 
 #ifndef __ASSEMBLY__
-/*
- * One bit per slice. We have lower slices which cover 256MB segments
- * upto 4G range. That gets us 16 low slices. For the rest we track slices
- * in 1TB size.
- * 64 below is actually SLICE_NUM_HIGH to fixup complie errros
- */
-struct slice_mask {
-	u16 low_slices;
-	DECLARE_BITMAP(high_slices, 512);
-};
-
 struct mm_struct;
 
 extern unsigned long slice_get_unmapped_area(unsigned long addr,
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index b3f45e413a60..08ac27eae408 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -37,7 +37,16 @@
 #include <asm/hugetlb.h>
 
 static DEFINE_SPINLOCK(slice_convert_lock);
-
+/*
+ * One bit per slice. We have lower slices which cover 256MB segments
+ * upto 4G range. That gets us 16 low slices. For the rest we track slices
+ * in 1TB size.
+ * 64 below is actually SLICE_NUM_HIGH to fixup complie errros
+ */
+struct slice_mask {
+	u64 low_slices;
+	DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
+};
 
 #ifdef DEBUG
 int _slice_debug = 1;
@@ -407,8 +416,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
 	struct mm_struct *mm = current->mm;
 	unsigned long newaddr;
 
-	/* Make sure high_slices bitmap size is same as we expected */
-	BUILD_BUG_ON(512 != SLICE_NUM_HIGH);
 	/*
 	 * init different masks
 	 */
-- 
2.7.4

[PATCH V2 2/2] powerpc/mm/slice: Update slice mask printing to use bitmap printing.

[Aneesh Kumar K.V]

We now get output like below which is much better.

[    0.935306]  good_mask low_slice: 0-15
[    0.935360]  good_mask high_slice: 0-511

Compared to

[    0.953414]  good_mask:1111111111111111 - 1111111111111.........

I also fixed an error with slice_dbg printing.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/mm/slice.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 08ac27eae408..d3701b0f439f 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -53,29 +53,13 @@ int _slice_debug = 1;
 
 static void slice_print_mask(const char *label, struct slice_mask mask)
 {
-	char	*p, buf[SLICE_NUM_LOW + 3 + SLICE_NUM_HIGH + 1];
-	int	i;
-
 	if (!_slice_debug)
 		return;
-	p = buf;
-	for (i = 0; i < SLICE_NUM_LOW; i++)
-		*(p++) = (mask.low_slices & (1 << i)) ? '1' : '0';
-	*(p++) = ' ';
-	*(p++) = '-';
-	*(p++) = ' ';
-	for (i = 0; i < SLICE_NUM_HIGH; i++) {
-		if (test_bit(i, mask.high_slices))
-			*(p++) = '1';
-		else
-			*(p++) = '0';
-	}
-	*(p++) = 0;
-
-	printk(KERN_DEBUG "%s:%s\n", label, buf);
+	pr_devel("%s low_slice: %*pbl\n", label, (int)SLICE_NUM_LOW, &mask.low_slices);
+	pr_devel("%s high_slice: %*pbl\n", label, (int)SLICE_NUM_HIGH, mask.high_slices);
 }
 
-#define slice_dbg(fmt...) do { if (_slice_debug) pr_debug(fmt); } while(0)
+#define slice_dbg(fmt...) do { if (_slice_debug) pr_devel(fmt); } while (0)
 
 #else
 
@@ -242,8 +226,8 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
 	}
 
 	slice_dbg(" lsps=%lx, hsps=%lx\n",
-		  mm->context.low_slices_psize,
-		  mm->context.high_slices_psize);
+		  (unsigned long)mm->context.low_slices_psize,
+		  (unsigned long)mm->context.high_slices_psize);
 
 	spin_unlock_irqrestore(&slice_convert_lock, flags);
 
@@ -685,8 +669,8 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
 
 
 	slice_dbg(" lsps=%lx, hsps=%lx\n",
-		  mm->context.low_slices_psize,
-		  mm->context.high_slices_psize);
+		  (unsigned long)mm->context.low_slices_psize,
+		  (unsigned long)mm->context.high_slices_psize);
 
  bail:
 	spin_unlock_irqrestore(&slice_convert_lock, flags);
-- 
2.7.4

Re: [PATCH V2 1/2] powerpc/mm/slice: Move slice_mask struct definition to slice.c

[Michael Ellerman]

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index b3f45e413a60..08ac27eae408 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -37,7 +37,16 @@
>  #include <asm/hugetlb.h>
>  
>  static DEFINE_SPINLOCK(slice_convert_lock);
> -
> +/*
> + * One bit per slice. We have lower slices which cover 256MB segments
> + * upto 4G range. That gets us 16 low slices. For the rest we track slices
> + * in 1TB size.

Can we tighten this comment up a bit.

What about:

> + * One bit per slice. The low slices cover the range 0 - 4GB, each
>   * slice being 256MB in size, for 16 low slices. The high slices
>   * cover the rest of the address space at 1TB granularity, with the
>   * exception of high slice 0 which covers the range 4GB - 1TB.

OK?

> + * 64 below is actually SLICE_NUM_HIGH to fixup complie errros

That line is bogus AFAICS, it refers to the old hardcoded value (prior
to 512), I'll drop it.

> + */
> +struct slice_mask {
> +	u64 low_slices;
> +	DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
> +};


cheers

Re: [PATCH V2 1/2] powerpc/mm/slice: Move slice_mask struct definition to slice.c

[Aneesh Kumar K.V]

On Tuesday 14 February 2017 11:55 AM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
>
>> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
>> index b3f45e413a60..08ac27eae408 100644
>> --- a/arch/powerpc/mm/slice.c
>> +++ b/arch/powerpc/mm/slice.c
>> @@ -37,7 +37,16 @@
>>   #include <asm/hugetlb.h>
>>   
>>   static DEFINE_SPINLOCK(slice_convert_lock);
>> -
>> +/*
>> + * One bit per slice. We have lower slices which cover 256MB segments
>> + * upto 4G range. That gets us 16 low slices. For the rest we track slices
>> + * in 1TB size.
> Can we tighten this comment up a bit.
>
> What about:
>
>> + * One bit per slice. The low slices cover the range 0 - 4GB, each
>>    * slice being 256MB in size, for 16 low slices. The high slices
>>    * cover the rest of the address space at 1TB granularity, with the
>>    * exception of high slice 0 which covers the range 4GB - 1TB.
> OK?


good.

>
>> + * 64 below is actually SLICE_NUM_HIGH to fixup complie errros
> That line is bogus AFAICS, it refers to the old hardcoded value (prior
> to 512), I'll drop it.


Thanks


-aneesh