powerpc/mm/slice: improve slice speed and stack use

Signed-off-by Nicholas Piggin <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

arch/powerpc/mm/mmu_context_book3s64.c +0 -6

index 929d9ef7083f..80acad52b006 100644
@@ -93,13 +93,6 @@ static int hash__init_new_context(struct mm_struct *mm)
 	if (index < 0)
 		return index;
 
-	/*
-	 * In the case of exec, use the default limit,
-	 * otherwise inherit it from the mm we are duplicating.
-	 */
-	if (!mm->context.slb_addr_limit)
-		mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW_USER64;

arch/powerpc/mm/mmu_context_nohash.c +0 -3

index d98f7e5c141b..be8f5c9d4d08 100644
@@ -332,9 +332,6 @@ int init_new_context(struct task_struct *t, struct mm_struct *mm)
 	pr_hard("initing context for mm @%p\n", mm);
 
 #ifdef	CONFIG_PPC_MM_SLICES
-	if (!mm->context.slb_addr_limit)
-		mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW;
-

arch/powerpc/mm/slice.c +2 -26

index 5e9e1e57d580..af4351b15d01 100644
@@ -671,70 +671,29 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
 }
 EXPORT_SYMBOL_GPL(get_slice_psize);
 
-/*
- * This is called by hash_page when it needs to do a lazy conversion of
- * an address space from real 64K pages to combo 4K pages (typically
- * when hitting a non cacheable mapping on a processor or hypervisor
- * that won't allow them for 64K pages).
- *
- * This is also called in init_new_context() to change back the user
- * psize from whatever the parent context had it set to
- * N.B. This may be called before mm->context.id has been set.
- *
- * This function will only change the content of the {low,high)_slice_psize
- * masks, it will not flush SLBs as this shall be handled lazily by the
- * caller.
- */
-void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
+void slice_init_new_context_exec(struct mm_struct *mm)
 {
-	int index, mask_index;
 	unsigned char *hpsizes, *lpsizes;
-	unsigned long flags;
-	unsigned int old_psize;
-	int i;
-
-	slice_dbg("slice_set_user_psize(mm=%p, psize=%d)\n", mm, psize);
-
-	VM_BUG_ON(radix_enabled());
-	spin_lock_irqsave(&slice_convert_lock, flags);
+	unsigned int psize = mmu_virtual_psize;
 
-	old_psize = mm->context.user_psize;
-	slice_dbg(" old_psize=%d\n", old_psize);

Pass around const pointers to struct slice_mask where possible, rather than copies of slice_mask, to reduce stack and call overhead.

checkstack.pl gives, before:
0x00000d1c slice_get_unmapped_area [slice.o]: 592
0x00001864 is_hugepage_only_range [slice.o]: 448
0x00000754 slice_find_area_topdown [slice.o]: 400
0x00000484 slice_find_area_bottomup.isra.1 [slice.o]: 272
0x000017b4 slice_set_range_psize [slice.o]: 224
0x00000a4c slice_find_area [slice.o]: 128
0x00000160 slice_check_fit [slice.o]: 112

after:
0x00000ad0 slice_get_unmapped_area [slice.o]: 448
0x00001464 is_hugepage_only_range [slice.o]: 288
0x000006c0 slice_find_area [slice.o]: 144
0x0000016c slice_check_fit [slice.o]: 128
0x00000528 slice_find_area_bottomup.isra.2 [slice.o]: 128
0x000013e4 slice_set_range_psize [slice.o]: 128

This increases vfork+exec+exit performance by 1.5%.

Reduces time to mmap+munmap a 64kB page by 17%.

Signed-off-by Nicholas Piggin <[email protected]>

---
  arch/powerpc/mm/slice.c | 87 ++++++++++++++++++++++++++-----------------------
  1 file changed, 47 insertions(+), 40 deletions(-)

arch/powerpc/mm/slice.c +6 -4

index 9625ceb35685..233c42d593dc 100644
@@ -50,19 +50,21 @@ struct slice_mask {
  #ifdef DEBUG
  int _slice_debug = 1;
 
-static void slice_print_mask(const char *label, struct slice_mask mask)
+static void slice_print_mask(const char *label, const struct slice_mask *mask)
  {
  	if (!_slice_debug)
  		return;
-	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);
+	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_devel(fmt); } while (0)
 
  #else
 
-static void slice_print_mask(const char *label, struct slice_mask mask) {}
+static void slice_print_mask(const char *label, const struct slice_mask *mask) {}
  #define slice_dbg(fmt...)
 
  #endif

arch/powerpc/mm/slice.c +39 -35

  				unsigned long high_limit)
  {
  	unsigned char *hpsizes, *lpsizes;

@@ -179,7 +182,8 @@ static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_ma
  }
 
  static int slice_check_fit(struct mm_struct *mm,
-			   struct slice_mask mask, struct slice_mask available)
+			   const struct slice_mask *mask,
+			   const struct slice_mask *available)
  {
  	DECLARE_BITMAP(result, SLICE_NUM_HIGH);
  	/*

@@ -189,14 +193,14 @@ static int slice_check_fit(struct mm_struct *mm,
  	unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
 
  	if (!SLICE_NUM_HIGH)
-		return (mask.low_slices & available.low_slices) ==
-		       mask.low_slices;
+		return (mask->low_slices & available->low_slices) ==
+		       mask->low_slices;
 
-	bitmap_and(result, mask.high_slices,
-		   available.high_slices, slice_count);
+	bitmap_and(result, mask->high_slices,
+		   available->high_slices, slice_count);
 
-	return (mask.low_slices & available.low_slices) == mask.low_slices &&
-		bitmap_equal(result, mask.high_slices, slice_count);
+	return (mask->low_slices & available->low_slices) == mask->low_slices &&
+		bitmap_equal(result, mask->high_slices, slice_count);
  }
 
  static void slice_flush_segments(void *parm)

@@ -216,7 +220,8 @@ static void slice_flush_segments(void *parm)
  #endif
  }
 
-static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psize)
+static void slice_convert(struct mm_struct *mm,
+				const struct slice_mask *mask, int psize)
  {
  	int index, mask_index;
  	/* Write the new slice psize bits */

@@ -233,7 +238,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
 
  	lpsizes = mm->context.low_slices_psize;
  	for (i = 0; i < SLICE_NUM_LOW; i++) {
-		if (!(mask.low_slices & (1u << i)))
+		if (!(mask->low_slices & (1u << i)))
  			continue;
 
  		mask_index = i & 0x1;

@@ -244,7 +249,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
 
  	hpsizes = mm->context.high_slices_psize;
  	for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
-		if (!test_bit(i, mask.high_slices))
+		if (!test_bit(i, mask->high_slices))
  			continue;
 
  		mask_index = i & 0x1;

@@ -270,26 +275,25 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
   * 'available' slice_mark.
   */
  static bool slice_scan_available(unsigned long addr,
-				 struct slice_mask available,
-				 int end,
-				 unsigned long *boundary_addr)
+				 const struct slice_mask *available,
+				 int end, unsigned long *boundary_addr)
  {
  	unsigned long slice;
  	if (addr < SLICE_LOW_TOP) {
  		slice = GET_LOW_SLICE_INDEX(addr);
  		*boundary_addr = (slice + end) << SLICE_LOW_SHIFT;
-		return !!(available.low_slices & (1u << slice));
+		return !!(available->low_slices & (1u << slice));
  	} else {
  		slice = GET_HIGH_SLICE_INDEX(addr);
  		*boundary_addr = (slice + end) ?
  			((slice + end) << SLICE_HIGH_SHIFT) : SLICE_LOW_TOP;
-		return !!test_bit(slice, available.high_slices);
+		return !!test_bit(slice, available->high_slices);
  	}
  }
 
  static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
  					      unsigned long len,
-					      struct slice_mask available,
+					      const struct slice_mask *available,
  					      int psize, unsigned long high_limit)
  {
  	int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);

@@ -335,7 +339,7 @@ static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
 
  static unsigned long slice_find_area_topdown(struct mm_struct *mm,
  					     unsigned long len,
-					     struct slice_mask available,
+					     const struct slice_mask *available,
  					     int psize, unsigned long high_limit)
  {
  	int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);

@@ -393,7 +397,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
 
 
  static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
-				     struct slice_mask mask, int psize,
+				     const struct slice_mask *mask, int psize,
  				     int topdown, unsigned long high_limit)
  {
  	if (topdown)

@@ -402,7 +406,8 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
  		return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
  }
 
-static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
+static inline void slice_or_mask(struct slice_mask *dst,
+					const struct slice_mask *src)
  {
  	dst->low_slices |= src->low_slices;
  	if (!SLICE_NUM_HIGH)

@@ -411,7 +416,8 @@ static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
  		  SLICE_NUM_HIGH);
  }
 
-static inline void slice_andnot_mask(struct slice_mask *dst, struct slice_mask *src)
+static inline void slice_andnot_mask(struct slice_mask *dst,
+					const struct slice_mask *src)
  {
  	dst->low_slices &= ~src->low_slices;
 

@@ -501,7 +507,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	 * already
  	 */
  	slice_mask_for_size(mm, psize, &good_mask, high_limit);
-	slice_print_mask(" good_mask", good_mask);
+	slice_print_mask(" good_mask", &good_mask);
 
  	/*
  	 * Here "good" means slices that are already the right page size,

@@ -535,12 +541,12 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	if (addr != 0 || fixed) {
  		/* Build a mask for the requested range */
  		slice_range_to_mask(addr, len, &mask);
-		slice_print_mask(" mask", mask);
+		slice_print_mask(" mask", &mask);
 
  		/* Check if we fit in the good mask. If we do, we just return,
  		 * nothing else to do
  		 */
-		if (slice_check_fit(mm, mask, good_mask)) {
+		if (slice_check_fit(mm, &mask, &good_mask)) {
  			slice_dbg(" fits good !\n");
  			return addr;
  		}

@@ -548,7 +554,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  		/* Now let's see if we can find something in the existing
  		 * slices for that size
  		 */
-		newaddr = slice_find_area(mm, len, good_mask,
+		newaddr = slice_find_area(mm, len, &good_mask,
  					  psize, topdown, high_limit);
  		if (newaddr != -ENOMEM) {
  			/* Found within the good mask, we don't have to setup,

@@ -564,9 +570,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	 */
  	slice_mask_for_free(mm, &potential_mask, high_limit);
  	slice_or_mask(&potential_mask, &good_mask);
-	slice_print_mask(" potential", potential_mask);
+	slice_print_mask(" potential", &potential_mask);
 
-	if ((addr != 0 || fixed) && slice_check_fit(mm, mask, potential_mask)) {
+	if ((addr != 0 || fixed) &&
+			slice_check_fit(mm, &mask, &potential_mask)) {
  		slice_dbg(" fits potential !\n");
  		goto convert;
  	}

@@ -581,7 +588,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	 * anywhere in the good area.
  	 */
  	if (addr) {
-		addr = slice_find_area(mm, len, good_mask,
+		addr = slice_find_area(mm, len, &good_mask,
  				       psize, topdown, high_limit);
  		if (addr != -ENOMEM) {
  			slice_dbg(" found area at 0x%lx\n", addr);

@@ -592,14 +599,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	/* Now let's see if we can find something in the existing slices
  	 * for that size plus free slices
  	 */
-	addr = slice_find_area(mm, len, potential_mask,
+	addr = slice_find_area(mm, len, &potential_mask,
  			       psize, topdown, high_limit);
 
  #ifdef CONFIG_PPC_64K_PAGES
  	if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
  		/* retry the search with 4k-page slices included */
  		slice_or_mask(&potential_mask, &compat_mask);
-		addr = slice_find_area(mm, len, potential_mask,
+		addr = slice_find_area(mm, len, &potential_mask,
  				       psize, topdown, high_limit);
  	}
  #endif

@@ -609,7 +616,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
 
  	slice_range_to_mask(addr, len, &mask);
  	slice_dbg(" found potential area at 0x%lx\n", addr);
-	slice_print_mask(" mask", mask);
+	slice_print_mask(" mask", &mask);
 
   convert:
  	slice_andnot_mask(&mask, &good_mask);

@@ -617,7 +624,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	if (mask.low_slices ||
  	    (SLICE_NUM_HIGH &&
  	     !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
-		slice_convert(mm, mask, psize);
+		slice_convert(mm, &mask, psize);
  		if (psize > MMU_PAGE_BASE)
  			on_each_cpu(slice_flush_segments, mm, 1);
  	}

@@ -706,7 +713,7 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
  	VM_BUG_ON(radix_enabled());
 
  	slice_range_to_mask(start, len, &mask);
-	slice_convert(mm, mask, psize);
+	slice_convert(mm, &mask, psize);
  }
 
  #ifdef CONFIG_HUGETLB_PAGE

@@ -753,9 +760,9 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  #if 0 /* too verbose */
  	slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
  		 mm, addr, len);
-	slice_print_mask(" mask", mask);
-	slice_print_mask(" available", available);
+	slice_print_mask(" mask", &mask);
+	slice_print_mask(" available", &available);
  #endif
-	return !slice_check_fit(mm, mask, available);
+	return !slice_check_fit(mm, &mask, &available);
  }
  #endif

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

arch/powerpc/mm/slice.c static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret, · line 153

Quoted 7 lines

arch/powerpc/mm/slice.c +2 -1

@@ -147,7 +149,8 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
  			__set_bit(i, ret->high_slices);
  }
 
-static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_mask *ret,
+static void slice_mask_for_size(struct mm_struct *mm, int psize,
+				struct slice_mask *ret,  

That seems to be cleanup only, is it worth including that in this patch ?

Calculating the slice mask can become a signifcant overhead for get_unmapped_area. This patch adds a struct slice_mask for each page size in the mm_context, and keeps these in synch with the slices psize arrays and slb_addr_limit.

On Book3S/64 this adds 288 bytes to the mm_context_t for the slice mask caches.

On POWER8, this increases vfork+exec+exit performance by 9.9% and reduces time to mmap+munmap a 64kB page by 28%.

Reduces time to mmap+munmap by about 10% on 8xx.

Cc Benjamin Herrenschmidt <[email protected]>

Cc Anton Blanchard <[email protected]>

---
  arch/powerpc/include/asm/book3s/64/mmu.h |  18 +++++
  arch/powerpc/include/asm/mmu-8xx.h       |  14 ++++
  arch/powerpc/mm/slice.c                  | 118 ++++++++++++++++++++-----------
  3 files changed, 107 insertions(+), 43 deletions(-)

arch/powerpc/include/asm/book3s/64/mmu.h +18 -0

index bef6e39ed63a..777778579305 100644
@@ -80,6 +80,16 @@ struct spinlock;
  /* Maximum possible number of NPUs in a system. */
  #define NV_MAX_NPUS 8
 
+/*
+ * 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.
+ */
+struct slice_mask {
+	u64 low_slices;
+	DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
+};
+
  typedef struct {
  	mm_context_id_t id;
  	u16 user_psize;		/* page size index */

@@ -95,6 +105,14 @@ typedef struct {
  	unsigned char low_slices_psize[BITS_PER_LONG / BITS_PER_BYTE];
  	unsigned char high_slices_psize[SLICE_ARRAY_SIZE];
  	unsigned long slb_addr_limit;
+# ifdef CONFIG_PPC_64K_PAGES
+	struct slice_mask mask_64k;
+# endif
+	struct slice_mask mask_4k;
+# ifdef CONFIG_HUGETLB_PAGE
+	struct slice_mask mask_16m;
+	struct slice_mask mask_16g;
+# endif
  #else
  	u16 sllp;		/* SLB page size encoding */
  #endif

arch/powerpc/include/asm/mmu-8xx.h +5 -0

index d3d7e79140c6..4c3b14703b3e 100644
@@ -192,6 +192,11 @@
  #endif
 
  #ifndef __ASSEMBLY__
+struct slice_mask {
+	u64 low_slices;
+	DECLARE_BITMAP(high_slices, 0);
+};
+
  typedef struct {
  	unsigned int id;
  	unsigned int active;

arch/powerpc/include/asm/mmu-8xx.h +4 -0

+# ifdef CONFIG_HUGETLB_PAGE
+	struct slice_mask mask_512k;
+	struct slice_mask mask_8m;
+# endif
  #endif
  } mm_context_t;
 

arch/powerpc/mm/slice.c +15 -38

index 233c42d593dc..2115efe5e869 100644
@@ -37,15 +37,6 @@
  #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.
- */
-struct slice_mask {
-	u64 low_slices;
-	DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
-};
 
  #ifdef DEBUG
  int _slice_debug = 1;

@@ -149,37 +140,44 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
  			__set_bit(i, ret->high_slices);
  }
 
-static void slice_mask_for_size(struct mm_struct *mm, int psize,
-				struct slice_mask *ret,
-				unsigned long high_limit)
+#ifdef CONFIG_PPC_BOOK3S_64
+static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
  {
-	unsigned char *hpsizes, *lpsizes;
-	int index, mask_index;
-	unsigned long i;
-
-	ret->low_slices = 0;
-	if (SLICE_NUM_HIGH)
-		bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
-	lpsizes = mm->context.low_slices_psize;
-	for (i = 0; i < SLICE_NUM_LOW; i++) {
-		mask_index = i & 0x1;
-		index = i >> 1;
-		if (((lpsizes[index] >> (mask_index * 4)) & 0xf) == psize)
-			ret->low_slices |= 1u << i;
-	}
-
-	if (high_limit <= SLICE_LOW_TOP)
-		return;
-
-	hpsizes = mm->context.high_slices_psize;
-	for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++) {
-		mask_index = i & 0x1;
-		index = i >> 1;
-		if (((hpsizes[index] >> (mask_index * 4)) & 0xf) == psize)
-			__set_bit(i, ret->high_slices);
-	}
+#ifdef CONFIG_PPC_64K_PAGES
+	if (psize == MMU_PAGE_64K)
+		return &mm->context.mask_64k;
+#endif
+	if (psize == MMU_PAGE_4K)
+		return &mm->context.mask_4k;
+#ifdef CONFIG_HUGETLB_PAGE
+	if (psize == MMU_PAGE_16M)
+		return &mm->context.mask_16m;
+	if (psize == MMU_PAGE_16G)
+		return &mm->context.mask_16g;
+#endif
+	BUG();
  }

arch/powerpc/mm/slice.c +50 -5

+#ifdef CONFIG_HUGETLB_PAGE
+	if (psize == MMU_PAGE_512K)
+		return &mm->context.mask_512k;
+	if (psize == MMU_PAGE_8M)
+		return &mm->context.mask_8m;
+#endif
+	BUG();
+}
+#else
+#error "Must define the slice masks for page sizes supported by the platform"
+#endif
 
  static int slice_check_fit(struct mm_struct *mm,
  			   const struct slice_mask *mask,

@@ -226,11 +224,15 @@ static void slice_convert(struct mm_struct *mm,
  	int index, mask_index;
  	/* Write the new slice psize bits */
  	unsigned char *hpsizes, *lpsizes;
+	struct slice_mask *psize_mask, *old_mask;
  	unsigned long i, flags;
+	int old_psize;
 
  	slice_dbg("slice_convert(mm=%p, psize=%d)\n", mm, psize);
  	slice_print_mask(" mask", mask);
 
+	psize_mask = slice_mask_for_size(mm, psize);
+
  	/* We need to use a spinlock here to protect against
  	 * concurrent 64k -> 4k demotion ...
  	 */

@@ -243,6 +245,14 @@ static void slice_convert(struct mm_struct *mm,
 
  		mask_index = i & 0x1;
  		index = i >> 1;
+
+		/* Update the slice_mask */
+		old_psize = (lpsizes[index] >> (mask_index * 4)) & 0xf;
+		old_mask = slice_mask_for_size(mm, old_psize);
+		old_mask->low_slices &= ~(1u << i);
+		psize_mask->low_slices |= 1u << i;
+
+		/* Update the sizes array */
  		lpsizes[index] = (lpsizes[index] & ~(0xf << (mask_index * 4))) |
  				(((unsigned long)psize) << (mask_index * 4));
  	}

@@ -254,6 +264,14 @@ static void slice_convert(struct mm_struct *mm,
 
  		mask_index = i & 0x1;
  		index = i >> 1;
+
+		/* Update the slice_mask */
+		old_psize = (hpsizes[index] >> (mask_index * 4)) & 0xf;
+		old_mask = slice_mask_for_size(mm, old_psize);
+		__clear_bit(i, old_mask->high_slices);
+		__set_bit(i, psize_mask->high_slices);
+
+		/* Update the sizes array */
  		hpsizes[index] = (hpsizes[index] & ~(0xf << (mask_index * 4))) |
  				(((unsigned long)psize) << (mask_index * 4));
  	}

@@ -464,7 +482,13 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	}
 
  	if (high_limit > mm->context.slb_addr_limit) {
+		/*
+		 * Increasing the slb_addr_limit does not require
+		 * slice mask cache to be recalculated because it should
+		 * be already initialised beyond the old address limit.
+		 */
  		mm->context.slb_addr_limit = high_limit;
+
  		on_each_cpu(slice_flush_segments, mm, 1);
  	}
 

@@ -506,7 +530,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	/* First make up a "good" mask of slices that have the right size
  	 * already
  	 */
-	slice_mask_for_size(mm, psize, &good_mask, high_limit);
+	good_mask = *slice_mask_for_size(mm, psize);
  	slice_print_mask(" good_mask", &good_mask);
 
  	/*

@@ -531,7 +555,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  #ifdef CONFIG_PPC_64K_PAGES
  	/* If we support combo pages, we can allow 64k pages in 4k slices */
  	if (psize == MMU_PAGE_64K) {
-		slice_mask_for_size(mm, MMU_PAGE_4K, &compat_mask, high_limit);
+		compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
  		if (fixed)
  			slice_or_mask(&good_mask, &compat_mask);
  	}

@@ -683,6 +707,7 @@ EXPORT_SYMBOL_GPL(get_slice_psize);
  void slice_init_new_context_exec(struct mm_struct *mm)
  {
  	unsigned char *hpsizes, *lpsizes;
+	struct slice_mask *mask;
  	unsigned int psize = mmu_virtual_psize;
 
  	slice_dbg("slice_init_new_context_exec(mm=%p)\n", mm);

@@ -703,6 +728,14 @@ void slice_init_new_context_exec(struct mm_struct *mm)
 
  	hpsizes = mm->context.high_slices_psize;
  	memset(hpsizes, (psize << 4) | psize, SLICE_NUM_HIGH >> 1);
+
+	/*
+	 * Slice mask cache starts zeroed, fill the default size cache.
+	 */
+	mask = slice_mask_for_size(mm, psize);
+	mask->low_slices = ~0UL;
+	if (SLICE_NUM_HIGH)
+		bitmap_fill(mask->high_slices, SLICE_NUM_HIGH);
  }
 
  void slice_set_range_psize(struct mm_struct *mm, unsigned long start,

@@ -741,18 +774,17 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  {
  	struct slice_mask mask, available;
  	unsigned int psize = mm->context.user_psize;
-	unsigned long high_limit = mm->context.slb_addr_limit;
 
  	if (radix_enabled())
  		return 0;
 
  	slice_range_to_mask(addr, len, &mask);
-	slice_mask_for_size(mm, psize, &available, high_limit);
+	available = *slice_mask_for_size(mm, psize);
  #ifdef CONFIG_PPC_64K_PAGES
  	/* We need to account for 4k slices too */
  	if (psize == MMU_PAGE_64K) {
  		struct slice_mask compat_mask;
-		slice_mask_for_size(mm, MMU_PAGE_4K, &compat_mask, high_limit);
+		compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
  		slice_or_mask(&available, &compat_mask);
  	}
  #endif

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

arch/powerpc/include/asm/mmu-8xx.h typedef struct { · line 213

Quoted 9 lines

arch/powerpc/include/asm/mmu-8xx.h +5 -0

@@ -201,6 +206,15 @@ typedef struct {
  	unsigned char low_slices_psize[SLICE_ARRAY_SIZE];
  	unsigned char high_slices_psize[0];
  	unsigned long slb_addr_limit;
+# ifdef CONFIG_PPC_16K_PAGES
+	struct slice_mask mask_16k;
+# else
+	struct slice_mask mask_4k;
+# endif  

Could we just call it mask_base or something like that regardless of the standard page size ?

arch/powerpc/mm/slice.c static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret, · line 169

Quoted 10 lines

arch/powerpc/mm/slice.c +10 -0

+#elif defined(CONFIG_PPC_8xx)
+static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
+{
+#ifdef CONFIG_PPC_16K_PAGES
+	if (psize == MMU_PAGE_16K)
+		return &mm->context.mask_16k;
+#else
+	if (psize == MMU_PAGE_4K)
+		return &mm->context.mask_4k;
+#endif  

What about the following instead:

Rather than build slice masks from a range then use that to check for fit in a candidate mask, implement slice_check_range_fits that checks if a range fits in a mask directly.

This allows several structures to be removed from stacks, and also we don't expect a huge range in a lot of these cases, so building and comparing a full mask is going to be more expensive than testing just one or two bits of the range.

On POWER8, this increases vfork+exec+exit performance by 0.3% and reduces time to mmap+munmap a 64kB page by 5%.

Signed-off-by Nicholas Piggin <[email protected]>

---
  arch/powerpc/mm/slice.c | 63 ++++++++++++++++++++++++++-----------------------
  1 file changed, 34 insertions(+), 29 deletions(-)

arch/powerpc/mm/slice.c +3 -7

index 2115efe5e869..3841fca75006 100644
@@ -179,26 +179,35 @@ static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
  #error "Must define the slice masks for page sizes supported by the platform"
  #endif
 
-static int slice_check_fit(struct mm_struct *mm,
-			   const struct slice_mask *mask,
-			   const struct slice_mask *available)
+static bool slice_check_range_fits(struct mm_struct *mm,
+			   const struct slice_mask *available,
+			   unsigned long start, unsigned long len)
  {
-	DECLARE_BITMAP(result, SLICE_NUM_HIGH);
-	/*
-	 * Make sure we just do bit compare only to the max
-	 * addr limit and not the full bit map size.

arch/powerpc/mm/slice.c +9 -0

+
+		low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
+				- (1u << GET_LOW_SLICE_INDEX(start));
+	}
+	if ((low_slices & available->low_slices) != low_slices)
+		return false;
+
+	if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
+		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);

arch/powerpc/mm/slice.c +2 -2

+	}
 
-	return (mask->low_slices & available->low_slices) == mask->low_slices &&
-		bitmap_equal(result, mask->high_slices, slice_count);
+	return true;
  }
 
  static void slice_flush_segments(void *parm)

arch/powerpc/mm/slice.c +1 -1

  		/* Check if we fit in the good mask. If we do, we just return,
  		 * nothing else to do
  		 */
-		if (slice_check_fit(mm, &mask, &good_mask)) {
+		if (slice_check_range_fits(mm, &good_mask, addr, len)) {
  			slice_dbg(" fits good !\n");
  			return addr;
  		}

@@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	slice_or_mask(&potential_mask, &good_mask);
  	slice_print_mask(" potential", &potential_mask);

arch/powerpc/mm/slice.c +2 -3

  	}
 
  	/* If we have MAP_FIXED and failed the above steps, then error out */

@@ -772,13 +778,12 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
  int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  			   unsigned long len)
  {
-	struct slice_mask mask, available;
+	struct slice_mask available;
  	unsigned int psize = mm->context.user_psize;
 
  	if (radix_enabled())
  		return 0;
 
-	slice_range_to_mask(addr, len, &mask);
  	available = *slice_mask_for_size(mm, psize);
  #ifdef CONFIG_PPC_64K_PAGES
  	/* We need to account for 4k slices too */

@@ -795,6 +800,6 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  	slice_print_mask(" mask", &mask);
  	slice_print_mask(" available", &available);
  #endif
-	return !slice_check_fit(mm, &mask, &available);
+	return !slice_check_range_fits(mm, &available, addr, len);
  }
  #endif

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

Quoted 8 lines

arch/powerpc/mm/slice.c +3 -4

+static bool slice_check_range_fits(struct mm_struct *mm,
+			   const struct slice_mask *available,
+			   unsigned long start, unsigned long len)
  {
-	DECLARE_BITMAP(result, SLICE_NUM_HIGH);
-	/*
-	 * Make sure we just do bit compare only to the max
-	 * addr limit and not the full bit map size.

arch/powerpc/mm/slice.c static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize) · line 190

Quoted 10 lines

arch/powerpc/mm/slice.c +4 -5

-	 */
-	unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+	unsigned long end = start + len - 1;
+	u64 low_slices = 0;
 
-	if (!SLICE_NUM_HIGH)
-		return (mask->low_slices & available->low_slices) ==
-		       mask->low_slices;
+	if (start < SLICE_LOW_TOP) {
+		unsigned long mend = min(end, (SLICE_LOW_TOP - 1));  

See slice_range_to_mask()

You'll have an issue here with PPC32, you have to cast (SLICE_LOW_TOP -
1) to unsigned long because SLICE_LOW_TOP is unsigned long long on PPC32

Quoted 13 lines

arch/powerpc/mm/slice.c +12 -0

+
+		low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
+				- (1u << GET_LOW_SLICE_INDEX(start));
+	}
+	if ((low_slices & available->low_slices) != low_slices)
+		return false;
+
+	if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
+		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
+		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
+		unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
+		unsigned long i;

arch/powerpc/mm/slice.c static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize) · line 207

Quoted 6 lines

arch/powerpc/mm/slice.c +4 -2

-	bitmap_and(result, mask->high_slices,
-		   available->high_slices, slice_count);
+		for (i = start_index; i < start_index + count; i++) {
+			if (!test_bit(i, available->high_slices))
+				return false;
+		}  

What about using bitmap_find_next_zero_area()

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 574

Quoted 10 lines

arch/powerpc/mm/slice.c +1 -5

@@ -562,15 +571,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  #endif
 
  	/* First check hint if it's valid or if we have MAP_FIXED */
-	if (addr != 0 || fixed) {
-		/* Build a mask for the requested range */
-		slice_range_to_mask(addr, len, &mask);
-		slice_print_mask(" mask", &mask);
-
+	if (addr || fixed) {  

It is cleanup, should it really be part of this patch ?

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 608

Quoted 13 lines

arch/powerpc/mm/slice.c +5 -4

@@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	slice_or_mask(&potential_mask, &good_mask);
  	slice_print_mask(" potential", &potential_mask);
 
-	if ((addr != 0 || fixed) &&
-			slice_check_fit(mm, &mask, &potential_mask)) {
-		slice_dbg(" fits potential !\n");
-		goto convert;
+	if (addr || fixed) {
+		if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
+			slice_dbg(" fits potential !\n");
+			goto convert;
+		}  

Why not keep the original structure and just replacing slice_check_fit() by slice_check_range_fits() ?

I believe cleanups should not be mixed with real feature changes. If needed, you should have a cleanup patch up front the serie.

Quoted 24 lines · On Tue, 6 Mar 2018 15:41:00 +0100 Christophe LEROY <[email protected]>…

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

Quoted 8 lines

arch/powerpc/mm/slice.c +3 -4

+static bool slice_check_range_fits(struct mm_struct *mm,
+			   const struct slice_mask *available,
+			   unsigned long start, unsigned long len)
   {
-	DECLARE_BITMAP(result, SLICE_NUM_HIGH);
-	/*
-	 * Make sure we just do bit compare only to the max
-	 * addr limit and not the full bit map size.

arch/powerpc/mm/slice.c static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize) · line 190

Quoted 10 lines

arch/powerpc/mm/slice.c +4 -5

-	 */
-	unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+	unsigned long end = start + len - 1;
+	u64 low_slices = 0;
 
-	if (!SLICE_NUM_HIGH)
-		return (mask->low_slices & available->low_slices) ==
-		       mask->low_slices;
+	if (start < SLICE_LOW_TOP) {
+		unsigned long mend = min(end, (SLICE_LOW_TOP - 1));

See slice_range_to_mask()

Quoted 2 lines

You'll have an issue here with PPC32, you have to cast (SLICE_LOW_TOP -
1) to unsigned long because SLICE_LOW_TOP is unsigned long long on PPC32

Okay thanks. Forgot to cross compiled it on 8xx, so I'll do that next time.

Quoted 13 lines

Quoted 13 lines

arch/powerpc/mm/slice.c +12 -0

+
+		low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
+				- (1u << GET_LOW_SLICE_INDEX(start));
+	}
+	if ((low_slices & available->low_slices) != low_slices)
+		return false;
+
+	if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
+		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
+		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
+		unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
+		unsigned long i;

Quoted 8 lines

arch/powerpc/mm/slice.c static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize) · line 207

Quoted 6 lines

arch/powerpc/mm/slice.c +4 -2

-	bitmap_and(result, mask->high_slices,
-		   available->high_slices, slice_count);
+		for (i = start_index; i < start_index + count; i++) {
+			if (!test_bit(i, available->high_slices))
+				return false;
+		}

What about using bitmap_find_next_zero_area()

I'll look at it. Perhaps in another patch, because existing loops are not using bitmap range operations either. A series to convert those is a good idea.

Quoted 32 lines

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 574

Quoted 10 lines

arch/powerpc/mm/slice.c +1 -5

@@ -562,15 +571,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
   #endif
 
   	/* First check hint if it's valid or if we have MAP_FIXED */
-	if (addr != 0 || fixed) {
-		/* Build a mask for the requested range */
-		slice_range_to_mask(addr, len, &mask);
-		slice_print_mask(" mask", &mask);
-
+	if (addr || fixed) {

It is cleanup, should it really be part of this patch ?

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 608

Quoted 13 lines

arch/powerpc/mm/slice.c +5 -4

@@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
   	slice_or_mask(&potential_mask, &good_mask);
   	slice_print_mask(" potential", &potential_mask);
 
-	if ((addr != 0 || fixed) &&
-			slice_check_fit(mm, &mask, &potential_mask)) {
-		slice_dbg(" fits potential !\n");
-		goto convert;
+	if (addr || fixed) {
+		if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
+			slice_dbg(" fits potential !\n");
+			goto convert;
+		}

Why not keep the original structure and just replacing slice_check_fit() by slice_check_range_fits() ?

Quoted 2 lines

I believe cleanups should not be mixed with real feature changes. If needed, you should have a cleanup patch up front the serie.

For code that is already changing, I think minor cleanups are okay if they're very simple. Maybe this is getting to the point of needing another patch. You've made valid points for a lot of other unnecessary cleanups though, so I'll fix all of those.

Le 07/03/2018 à 00:12, Nicholas Piggin a écrit :

Quoted 25 lines

Quoted 21 lines · On Tue, 6 Mar 2018 15:41:00 +0100 Christophe LEROY <[email protected]>…

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 608

Quoted 13 lines

arch/powerpc/mm/slice.c +5 -4

@@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
   	slice_or_mask(&potential_mask, &good_mask);
   	slice_print_mask(" potential", &potential_mask);
 
-	if ((addr != 0 || fixed) &&
-			slice_check_fit(mm, &mask, &potential_mask)) {
-		slice_dbg(" fits potential !\n");
-		goto convert;
+	if (addr || fixed) {
+		if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
+			slice_dbg(" fits potential !\n");
+			goto convert;
+		}  

Why not keep the original structure and just replacing slice_check_fit() by slice_check_range_fits() ?

I believe cleanups should not be mixed with real feature changes. If needed, you should have a cleanup patch up front the serie.

Quoted 4 lines

For code that is already changing, I think minor cleanups are okay if they're very simple. Maybe this is getting to the point of needing another patch. You've made valid points for a lot of other unnecessary cleanups though, so I'll fix all of those.

Ok, that's not a big point, but I like when patches really modifies only the lines they need to modify.

Quoted 26 lines · On Wed, 7 Mar 2018 07:12:23 +0100 Christophe LEROY <[email protected]>…

Le 07/03/2018 à 00:12, Nicholas Piggin a écrit :

Quoted 25 lines

Quoted 21 lines · On Tue, 6 Mar 2018 15:41:00 +0100 Christophe LEROY <[email protected]>…

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 608

Quoted 13 lines

arch/powerpc/mm/slice.c +5 -4

@@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
    	slice_or_mask(&potential_mask, &good_mask);
    	slice_print_mask(" potential", &potential_mask);
 
-	if ((addr != 0 || fixed) &&
-			slice_check_fit(mm, &mask, &potential_mask)) {
-		slice_dbg(" fits potential !\n");
-		goto convert;
+	if (addr || fixed) {
+		if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
+			slice_dbg(" fits potential !\n");
+			goto convert;
+		}

Why not keep the original structure and just replacing slice_check_fit() by slice_check_range_fits() ?

I believe cleanups should not be mixed with real feature changes. If needed, you should have a cleanup patch up front the serie.

Quoted 7 lines

Quoted 4 lines

For code that is already changing, I think minor cleanups are okay if they're very simple. Maybe this is getting to the point of needing another patch. You've made valid points for a lot of other unnecessary cleanups though, so I'll fix all of those.

Ok, that's not a big point, but I like when patches really modifies only the lines they need to modify.

Fair point, and in the end I agree mostly they should do that. But I don't think entirely if you can make the code slightly better as you go (again, so long as the change is obvious). I think having extra patches for trivial cleanups is not that great either.

Quoted 5 lines

Why do we need a double if ?

Why not just the following ? With proper alignment of the second line with the open parenthese, it fits in one line

Review

Quoted 5 lines

	if ((addr != 0 || fixed) &&

(quoted diff) +1 -1

-			slice_check_fit(mm, &mask, &potential_mask)) {
+	    slice_check_range_fits(mm, &potential_mask, addr, len)) {
   		slice_dbg(" fits potential !\n");
   		goto convert;

For this case the main motivation was to make this test match the form of the same test (with different mask) above here. Doing the same thing with different coding styles annoys me.

I think I kept this one but fixed all your other suggestions in the v2 series.

Thanks,
Nick

This converts the slice_mask bit operation helpers to be the usual 3-operand kind, which is clearer to work with.

Signed-off-by Nicholas Piggin <[email protected]>

---
  arch/powerpc/mm/slice.c | 38 +++++++++++++++++++++++---------------
  1 file changed, 23 insertions(+), 15 deletions(-)

arch/powerpc/mm/slice.c +0 -0

index 3841fca75006..46daa1d1794f 100644

arch/powerpc/mm/slice.c +22 -14

  {
-	dst->low_slices |= src->low_slices;
+	dst->low_slices = src->low_slices;
  	if (!SLICE_NUM_HIGH)
  		return;
-	bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
-		  SLICE_NUM_HIGH);
+	bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
  }
 
-static inline void slice_andnot_mask(struct slice_mask *dst,
-					const struct slice_mask *src)
+static inline void slice_or_mask(struct slice_mask *dst,
+					const struct slice_mask *src1,
+					const struct slice_mask *src2)
  {
-	dst->low_slices &= ~src->low_slices;
+	dst->low_slices = src1->low_slices | src2->low_slices;
+	if (!SLICE_NUM_HIGH)
+		return;
+	bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
+}
 
+static inline void slice_andnot_mask(struct slice_mask *dst,
+					const struct slice_mask *src1,
+					const struct slice_mask *src2)
+{
+	dst->low_slices = src1->low_slices & ~src2->low_slices;
  	if (!SLICE_NUM_HIGH)
  		return;
-	bitmap_andnot(dst->high_slices, dst->high_slices, src->high_slices,
-		      SLICE_NUM_HIGH);
+	bitmap_andnot(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
  }
 
  #ifdef CONFIG_PPC_64K_PAGES

@@ -566,7 +574,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	if (psize == MMU_PAGE_64K) {
  		compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
  		if (fixed)
-			slice_or_mask(&good_mask, &compat_mask);
+			slice_or_mask(&good_mask, &good_mask, &compat_mask);
  	}
  #endif
 

@@ -598,7 +606,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	 * empty and thus can be converted
  	 */
  	slice_mask_for_free(mm, &potential_mask, high_limit);
-	slice_or_mask(&potential_mask, &good_mask);
+	slice_or_mask(&potential_mask, &potential_mask, &good_mask);
  	slice_print_mask(" potential", &potential_mask);
 
  	if (addr || fixed) {

@@ -635,7 +643,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  #ifdef CONFIG_PPC_64K_PAGES
  	if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
  		/* retry the search with 4k-page slices included */
-		slice_or_mask(&potential_mask, &compat_mask);
+		slice_or_mask(&potential_mask, &potential_mask, &compat_mask);
  		addr = slice_find_area(mm, len, &potential_mask,
  				       psize, topdown, high_limit);
  	}

@@ -649,8 +657,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	slice_print_mask(" mask", &mask);
 
   convert:
-	slice_andnot_mask(&mask, &good_mask);
-	slice_andnot_mask(&mask, &compat_mask);
+	slice_andnot_mask(&mask, &mask, &good_mask);
+	slice_andnot_mask(&mask, &mask, &compat_mask);
  	if (mask.low_slices ||
  	    (SLICE_NUM_HIGH &&
  	     !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {

@@ -790,7 +798,7 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  	if (psize == MMU_PAGE_64K) {
  		struct slice_mask compat_mask;
  		compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
-		slice_or_mask(&available, &compat_mask);
+		slice_or_mask(&available, &available, &compat_mask);
  	}
  #endif
 

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

Quoted 2 lines

This converts the slice_mask bit operation helpers to be the usual 3-operand kind, which is clearer to work with.

What's the real benefit of doing that ?

It is helps for a subsequent patch, say it.

arch/powerpc/mm/slice.c static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len, · line 437

Quoted 7 lines

arch/powerpc/mm/slice.c +1 -1

@@ -433,25 +433,33 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
  		return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
  }
 
-static inline void slice_or_mask(struct slice_mask *dst,
+static inline void slice_copy_mask(struct slice_mask *dst,
  					const struct slice_mask *src)  

This new function is not used, the compiler while probably not be happy.

The slice_mask cache was a basic conversion which copied the slice mask into caller's structures, because that's how the original code worked. In most cases the pointer can be used directly instead, saving a copy and an on-stack structure.

On POWER8, this increases vfork+exec+exit performance by 0.3% and reduces time to mmap+munmap a 64kB page by 2%.

Signed-off-by Nicholas Piggin <[email protected]>

---
  arch/powerpc/mm/slice.c | 77 +++++++++++++++++++++----------------------------
  1 file changed, 33 insertions(+), 44 deletions(-)

arch/powerpc/mm/slice.c +4 -21

index 46daa1d1794f..086c31b8b982 100644
@@ -472,10 +472,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  				      unsigned long flags, unsigned int psize,
  				      int topdown)
  {
-	struct slice_mask mask;
  	struct slice_mask good_mask;
  	struct slice_mask potential_mask;
-	struct slice_mask compat_mask;
+	const struct slice_mask *maskp;
+	const struct slice_mask *compat_maskp = NULL;
  	int fixed = (flags & MAP_FIXED);
  	int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
  	unsigned long page_size = 1UL << pshift;

@@ -509,22 +509,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  		on_each_cpu(slice_flush_segments, mm, 1);
  	}
 
-	/*
-	 * init different masks
-	 */
-	mask.low_slices = 0;
-
-	/* silence stupid warning */;
-	potential_mask.low_slices = 0;
-
-	compat_mask.low_slices = 0;
-
-	if (SLICE_NUM_HIGH) {
-		bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
-		bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
-		bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
-	}
-
  	/* Sanity checks */
  	BUG_ON(mm->task_size == 0);
  	BUG_ON(mm->context.slb_addr_limit == 0);

@@ -547,8 +531,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	/* First make up a "good" mask of slices that have the right size
  	 * already
  	 */
-	good_mask = *slice_mask_for_size(mm, psize);
-	slice_print_mask(" good_mask", &good_mask);
+	maskp = slice_mask_for_size(mm, psize);
 
  	/*
  	 * Here "good" means slices that are already the right page size,

@@ -572,11 +555,19 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  #ifdef CONFIG_PPC_64K_PAGES
  	/* If we support combo pages, we can allow 64k pages in 4k slices */
  	if (psize == MMU_PAGE_64K) {
-		compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
+		compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
  		if (fixed)

arch/powerpc/mm/slice.c +20 -14

+	slice_print_mask(" good_mask", &good_mask);
+	if (compat_maskp)
+		slice_print_mask(" compat_mask", compat_maskp);
 
  	/* First check hint if it's valid or if we have MAP_FIXED */
  	if (addr || fixed) {

@@ -643,7 +634,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  #ifdef CONFIG_PPC_64K_PAGES
  	if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
  		/* retry the search with 4k-page slices included */
-		slice_or_mask(&potential_mask, &potential_mask, &compat_mask);
+		slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
  		addr = slice_find_area(mm, len, &potential_mask,
  				       psize, topdown, high_limit);
  	}

@@ -652,17 +643,18 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	if (addr == -ENOMEM)
  		return -ENOMEM;
 
-	slice_range_to_mask(addr, len, &mask);
+	slice_range_to_mask(addr, len, &potential_mask);
  	slice_dbg(" found potential area at 0x%lx\n", addr);
-	slice_print_mask(" mask", &mask);
+	slice_print_mask(" mask", &potential_mask);
 
   convert:
-	slice_andnot_mask(&mask, &mask, &good_mask);
-	slice_andnot_mask(&mask, &mask, &compat_mask);
-	if (mask.low_slices ||
-	    (SLICE_NUM_HIGH &&
-	     !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
-		slice_convert(mm, &mask, psize);
+	slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
+	if (compat_maskp && !fixed)
+		slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
+	if (potential_mask.low_slices ||
+		(SLICE_NUM_HIGH &&
+		 !bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH))) {
+		slice_convert(mm, &potential_mask, psize);
  		if (psize > MMU_PAGE_BASE)
  			on_each_cpu(slice_flush_segments, mm, 1);
  	}

@@ -786,28 +778,25 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
  int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  			   unsigned long len)
  {
-	struct slice_mask available;
+	const struct slice_mask *maskp;
  	unsigned int psize = mm->context.user_psize;
 
  	if (radix_enabled())
  		return 0;
 
-	available = *slice_mask_for_size(mm, psize);
+	maskp = slice_mask_for_size(mm, psize);
  #ifdef CONFIG_PPC_64K_PAGES
  	/* We need to account for 4k slices too */
  	if (psize == MMU_PAGE_64K) {
-		struct slice_mask compat_mask;
-		compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
-		slice_or_mask(&available, &available, &compat_mask);
+		const struct slice_mask *compat_maskp;
+		struct slice_mask available;
+
+		compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
+		slice_or_mask(&available, maskp, compat_maskp);

arch/powerpc/mm/slice.c +1 -1

-	return !slice_check_range_fits(mm, &available, addr, len);
+	return !slice_check_range_fits(mm, maskp, addr, len);
  }
  #endif

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

Quoted 7 lines

arch/powerpc/mm/slice.c +1 -1

@@ -572,11 +555,19 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  #ifdef CONFIG_PPC_64K_PAGES
  	/* If we support combo pages, we can allow 64k pages in 4k slices */
  	if (psize == MMU_PAGE_64K) {
-		compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
+		compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
  		if (fixed)

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 567

Quoted 10 lines

arch/powerpc/mm/slice.c +7 -2

-			slice_or_mask(&good_mask, &good_mask, &compat_mask);
-	}
+			slice_or_mask(&good_mask, maskp, compat_maskp);
+		else
+			slice_copy_mask(&good_mask, maskp);
+	} else
  #endif
+	{
+		slice_copy_mask(&good_mask, maskp);
+	}  

You could get something nicer by removing that #ifdef and doing instead:

	if (IS_ENABLED(CONFIG_PPC_64K_PAGES) && psize == MMU_PAGE_64K) {
		...
	} else {
		slice_copy_mask(&good_mask, maskp);
	}

arch/powerpc/mm/slice.c void slice_set_range_psize(struct mm_struct *mm, unsigned long start, · line 810

Quoted 7 lines

arch/powerpc/mm/slice.c +0 -6

-#if 0 /* too verbose */
-	slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
-		 mm, addr, len);
-	slice_print_mask(" mask", &mask);
-	slice_print_mask(" available", &available);
-#endif  

That's cleanup, should be in a previous patch.

The number of high slices a process might use now depends on its address space size, and what allocation address it has requested.

This patch uses that limit throughout call chains where possible, rather than use the fixed SLICE_NUM_HIGH for bitmap operations. This saves some cost for processes that don't use very large address spaces.

Perormance numbers aren't changed significantly, this may change with larger address spaces or different mmap access patterns that require more slice mask building.

Signed-off-by Nicholas Piggin <[email protected]>

---
  arch/powerpc/mm/slice.c | 75 +++++++++++++++++++++++++++++--------------------
  1 file changed, 45 insertions(+), 30 deletions(-)

arch/powerpc/mm/slice.c +2 -4

index 086c31b8b982..507d17e2cfcd 100644
@@ -61,14 +61,12 @@ static void slice_print_mask(const char *label, const struct slice_mask *mask) {
  #endif
 
  static void slice_range_to_mask(unsigned long start, unsigned long len,
-				struct slice_mask *ret)
+				struct slice_mask *ret,
+				unsigned long high_slices)
  {
  	unsigned long end = start + len - 1;
 
  	ret->low_slices = 0;
-	if (SLICE_NUM_HIGH)
-		bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
  	if (start < SLICE_LOW_TOP) {
  		unsigned long mend = min(end,
  					 (unsigned long)(SLICE_LOW_TOP - 1));

arch/powerpc/mm/slice.c +10 -9

  	if ((start + len) > SLICE_LOW_TOP) {
  		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
  		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));

@@ -120,22 +122,20 @@ static int slice_high_has_vma(struct mm_struct *mm, unsigned long slice)
  }
 
  static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
-				unsigned long high_limit)
+				unsigned long high_slices)
  {
  	unsigned long i;
 
  	ret->low_slices = 0;
-	if (SLICE_NUM_HIGH)
-		bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
  	for (i = 0; i < SLICE_NUM_LOW; i++)
  		if (!slice_low_has_vma(mm, i))
  			ret->low_slices |= 1u << i;
 
-	if (high_limit <= SLICE_LOW_TOP)
+	if (!SLICE_NUM_HIGH || !high_slices)
  		return;
 
-	for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
+	bitmap_zero(ret->high_slices, high_slices);
+	for (i = 0; i < high_slices; i++)
  		if (!slice_high_has_vma(mm, i))
  			__set_bit(i, ret->high_slices);
  }

@@ -232,6 +232,7 @@ static void slice_convert(struct mm_struct *mm,
  {
  	int index, mask_index;
  	/* Write the new slice psize bits */
+	unsigned long high_slices;
  	unsigned char *hpsizes, *lpsizes;
  	struct slice_mask *psize_mask, *old_mask;
  	unsigned long i, flags;

@@ -267,7 +268,8 @@ static void slice_convert(struct mm_struct *mm,
  	}
 
  	hpsizes = mm->context.high_slices_psize;
-	for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
+	high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+	for (i = 0; SLICE_NUM_HIGH && i < high_slices; i++) {
  		if (!test_bit(i, mask->high_slices))
  			continue;
 

@@ -434,32 +436,37 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
  }
 
  static inline void slice_copy_mask(struct slice_mask *dst,
-					const struct slice_mask *src)
+					const struct slice_mask *src,
+					unsigned long high_slices)
  {
  	dst->low_slices = src->low_slices;
  	if (!SLICE_NUM_HIGH)
  		return;
-	bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
+	bitmap_copy(dst->high_slices, src->high_slices, high_slices);
  }
 
  static inline void slice_or_mask(struct slice_mask *dst,
  					const struct slice_mask *src1,

arch/powerpc/mm/slice.c +0 -0

  }
 
  static inline void slice_andnot_mask(struct slice_mask *dst,
  					const struct slice_mask *src1,

arch/powerpc/mm/slice.c +9 -7

  }
 
  #ifdef CONFIG_PPC_64K_PAGES

@@ -482,6 +489,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	struct mm_struct *mm = current->mm;
  	unsigned long newaddr;
  	unsigned long high_limit;
+	unsigned long high_slices;
 
  	high_limit = DEFAULT_MAP_WINDOW;
  	if (addr >= high_limit || (fixed && (addr + len > high_limit)))

@@ -498,6 +506,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  			return -ENOMEM;
  	}
 
+	high_slices = GET_HIGH_SLICE_INDEX(high_limit);
  	if (high_limit > mm->context.slb_addr_limit) {
  		/*
  		 * Increasing the slb_addr_limit does not require

@@ -557,13 +566,13 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	if (psize == MMU_PAGE_64K) {
  		compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
  		if (fixed)
-			slice_or_mask(&good_mask, maskp, compat_maskp);
+			slice_or_mask(&good_mask, maskp, compat_maskp, high_slices);
  		else
-			slice_copy_mask(&good_mask, maskp);
+			slice_copy_mask(&good_mask, maskp, high_slices);
  	} else
  #endif
  	{
-		slice_copy_mask(&good_mask, maskp);
+		slice_copy_mask(&good_mask, maskp, high_slices);
  	}
  	slice_print_mask(" good_mask", &good_mask);
  	if (compat_maskp)

@@ -596,8 +605,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	 * We don't fit in the good mask, check what other slices are
  	 * empty and thus can be converted
  	 */
-	slice_mask_for_free(mm, &potential_mask, high_limit);
-	slice_or_mask(&potential_mask, &potential_mask, &good_mask);
+	slice_mask_for_free(mm, &potential_mask, high_slices);
+	slice_or_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
  	slice_print_mask(" potential", &potential_mask);
 
  	if (addr || fixed) {

@@ -634,7 +643,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  #ifdef CONFIG_PPC_64K_PAGES
  	if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
  		/* retry the search with 4k-page slices included */
-		slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
+		slice_or_mask(&potential_mask, &potential_mask, compat_maskp, high_slices);
  		addr = slice_find_area(mm, len, &potential_mask,
  				       psize, topdown, high_limit);
  	}

@@ -643,17 +652,17 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	if (addr == -ENOMEM)
  		return -ENOMEM;
 
-	slice_range_to_mask(addr, len, &potential_mask);
+	slice_range_to_mask(addr, len, &potential_mask, high_slices);
  	slice_dbg(" found potential area at 0x%lx\n", addr);
  	slice_print_mask(" mask", &potential_mask);
 

arch/powerpc/mm/slice.c +9 -3

  		slice_convert(mm, &potential_mask, psize);
  		if (psize > MMU_PAGE_BASE)
  			on_each_cpu(slice_flush_segments, mm, 1);

@@ -727,7 +736,9 @@ void slice_init_new_context_exec(struct mm_struct *mm)
  	mm->context.user_psize = psize;
 
  	/*
-	 * Set all slice psizes to the default.
+	 * Set all slice psizes to the default. High slices could
+	 * be initialised up to slb_addr_limit if we ensure to
+	 * initialise the rest of them as slb_addr_limit is expanded.
  	 */
  	lpsizes = mm->context.low_slices_psize;
  	memset(lpsizes, (psize << 4) | psize, SLICE_NUM_LOW >> 1);

@@ -748,10 +759,12 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
  			   unsigned long len, unsigned int psize)
  {
  	struct slice_mask mask;
+	unsigned long high_slices;
 
  	VM_BUG_ON(radix_enabled());
 
-	slice_range_to_mask(start, len, &mask);
+	high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+	slice_range_to_mask(start, len, &mask, high_slices);
  	slice_convert(mm, &mask, psize);
  }
 

@@ -790,9 +803,11 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  	if (psize == MMU_PAGE_64K) {
  		const struct slice_mask *compat_maskp;
  		struct slice_mask available;
+		unsigned long high_slices;
 
  		compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
-		slice_or_mask(&available, maskp, compat_maskp);
+		high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+		slice_or_mask(&available, maskp, compat_maskp, high_slices);
  		return !slice_check_range_fits(mm, &available, addr, len);
  	}
  #endif

Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :

Quoted 39 lines

The number of high slices a process might use now depends on its address space size, and what allocation address it has requested.

This patch uses that limit throughout call chains where possible, rather than use the fixed SLICE_NUM_HIGH for bitmap operations. This saves some cost for processes that don't use very large address spaces.

Perormance numbers aren't changed significantly, this may change with larger address spaces or different mmap access patterns that require more slice mask building.

Signed-off-by Nicholas Piggin <[email protected]>

---
  arch/powerpc/mm/slice.c | 75 +++++++++++++++++++++++++++++--------------------
  1 file changed, 45 insertions(+), 30 deletions(-)

arch/powerpc/mm/slice.c +2 -4

index 086c31b8b982..507d17e2cfcd 100644
@@ -61,14 +61,12 @@ static void slice_print_mask(const char *label, const struct slice_mask *mask) {
  #endif
 
  static void slice_range_to_mask(unsigned long start, unsigned long len,
-				struct slice_mask *ret)
+				struct slice_mask *ret,
+				unsigned long high_slices)
  {
  	unsigned long end = start + len - 1;
 
  	ret->low_slices = 0;
-	if (SLICE_NUM_HIGH)
-		bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
  	if (start < SLICE_LOW_TOP) {
  		unsigned long mend = min(end,
  					 (unsigned long)(SLICE_LOW_TOP - 1));

arch/powerpc/mm/slice.c static void slice_range_to_mask(unsigned long start, unsigned long len, · line 81

Quoted 7 lines

arch/powerpc/mm/slice.c +4 -0

@@ -77,6 +75,10 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
  			- (1u << GET_LOW_SLICE_INDEX(start));
  	}
 
+	if (!SLICE_NUM_HIGH)
+		return;
+
+	bitmap_zero(ret->high_slices, high_slices);  

In include/linux/bitmap.h, it is said:

* Note that nbits should be always a compile time evaluable constant.
* Otherwise many inlines will generate horrible code.

Not sure that's true, but it is written ...

Quoted 7 lines

arch/powerpc/mm/slice.c +2 -1

  static inline void slice_or_mask(struct slice_mask *dst,
  					const struct slice_mask *src1,
-					const struct slice_mask *src2)
+					const struct slice_mask *src2,
+					unsigned long high_slices)
  {
  	dst->low_slices = src1->low_slices | src2->low_slices;

arch/powerpc/mm/slice.c static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len, · line 457

Quoted 5 lines

if (!SLICE_NUM_HIGH)
return;

arch/powerpc/mm/slice.c +2 -1

-	bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
+	bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices,
+			high_slices);  

Why a new line here, this line is shorter than before. Or that was forgotten in a previous patch ?

Quoted 9 lines

arch/powerpc/mm/slice.c +1 -1

@@ -643,17 +652,17 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  	if (addr == -ENOMEM)
  		return -ENOMEM;
 
-	slice_range_to_mask(addr, len, &potential_mask);
+	slice_range_to_mask(addr, len, &potential_mask, high_slices);
  	slice_dbg(" found potential area at 0x%lx\n", addr);
  	slice_print_mask(" mask", &potential_mask);
 

arch/powerpc/mm/slice.c unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len, · line 665

Quoted 10 lines

arch/powerpc/mm/slice.c +3 -3

   convert:
-	slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
+	slice_andnot_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
  	if (compat_maskp && !fixed)
-		slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
+		slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp, high_slices);
  	if (potential_mask.low_slices ||
  		(SLICE_NUM_HIGH &&
-		 !bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH))) {
+		 !bitmap_empty(potential_mask.high_slices, high_slices))) {  

Are we sure high_slices is not nul here when SLICE_NUM_HIGH is not nul ?

This is a tidy up which removes radix MMU calls into the slice code.

Signed-off-by Nicholas Piggin <[email protected]>

---
  arch/powerpc/include/asm/hugetlb.h |  9 ++++++---
  arch/powerpc/mm/hugetlbpage.c      |  5 +++--
  arch/powerpc/mm/slice.c            | 17 ++++-------------
  3 files changed, 13 insertions(+), 18 deletions(-)

arch/powerpc/include/asm/hugetlb.h +1 -2

index 1a4847f67ea8..59885d444695 100644
@@ -90,16 +90,19 @@ pte_t *huge_pte_offset_and_shift(struct mm_struct *mm,
  void flush_dcache_icache_hugepage(struct page *page);
 
  #if defined(CONFIG_PPC_MM_SLICES)
-int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
+int slice_is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
  			   unsigned long len);
-#else

arch/powerpc/include/asm/hugetlb.h +0 -1

  }
-#endif
 
  void book3e_hugetlb_preload(struct vm_area_struct *vma, unsigned long ea,
  			    pte_t pte);

arch/powerpc/mm/hugetlbpage.c +0 -0

index 590be3fa0ce2..b29d40889d1c 100644