LinuxLists.cc - [RFC REBASED 1/5] powerpc/mm/slice: pass pointers to struct slice

[permalink] [raw]

Subject: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

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.

Signed-off-by: Nicholas Piggin <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/mm/slice.c | 111 ++++++++++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 51 deletions(-)

diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index b8b691369c29..683ff4604ab4 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -61,13 +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;
- slice_bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
if (start < SLICE_LOW_TOP) {
unsigned long mend = min(end,
(unsigned long)(SLICE_LOW_TOP - 1));
@@ -76,6 +75,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
- (1u << GET_LOW_SLICE_INDEX(start));
}

+ slice_bitmap_zero(ret->high_slices, high_slices);
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));
@@ -119,28 +119,27 @@ 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;
- slice_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 (!high_slices)
return;

- for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
+ slice_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);
}

static void calc_slice_mask_for_size(struct mm_struct *mm, int psize,
struct slice_mask *ret,
- unsigned long high_limit)
+ unsigned long high_slices)
{
unsigned char *hpsizes;
int index, mask_index;
@@ -148,18 +147,17 @@ static void calc_slice_mask_for_size(struct mm_struct *mm, int psize,
u64 lpsizes;

ret->low_slices = 0;
- slice_bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
lpsizes = mm->context.low_slices_psize;
for (i = 0; i < SLICE_NUM_LOW; i++)
if (((lpsizes >> (i * 4)) & 0xf) == psize)
ret->low_slices |= 1u << i;

- if (high_limit <= SLICE_LOW_TOP)
+ if (!high_slices)
return;

+ slice_bitmap_zero(ret->high_slices, high_slices);
hpsizes = mm->context.high_slices_psize;
- for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++) {
+ for (i = 0; i < high_slices; i++) {
mask_index = i & 0x1;
index = i >> 1;
if (((hpsizes[index] >> (mask_index * 4)) & 0xf) == psize)
@@ -168,16 +166,15 @@ static void calc_slice_mask_for_size(struct mm_struct *mm, int psize,
}

#ifdef CONFIG_PPC_BOOK3S_64
-static void recalc_slice_mask_cache(struct mm_struct *mm)
+static void recalc_slice_mask_cache(struct mm_struct *mm, unsigned long high_slices)
{
- unsigned long l = mm->context.slb_addr_limit;
- calc_slice_mask_for_size(mm, MMU_PAGE_4K, &mm->context.mask_4k, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_4K, &mm->context.mask_4k, high_slices);
#ifdef CONFIG_PPC_64K_PAGES
- calc_slice_mask_for_size(mm, MMU_PAGE_64K, &mm->context.mask_64k, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_64K, &mm->context.mask_64k, high_slices);
#endif
#ifdef CONFIG_HUGETLB_PAGE
- calc_slice_mask_for_size(mm, MMU_PAGE_16M, &mm->context.mask_16m, l);
- calc_slice_mask_for_size(mm, MMU_PAGE_16G, &mm->context.mask_16g, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_16M, &mm->context.mask_16m, high_slices);
+ calc_slice_mask_for_size(mm, MMU_PAGE_16G, &mm->context.mask_16g, high_slices);
#endif
}

@@ -198,17 +195,16 @@ static const struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int ps
BUG();
}
#elif defined(CONFIG_PPC_8xx)
-static void recalc_slice_mask_cache(struct mm_struct *mm)
+static void recalc_slice_mask_cache(struct mm_struct *mm, unsigned long high_slices)
{
- unsigned long l = mm->context.slb_addr_limit;
#ifdef CONFIG_PPC_16K_PAGES
- calc_slice_mask_for_size(mm, MMU_PAGE_16K, &mm->context.mask_16k, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_16K, &mm->context.mask_16k, high_slices);
#else
- calc_slice_mask_for_size(mm, MMU_PAGE_4K, &mm->context.mask_4k, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_4K, &mm->context.mask_4k, high_slices);
#endif
#ifdef CONFIG_HUGETLB_PAGE
- calc_slice_mask_for_size(mm, MMU_PAGE_512K, &mm->context.mask_512k, l);
- calc_slice_mask_for_size(mm, MMU_PAGE_8M, &mm->context.mask_8m, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_512K, &mm->context.mask_512k, high_slices);
+ calc_slice_mask_for_size(mm, MMU_PAGE_8M, &mm->context.mask_8m, high_slices);
#endif
}

@@ -290,6 +286,7 @@ static void slice_convert(struct mm_struct *mm,
unsigned char *hpsizes;
u64 lpsizes;
unsigned long i, flags;
+ unsigned long high_slices;

slice_dbg("slice_convert(mm=%p, psize=%d)\n", mm, psize);
slice_print_mask(" mask", mask);
@@ -309,7 +306,8 @@ static void slice_convert(struct mm_struct *mm,
mm->context.low_slices_psize = lpsizes;

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; i < high_slices; i++) {
mask_index = i & 0x1;
index = i >> 1;
if (test_bit(i, mask->high_slices))
@@ -322,7 +320,7 @@ static void slice_convert(struct mm_struct *mm,
(unsigned long)mm->context.low_slices_psize,
(unsigned long)mm->context.high_slices_psize);

- recalc_slice_mask_cache(mm);
+ recalc_slice_mask_cache(mm, high_slices);

spin_unlock_irqrestore(&slice_convert_lock, flags);

@@ -469,29 +467,32 @@ 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;
- slice_bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
+ slice_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,
- const struct slice_mask *src2)
+ const struct slice_mask *src1,
+ const struct slice_mask *src2,
+ unsigned long high_slices)
{
dst->low_slices = src1->low_slices | src2->low_slices;
slice_bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices,
- SLICE_NUM_HIGH);
+ high_slices);
}

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

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

#ifdef CONFIG_PPC_64K_PAGES
@@ -514,6 +515,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)))
@@ -530,13 +532,14 @@ 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) {
unsigned long flags;

mm->context.slb_addr_limit = high_limit;

spin_lock_irqsave(&slice_convert_lock, flags);
- recalc_slice_mask_cache(mm);
+ recalc_slice_mask_cache(mm, high_slices);
spin_unlock_irqrestore(&slice_convert_lock, flags);

on_each_cpu(slice_flush_segments, mm, 1);
@@ -544,7 +547,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,

/* silence stupid warning */;
potential_mask.low_slices = 0;
- slice_bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
+ slice_bitmap_zero(potential_mask.high_slices, high_slices);

/* Sanity checks */
BUG_ON(mm->task_size == 0);
@@ -595,13 +598,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);
}

/* First check hint if it's valid or if we have MAP_FIXED */
@@ -631,8 +634,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) {
@@ -669,7 +672,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);
}
@@ -678,16 +681,16 @@ 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", maskp);

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_bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH)) {
+ !slice_bitmap_empty(potential_mask.high_slices, high_slices)) {
slice_convert(mm, &potential_mask, psize);
if (psize > MMU_PAGE_BASE)
on_each_cpu(slice_flush_segments, mm, 1);
@@ -764,6 +767,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
int index, mask_index;
unsigned char *hpsizes;
unsigned long flags, lpsizes;
+ unsigned long high_slices;
unsigned int old_psize;
int i;

@@ -789,7 +793,8 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
mm->context.low_slices_psize = lpsizes;

hpsizes = mm->context.high_slices_psize;
- for (i = 0; i < SLICE_NUM_HIGH; i++) {
+ high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ for (i = 0; i < high_slices; i++) {
mask_index = i & 0x1;
index = i >> 1;
if (((hpsizes[index] >> (mask_index * 4)) & 0xf) == old_psize)
@@ -805,7 +810,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
(unsigned long)mm->context.low_slices_psize,
(unsigned long)mm->context.high_slices_psize);

- recalc_slice_mask_cache(mm);
+ recalc_slice_mask_cache(mm, high_slices);
spin_unlock_irqrestore(&slice_convert_lock, flags);
return;
bail:
@@ -816,10 +821,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);
}

@@ -858,9 +865,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
--
2.13.3

2018-02-12 18:14:00

[permalink] [raw]

Subject: [RFC REBASED 2/5] powerpc/mm/slice: implement a slice mask cache

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.

This saves about 30% kernel time on a single-page mmap/munmap micro
benchmark.

Signed-off-by: Nicholas Piggin <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 20 ++++++-
arch/powerpc/include/asm/mmu-8xx.h | 16 ++++-
arch/powerpc/mm/slice.c | 100 ++++++++++++++++++++++++++-----
3 files changed, 118 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index 0abeb0e2d616..b6d136fd8ffd 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -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 */
@@ -91,9 +101,17 @@ typedef struct {
struct npu_context *npu_context;

#ifdef CONFIG_PPC_MM_SLICES
+ unsigned long slb_addr_limit;
u64 low_slices_psize; /* SLB page size encodings */
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
diff --git a/arch/powerpc/include/asm/mmu-8xx.h b/arch/powerpc/include/asm/mmu-8xx.h
index b324ab46d838..b97d4ed3dddf 100644
--- a/arch/powerpc/include/asm/mmu-8xx.h
+++ b/arch/powerpc/include/asm/mmu-8xx.h
@@ -187,15 +187,29 @@
#define M_APG3 0x00000060

#ifndef __ASSEMBLY__
+struct slice_mask {
+ u64 low_slices;
+ DECLARE_BITMAP(high_slices, 0);
+};
+
typedef struct {
unsigned int id;
unsigned int active;
unsigned long vdso_base;
#ifdef CONFIG_PPC_MM_SLICES
+ unsigned long slb_addr_limit;
u16 user_psize; /* page size index */
u64 low_slices_psize; /* page size encodings */
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
+# ifdef CONFIG_HUGETLB_PAGE
+ struct slice_mask mask_512k;
+ struct slice_mask mask_8m;
+# endif
#endif
} mm_context_t;

diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index db1278ac21c2..ddf015d2d05b 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -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;
@@ -147,7 +138,7 @@ 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,
+static void calc_slice_mask_for_size(struct mm_struct *mm, int psize,
struct slice_mask *ret,
unsigned long high_limit)
{
@@ -176,6 +167,72 @@ static void slice_mask_for_size(struct mm_struct *mm, int psize,
}
}

+#ifdef CONFIG_PPC_BOOK3S_64
+static void recalc_slice_mask_cache(struct mm_struct *mm)
+{
+ unsigned long l = mm->context.slb_addr_limit;
+ calc_slice_mask_for_size(mm, MMU_PAGE_4K, &mm->context.mask_4k, l);
+#ifdef CONFIG_PPC_64K_PAGES
+ calc_slice_mask_for_size(mm, MMU_PAGE_64K, &mm->context.mask_64k, l);
+#endif
+#ifdef CONFIG_HUGETLB_PAGE
+ calc_slice_mask_for_size(mm, MMU_PAGE_16M, &mm->context.mask_16m, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_16G, &mm->context.mask_16g, l);
+#endif
+}
+
+static const struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
+{
+#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();
+}
+#elif defined(CONFIG_PPC_8xx)
+static void recalc_slice_mask_cache(struct mm_struct *mm)
+{
+ unsigned long l = mm->context.slb_addr_limit;
+#ifdef CONFIG_PPC_16K_PAGES
+ calc_slice_mask_for_size(mm, MMU_PAGE_16K, &mm->context.mask_16k, l);
+#else
+ calc_slice_mask_for_size(mm, MMU_PAGE_4K, &mm->context.mask_4k, l);
+#endif
+#ifdef CONFIG_HUGETLB_PAGE
+ calc_slice_mask_for_size(mm, MMU_PAGE_512K, &mm->context.mask_512k, l);
+ calc_slice_mask_for_size(mm, MMU_PAGE_8M, &mm->context.mask_8m, l);
+#endif
+}
+
+static const 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
+#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,
const struct slice_mask *available)
@@ -251,6 +308,8 @@ static void slice_convert(struct mm_struct *mm,
(unsigned long)mm->context.low_slices_psize,
(unsigned long)mm->context.high_slices_psize);

+ recalc_slice_mask_cache(mm);
+
spin_unlock_irqrestore(&slice_convert_lock, flags);

copro_flush_all_slbs(mm);
@@ -449,7 +508,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
}

if (high_limit > mm->context.slb_addr_limit) {
+ unsigned long flags;
+
mm->context.slb_addr_limit = high_limit;
+
+ spin_lock_irqsave(&slice_convert_lock, flags);
+ recalc_slice_mask_cache(mm);
+ spin_unlock_irqrestore(&slice_convert_lock, flags);
+
on_each_cpu(slice_flush_segments, mm, 1);
}

@@ -488,7 +554,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);

/*
@@ -513,7 +579,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);
}
@@ -695,7 +761,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
goto bail;

mm->context.user_psize = psize;
- wmb();
+ wmb(); /* Why? */

lpsizes = mm->context.low_slices_psize;
for (i = 0; i < SLICE_NUM_LOW; i++)
@@ -722,6 +788,9 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
(unsigned long)mm->context.low_slices_psize,
(unsigned long)mm->context.high_slices_psize);

+ recalc_slice_mask_cache(mm);
+ spin_unlock_irqrestore(&slice_convert_lock, flags);
+ return;
bail:
spin_unlock_irqrestore(&slice_convert_lock, flags);
}
@@ -762,18 +831,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
--
2.13.3

2018-02-12 18:14:27

[permalink] [raw]

Subject: [RFC REBASED 4/5] powerpc/mm/slice: Use const pointers to cached slice masks where possible

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.

This also converts the slice_mask bit operation helpers to be the usual
3-operand kind, which is clearer to work with. And we remove some
unnecessary intermediate bitmaps, reducing stack and copy overhead
further.

Signed-off-by: Nicholas Piggin <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/book3s/64/slice.h | 7 +++
arch/powerpc/include/asm/nohash/32/slice.h | 6 +++
arch/powerpc/mm/slice.c | 77 ++++++++++++++++++------------
3 files changed, 59 insertions(+), 31 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/slice.h b/arch/powerpc/include/asm/book3s/64/slice.h
index f9a2c8bd7a77..be1ce8e91ad1 100644
--- a/arch/powerpc/include/asm/book3s/64/slice.h
+++ b/arch/powerpc/include/asm/book3s/64/slice.h
@@ -63,6 +63,13 @@ static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
{
bitmap_set(map, start, nbits);
}
+
+static inline void slice_bitmap_copy(unsigned long *dst,
+ const unsigned long *src,
+ unsigned int nbits)
+{
+ bitmap_copy(dst, src, nbits);
+}
#endif /* __ASSEMBLY__ */

#else /* CONFIG_PPC_MM_SLICES */
diff --git a/arch/powerpc/include/asm/nohash/32/slice.h b/arch/powerpc/include/asm/nohash/32/slice.h
index bcb4924f7d22..38f041e01a0a 100644
--- a/arch/powerpc/include/asm/nohash/32/slice.h
+++ b/arch/powerpc/include/asm/nohash/32/slice.h
@@ -58,6 +58,12 @@ static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
unsigned int nbits)
{
}
+
+static inline void slice_bitmap_copy(unsigned long *dst,
+ const unsigned long *src,
+ unsigned int nbits)
+{
+}
#endif /* __ASSEMBLY__ */

#endif /* CONFIG_PPC_MM_SLICES */
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 311168ca3939..b8b691369c29 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -468,21 +468,30 @@ 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)
{
- dst->low_slices |= src->low_slices;
- slice_bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
+ dst->low_slices = src->low_slices;
+ slice_bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
+}
+
+static inline void slice_or_mask(struct slice_mask *dst,
+ const struct slice_mask *src1,
+ const struct slice_mask *src2)
+{
+ dst->low_slices = src1->low_slices | src2->low_slices;
+ slice_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 *src)
+ 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;

- slice_bitmap_andnot(dst->high_slices, dst->high_slices,
- src->high_slices, SLICE_NUM_HIGH);
+ slice_bitmap_andnot(dst->high_slices, src1->high_slices,
+ src2->high_slices, SLICE_NUM_HIGH);
}

#ifdef CONFIG_PPC_64K_PAGES
@@ -495,10 +504,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;
@@ -537,9 +546,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
potential_mask.low_slices = 0;
slice_bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);

- compat_mask.low_slices = 0;
- slice_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);
@@ -562,7 +568,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);
+ maskp = slice_mask_for_size(mm, psize);
slice_print_mask(" good_mask", &good_mask);

/*
@@ -587,11 +593,16 @@ 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)
- slice_or_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);
+ }

/* First check hint if it's valid or if we have MAP_FIXED */
if (addr || fixed) {
@@ -621,7 +632,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) {
@@ -658,7 +669,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_maskp);
addr = slice_find_area(mm, len, &potential_mask,
psize, topdown, high_limit);
}
@@ -667,16 +678,17 @@ 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", maskp);

convert:
- slice_andnot_mask(&mask, &good_mask);
- slice_andnot_mask(&mask, &compat_mask);
- if (mask.low_slices ||
- !slice_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_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);
}
@@ -834,19 +846,22 @@ 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, &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);
+ return !slice_check_range_fits(mm, &available, addr, len);
}
#endif

@@ -856,6 +871,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_range_fits(mm, &available, addr, len);
+ return !slice_check_range_fits(mm, maskp, addr, len);
}
#endif
--
2.13.3

2018-02-12 18:15:07

[permalink] [raw]

Subject: [RFC REBASED 3/5] powerpc/mm/slice: implement slice_check_range_fits

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.

Signed-off-by: Nicholas Piggin <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/mm/slice.c | 68 ++++++++++++++++++++++++++-----------------------
1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index ddf015d2d05b..311168ca3939 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -233,22 +233,36 @@ static const struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int ps
#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.
- */
- unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ unsigned long end = start + len - 1;
+ u64 low_slices = 0;
+
+ if (start < SLICE_LOW_TOP) {
+ unsigned long mend = min(end,
+ (unsigned long)(SLICE_LOW_TOP - 1));
+
+ 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 ((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;

- slice_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;
+ }
+ }

- return (mask->low_slices & available->low_slices) == mask->low_slices &&
- slice_bitmap_equal(result, mask->high_slices, slice_count);
+ return true;
}

static void slice_flush_segments(void *parm)
@@ -519,12 +533,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;
- slice_bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
-
/* silence stupid warning */;
potential_mask.low_slices = 0;
slice_bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
@@ -586,15 +594,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) {
/* 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;
}
@@ -620,10 +624,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;
+ }
}

/* If we have MAP_FIXED and failed the above steps, then error out */
@@ -829,13 +834,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 */
@@ -852,6 +856,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
--
2.13.3

2018-02-27 07:31:01

[permalink] [raw]

Subject: Re: [RFC REBASED 4/5] powerpc/mm/slice: Use const pointers to cached slice masks where possible

Christophe Leroy <[email protected]> writes:

> 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.
>
> This also converts the slice_mask bit operation helpers to be the usual
> 3-operand kind, which is clearer to work with. And we remove some
> unnecessary intermediate bitmaps, reducing stack and copy overhead
> further.

Can we move the reduce unncessary intermediate bitmaps as another patch?

>
> Signed-off-by: Nicholas Piggin <[email protected]>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/include/asm/book3s/64/slice.h | 7 +++
> arch/powerpc/include/asm/nohash/32/slice.h | 6 +++
> arch/powerpc/mm/slice.c | 77 ++++++++++++++++++------------
> 3 files changed, 59 insertions(+), 31 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/slice.h b/arch/powerpc/include/asm/book3s/64/slice.h
> index f9a2c8bd7a77..be1ce8e91ad1 100644
> --- a/arch/powerpc/include/asm/book3s/64/slice.h
> +++ b/arch/powerpc/include/asm/book3s/64/slice.h
> @@ -63,6 +63,13 @@ static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
> {
> bitmap_set(map, start, nbits);
> }
> +
> +static inline void slice_bitmap_copy(unsigned long *dst,
> + const unsigned long *src,
> + unsigned int nbits)
> +{
> + bitmap_copy(dst, src, nbits);
> +}
> #endif /* __ASSEMBLY__ */
>
> #else /* CONFIG_PPC_MM_SLICES */
> diff --git a/arch/powerpc/include/asm/nohash/32/slice.h b/arch/powerpc/include/asm/nohash/32/slice.h
> index bcb4924f7d22..38f041e01a0a 100644
> --- a/arch/powerpc/include/asm/nohash/32/slice.h
> +++ b/arch/powerpc/include/asm/nohash/32/slice.h
> @@ -58,6 +58,12 @@ static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
> unsigned int nbits)
> {
> }
> +
> +static inline void slice_bitmap_copy(unsigned long *dst,
> + const unsigned long *src,
> + unsigned int nbits)
> +{
> +}
> #endif /* __ASSEMBLY__ */
>
> #endif /* CONFIG_PPC_MM_SLICES */
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 311168ca3939..b8b691369c29 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -468,21 +468,30 @@ 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)
> {
> - dst->low_slices |= src->low_slices;
> - slice_bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
> + dst->low_slices = src->low_slices;
> + slice_bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
> +}
> +
> +static inline void slice_or_mask(struct slice_mask *dst,
> + const struct slice_mask *src1,
> + const struct slice_mask *src2)
> +{
> + dst->low_slices = src1->low_slices | src2->low_slices;
> + slice_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 *src)
> + 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;
>
> - slice_bitmap_andnot(dst->high_slices, dst->high_slices,
> - src->high_slices, SLICE_NUM_HIGH);
> + slice_bitmap_andnot(dst->high_slices, src1->high_slices,
> + src2->high_slices, SLICE_NUM_HIGH);
> }
>
> #ifdef CONFIG_PPC_64K_PAGES
> @@ -495,10 +504,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;
> @@ -537,9 +546,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> potential_mask.low_slices = 0;
> slice_bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
>
> - compat_mask.low_slices = 0;
> - slice_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);
> @@ -562,7 +568,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);
> + maskp = slice_mask_for_size(mm, psize);
> slice_print_mask(" good_mask", &good_mask);
>
> /*
> @@ -587,11 +593,16 @@ 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)
> - slice_or_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);
> + }
>
> /* First check hint if it's valid or if we have MAP_FIXED */
> if (addr || fixed) {
> @@ -621,7 +632,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) {
> @@ -658,7 +669,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_maskp);
> addr = slice_find_area(mm, len, &potential_mask,
> psize, topdown, high_limit);
> }
> @@ -667,16 +678,17 @@ 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);

Can we avoid reusing variables like that. I had to look at the change
below to ensure that this is intended change. The print below is wrong
then. In my patch series I did try to remove potential_mask and compat
mask and used tmp_mask in its place. IMHO that resulted in much simpiler
code. I do recompute tmp_mask for 4K size because of lack of variable.
But then i guess with your change to cache mask for different page size,
it should not have an impact?

> slice_dbg(" found potential area at 0x%lx\n", addr);
> - slice_print_mask(" mask", &mask);
> + slice_print_mask(" mask", maskp);
>
> convert:
> - slice_andnot_mask(&mask, &good_mask);
> - slice_andnot_mask(&mask, &compat_mask);
> - if (mask.low_slices ||
> - !slice_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_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);
> }
> @@ -834,19 +846,22 @@ 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, &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);
> + return !slice_check_range_fits(mm, &available, addr, len);
> }
> #endif
>
> @@ -856,6 +871,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_range_fits(mm, &available, addr, len);
> + return !slice_check_range_fits(mm, maskp, addr, len);
> }
> #endif
> --
> 2.13.3

2018-02-27 07:46:04

[permalink] [raw]

Subject: Re: [RFC REBASED 3/5] powerpc/mm/slice: implement slice_check_range_fits

Christophe Leroy <[email protected]> writes:
+ 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));
> + unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> + unsigned long i;
>
> - slice_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;
> + }
> + }

why not bitmap_equal here instead of test_bit in loop?
>
> - return (mask->low_slices & available->low_slices) == mask->low_slices &&
> - slice_bitmap_equal(result, mask->high_slices, slice_count);
> + return true;
> }

-aneesh

2018-02-27 09:02:23

[permalink] [raw]

Subject: Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

Christophe Leroy <[email protected]> writes:

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

I haven't really looked at the final code. One of the issue we had was
with the below scenario.

mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
sure we build the mask such that we don't find the addr available.

-aneesh

2018-02-27 09:06:36

[permalink] [raw]

Subject: Re: [RFC REBASED 3/5] powerpc/mm/slice: implement slice_check_range_fits

On Tue, 27 Feb 2018 12:50:08 +0530
"Aneesh Kumar K.V" <[email protected]> wrote:

> Christophe Leroy <[email protected]> writes:
> + 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));
> > + unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> > + unsigned long i;
> >
> > - slice_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;
> > + }
> > + }
>
> why not bitmap_equal here instead of test_bit in loop?

Because we only have the available bitmap now. If we see large ranges
here we could use some bitmap operation like find_next_zero_bit perhaps.

Thanks,
Nick

2018-02-27 09:09:40

[permalink] [raw]

Subject: Re: [RFC REBASED 4/5] powerpc/mm/slice: Use const pointers to cached slice masks where possible

On Tue, 27 Feb 2018 12:59:53 +0530
"Aneesh Kumar K.V" <[email protected]> wrote:

> Christophe Leroy <[email protected]> writes:
>
> > 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.
> >
> > This also converts the slice_mask bit operation helpers to be the usual
> > 3-operand kind, which is clearer to work with. And we remove some
> > unnecessary intermediate bitmaps, reducing stack and copy overhead
> > further.
>
> Can we move the reduce unncessary intermediate bitmaps as another patch?

This was rightly split out -- it moved to Christophe's patch he already
made in his series. The changelog can just be adjusted.

Thanks,
Nick

2018-02-27 09:12:36

[permalink] [raw]

Subject: Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

On Tue, 27 Feb 2018 14:31:07 +0530
"Aneesh Kumar K.V" <[email protected]> wrote:

> Christophe Leroy <[email protected]> writes:
>
> > 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.
>
> I haven't really looked at the final code. One of the issue we had was
> with the below scenario.
>
> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
> sure we build the mask such that we don't find the addr available.

We should run it through the mmap regression tests. I *think* we moved
all of that logic from the slice code to get_ummapped_area before going
in to slices. I may have missed something though, it would be good to
have more eyes on it.

Thanks,
Nick

2018-02-27 12:42:44

[permalink] [raw]

Subject: Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

Nicholas Piggin <[email protected]> writes:

> On Tue, 27 Feb 2018 14:31:07 +0530
> "Aneesh Kumar K.V" <[email protected]> wrote:
>
>> Christophe Leroy <[email protected]> writes:
>>
>> > 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.
>>
>> I haven't really looked at the final code. One of the issue we had was
>> with the below scenario.
>>
>> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
>> sure we build the mask such that we don't find the addr available.
>
> We should run it through the mmap regression tests. I *think* we moved
> all of that logic from the slice code to get_ummapped_area before going
> in to slices. I may have missed something though, it would be good to
> have more eyes on it.
>

mmap(-1,...) failed with the test. Something like below fix it

@@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
mm->context.low_slices_psize = lpsizes;

hpsizes = mm->context.high_slices_psize;
- high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ high_slices = SLICE_NUM_HIGH;
for (i = 0; i < high_slices; i++) {
mask_index = i & 0x1;
index = i >> 1;

I guess for everything in the mm_context_t, we should compute it till
SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
slice mask cached in mm_context on slb_addr_limit, it was still derived
from the high_slices_psizes which was computed with lower value.

-aneesh

2018-02-28 07:00:55

[permalink] [raw]

Subject: Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

On 02/28/2018 12:23 PM, Nicholas Piggin wrote:
> On Tue, 27 Feb 2018 18:11:07 +0530
> "Aneesh Kumar K.V" <[email protected]> wrote:
>
>> Nicholas Piggin <[email protected]> writes:
>>
>>> On Tue, 27 Feb 2018 14:31:07 +0530
>>> "Aneesh Kumar K.V" <[email protected]> wrote:
>>>
>>>> Christophe Leroy <[email protected]> writes:
>>>>
>>>>> 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.
>>>>
>>>> I haven't really looked at the final code. One of the issue we had was
>>>> with the below scenario.
>>>>
>>>> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
>>>> sure we build the mask such that we don't find the addr available.
>>>
>>> We should run it through the mmap regression tests. I *think* we moved
>>> all of that logic from the slice code to get_ummapped_area before going
>>> in to slices. I may have missed something though, it would be good to
>>> have more eyes on it.
>>>
>>
>> mmap(-1,...) failed with the test. Something like below fix it
>>
>> @@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
>> mm->context.low_slices_psize = lpsizes;
>>
>> hpsizes = mm->context.high_slices_psize;
>> - high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
>> + high_slices = SLICE_NUM_HIGH;
>> for (i = 0; i < high_slices; i++) {
>> mask_index = i & 0x1;
>> index = i >> 1;
>>
>> I guess for everything in the mm_context_t, we should compute it till
>> SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
>> slice mask cached in mm_context on slb_addr_limit, it was still derived
>> from the high_slices_psizes which was computed with lower value.
>
> Okay thanks for catching that Aneesh. I guess that's a slow path so it
> should be okay. Christophe if you're taking care of the series can you
> fold it in? Otherwise I'll do that after yours gets merged.
>

should we also compute the mm_context_t.slice_mask using SLICE_NUM_HIGH
and skip the recalc_slice_mask_cache when we change the addr limit?

-aneesh

2018-02-28 07:08:52

[permalink] [raw]

Subject: Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

I also noticed that the slice mask printing use wrong variables now. I
guess this should take care of it

diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index fef3f36b0b73..6b3575c39668 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -535,8 +535,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
* already
*/
maskp = slice_mask_for_size(mm, psize);
- slice_print_mask(" good_mask", &good_mask);
-
/*
* Here "good" means slices that are already the right page size,
* "compat" means slices that have a compatible page size (i.e.
@@ -569,6 +567,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
{
slice_copy_mask(&good_mask, maskp, high_slices);
}
+ slice_print_mask(" good_mask", &good_mask);
+ slice_print_mask(" compat_mask", compat_maskp);

/* First check hint if it's valid or if we have MAP_FIXED */
if (addr || fixed) {
@@ -646,7 +646,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,

slice_range_to_mask(addr, len, &potential_mask, high_slices);
slice_dbg(" found potential area at 0x%lx\n", addr);
- slice_print_mask(" mask", maskp);
+ slice_print_mask(" mask", &potential_mask);

convert:
slice_andnot_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
@@ -836,13 +836,6 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
return !slice_check_range_fits(mm, &available, addr, len);
}
#endif
-
-#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
return !slice_check_range_fits(mm, maskp, addr, len);
}
#endif

2018-02-28 07:18:39

[permalink] [raw]

Subject: Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

On Tue, 27 Feb 2018 18:11:07 +0530
"Aneesh Kumar K.V" <[email protected]> wrote:

> Nicholas Piggin <[email protected]> writes:
>
> > On Tue, 27 Feb 2018 14:31:07 +0530
> > "Aneesh Kumar K.V" <[email protected]> wrote:
> >
> >> Christophe Leroy <[email protected]> writes:
> >>
> >> > 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.
> >>
> >> I haven't really looked at the final code. One of the issue we had was
> >> with the below scenario.
> >>
> >> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
> >> sure we build the mask such that we don't find the addr available.
> >
> > We should run it through the mmap regression tests. I *think* we moved
> > all of that logic from the slice code to get_ummapped_area before going
> > in to slices. I may have missed something though, it would be good to
> > have more eyes on it.
> >
>
> mmap(-1,...) failed with the test. Something like below fix it
>
> @@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
> mm->context.low_slices_psize = lpsizes;
>
> hpsizes = mm->context.high_slices_psize;
> - high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> + high_slices = SLICE_NUM_HIGH;
> for (i = 0; i < high_slices; i++) {
> mask_index = i & 0x1;
> index = i >> 1;
>
> I guess for everything in the mm_context_t, we should compute it till
> SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
> slice mask cached in mm_context on slb_addr_limit, it was still derived
> from the high_slices_psizes which was computed with lower value.

Okay thanks for catching that Aneesh. I guess that's a slow path so it
should be okay. Christophe if you're taking care of the series can you
fold it in? Otherwise I'll do that after yours gets merged.

Thanks,
Nick

2018-03-01 07:11:17

[permalink] [raw]

Subject: Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

Le 28/02/2018 à 07:53, Nicholas Piggin a écrit :
> On Tue, 27 Feb 2018 18:11:07 +0530
> "Aneesh Kumar K.V" <[email protected]> wrote:
>
>> Nicholas Piggin <[email protected]> writes:
>>
>>> On Tue, 27 Feb 2018 14:31:07 +0530
>>> "Aneesh Kumar K.V" <[email protected]> wrote:
>>>
>>>> Christophe Leroy <[email protected]> writes:
>>>>
>>>>> 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.
>>>>
>>>> I haven't really looked at the final code. One of the issue we had was
>>>> with the below scenario.
>>>>
>>>> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
>>>> sure we build the mask such that we don't find the addr available.
>>>
>>> We should run it through the mmap regression tests. I *think* we moved
>>> all of that logic from the slice code to get_ummapped_area before going
>>> in to slices. I may have missed something though, it would be good to
>>> have more eyes on it.
>>>
>>
>> mmap(-1,...) failed with the test. Something like below fix it
>>
>> @@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
>> mm->context.low_slices_psize = lpsizes;
>>
>> hpsizes = mm->context.high_slices_psize;
>> - high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
>> + high_slices = SLICE_NUM_HIGH;
>> for (i = 0; i < high_slices; i++) {
>> mask_index = i & 0x1;
>> index = i >> 1;
>>
>> I guess for everything in the mm_context_t, we should compute it till
>> SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
>> slice mask cached in mm_context on slb_addr_limit, it was still derived
>> from the high_slices_psizes which was computed with lower value.
>
> Okay thanks for catching that Aneesh. I guess that's a slow path so it
> should be okay. Christophe if you're taking care of the series can you
> fold it in? Otherwise I'll do that after yours gets merged.
>

I'm not really taking care of your serie, just took it once to see how
it fits on the 8xx.
I prefer if you can handle them. If you need my help for any test on
PPC32 don't hesitate to ask.

Christophe

2018-03-01 09:23:52