linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture
@ 2013-02-21 23:05 akpm
  2013-03-18 10:40 ` Aneesh Kumar K.V
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: akpm @ 2013-02-21 23:05 UTC (permalink / raw)
  To: benh; +Cc: paulus, akpm, walken, linuxppc-dev

From: Michel Lespinasse <walken@google.com>
Subject: mm: use vm_unmapped_area() on powerpc architecture

Update the powerpc slice_get_unmapped_area function to make use of
vm_unmapped_area() instead of implementing a brute force search.

Signed-off-by: Michel Lespinasse <walken@google.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

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

diff -puN arch/powerpc/mm/slice.c~mm-use-vm_unmapped_area-on-powerpc-architecture arch/powerpc/mm/slice.c
--- a/arch/powerpc/mm/slice.c~mm-use-vm_unmapped_area-on-powerpc-architecture
+++ a/arch/powerpc/mm/slice.c
@@ -237,36 +237,69 @@ static void slice_convert(struct mm_stru
 #endif
 }
 
+/*
+ * Compute which slice addr is part of;
+ * set *boundary_addr to the start or end boundary of that slice
+ * (depending on 'end' parameter);
+ * return boolean indicating if the slice is marked as available in the
+ * 'available' slice_mark.
+ */
+static bool slice_scan_available(unsigned long addr,
+				 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));
+	} else {
+		slice = GET_HIGH_SLICE_INDEX(addr);
+		*boundary_addr = (slice + end) ?
+			((slice + end) << SLICE_HIGH_SHIFT) : SLICE_LOW_TOP;
+		return !!(available.high_slices & (1u << slice));
+	}
+}
+
 static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
 					      unsigned long len,
 					      struct slice_mask available,
 					      int psize)
 {
-	struct vm_area_struct *vma;
-	unsigned long addr;
-	struct slice_mask mask;
 	int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
+	unsigned long addr, found, next_end;
+	struct vm_unmapped_area_info info;
 
-	addr = TASK_UNMAPPED_BASE;
-
-	for (;;) {
-		addr = _ALIGN_UP(addr, 1ul << pshift);
-		if ((TASK_SIZE - len) < addr)
-			break;
-		vma = find_vma(mm, addr);
-		BUG_ON(vma && (addr >= vma->vm_end));
+	info.flags = 0;
+	info.length = len;
+	info.align_mask = PAGE_MASK & ((1ul << pshift) - 1);
+	info.align_offset = 0;
 
-		mask = slice_range_to_mask(addr, len);
-		if (!slice_check_fit(mask, available)) {
-			if (addr < SLICE_LOW_TOP)
-				addr = _ALIGN_UP(addr + 1,  1ul << SLICE_LOW_SHIFT);
-			else
-				addr = _ALIGN_UP(addr + 1,  1ul << SLICE_HIGH_SHIFT);
+	addr = TASK_UNMAPPED_BASE;
+	while (addr < TASK_SIZE) {
+		info.low_limit = addr;
+		if (!slice_scan_available(addr, available, 1, &addr))
 			continue;
+
+ next_slice:
+		/*
+		 * At this point [info.low_limit; addr) covers
+		 * available slices only and ends at a slice boundary.
+		 * Check if we need to reduce the range, or if we can
+		 * extend it to cover the next available slice.
+		 */
+		if (addr >= TASK_SIZE)
+			addr = TASK_SIZE;
+		else if (slice_scan_available(addr, available, 1, &next_end)) {
+			addr = next_end;
+			goto next_slice;
 		}
-		if (!vma || addr + len <= vma->vm_start)
-			return addr;
-		addr = vma->vm_end;
+		info.high_limit = addr;
+
+		found = vm_unmapped_area(&info);
+		if (!(found & ~PAGE_MASK))
+			return found;
 	}
 
 	return -ENOMEM;
@@ -277,39 +310,39 @@ static unsigned long slice_find_area_top
 					     struct slice_mask available,
 					     int psize)
 {
-	struct vm_area_struct *vma;
-	unsigned long addr;
-	struct slice_mask mask;
 	int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
+	unsigned long addr, found, prev;
+	struct vm_unmapped_area_info info;
 
-	addr = mm->mmap_base;
-	while (addr > len) {
-		/* Go down by chunk size */
-		addr = _ALIGN_DOWN(addr - len, 1ul << pshift);
+	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
+	info.length = len;
+	info.align_mask = PAGE_MASK & ((1ul << pshift) - 1);
+	info.align_offset = 0;
 
-		/* Check for hit with different page size */
-		mask = slice_range_to_mask(addr, len);
-		if (!slice_check_fit(mask, available)) {
-			if (addr < SLICE_LOW_TOP)
-				addr = _ALIGN_DOWN(addr, 1ul << SLICE_LOW_SHIFT);
-			else if (addr < (1ul << SLICE_HIGH_SHIFT))
-				addr = SLICE_LOW_TOP;
-			else
-				addr = _ALIGN_DOWN(addr, 1ul << SLICE_HIGH_SHIFT);
+	addr = mm->mmap_base;
+	while (addr > PAGE_SIZE) {
+		info.high_limit = addr;
+		if (!slice_scan_available(addr - 1, available, 0, &addr))
 			continue;
-		}
 
+ prev_slice:
 		/*
-		 * Lookup failure means no vma is above this address,
-		 * else if new region fits below vma->vm_start,
-		 * return with success:
+		 * At this point [addr; info.high_limit) covers
+		 * available slices only and starts at a slice boundary.
+		 * Check if we need to reduce the range, or if we can
+		 * extend it to cover the previous available slice.
 		 */
-		vma = find_vma(mm, addr);
-		if (!vma || (addr + len) <= vma->vm_start)
-			return addr;
+		if (addr < PAGE_SIZE)
+			addr = PAGE_SIZE;
+		else if (slice_scan_available(addr - 1, available, 0, &prev)) {
+			addr = prev;
+			goto prev_slice;
+		}
+		info.low_limit = addr;
 
-		/* try just below the current vma->vm_start */
-		addr = vma->vm_start;
+		found = vm_unmapped_area(&info);
+		if (!(found & ~PAGE_MASK))
+			return found;
 	}
 
 	/*
_

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture
  2013-02-21 23:05 [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture akpm
@ 2013-03-18 10:40 ` Aneesh Kumar K.V
  2013-03-18 11:12 ` Aneesh Kumar K.V
  2013-03-19  6:31 ` David Gibson
  2 siblings, 0 replies; 6+ messages in thread
From: Aneesh Kumar K.V @ 2013-03-18 10:40 UTC (permalink / raw)
  To: akpm, benh; +Cc: linuxppc-dev, akpm, walken, paulus

akpm@linux-foundation.org writes:

> From: Michel Lespinasse <walken@google.com>
> Subject: mm: use vm_unmapped_area() on powerpc architecture
>
> Update the powerpc slice_get_unmapped_area function to make use of
> vm_unmapped_area() instead of implementing a brute force search.
>
> Signed-off-by: Michel Lespinasse <walken@google.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  arch/powerpc/mm/slice.c |  123 ++++++++++++++++++++++++--------------
>  1 file changed, 78 insertions(+), 45 deletions(-)
>
> diff -puN arch/powerpc/mm/slice.c~mm-use-vm_unmapped_area-on-powerpc-architecture arch/powerpc/mm/slice.c
> --- a/arch/powerpc/mm/slice.c~mm-use-vm_unmapped_area-on-powerpc-architecture
> +++ a/arch/powerpc/mm/slice.c
> @@ -237,36 +237,69 @@ static void slice_convert(struct mm_stru
>  #endif
>  }
>
> +/*
> + * Compute which slice addr is part of;
> + * set *boundary_addr to the start or end boundary of that slice
> + * (depending on 'end' parameter);
> + * return boolean indicating if the slice is marked as available in the
> + * 'available' slice_mark.
> + */
> +static bool slice_scan_available(unsigned long addr,
> +				 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));
> +	} else {
> +		slice = GET_HIGH_SLICE_INDEX(addr);
> +		*boundary_addr = (slice + end) ?
> +			((slice + end) << SLICE_HIGH_SHIFT) : SLICE_LOW_TOP;
> +		return !!(available.high_slices & (1u << slice));
> +	}
> +}


Can you clarify more on when boundary_addr is set to start of the slice
in the comment ? Also may be it would be better if we can say
slice_scan_prev_available and slice_scan_next_available and get rid of
end completely ?

I have tested with both the patches applied and the resulting kernel boots fine. 

-aneesh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture
  2013-02-21 23:05 [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture akpm
  2013-03-18 10:40 ` Aneesh Kumar K.V
@ 2013-03-18 11:12 ` Aneesh Kumar K.V
  2013-03-18 11:23   ` Michel Lespinasse
  2013-03-19  6:31 ` David Gibson
  2 siblings, 1 reply; 6+ messages in thread
From: Aneesh Kumar K.V @ 2013-03-18 11:12 UTC (permalink / raw)
  To: akpm, benh; +Cc: linuxppc-dev, akpm, walken, paulus

akpm@linux-foundation.org writes:

> From: Michel Lespinasse <walken@google.com>
> Subject: mm: use vm_unmapped_area() on powerpc architecture
>
> Update the powerpc slice_get_unmapped_area function to make use of
> vm_unmapped_area() instead of implementing a brute force search.
>
> Signed-off-by: Michel Lespinasse <walken@google.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  arch/powerpc/mm/slice.c |  123 ++++++++++++++++++++++++--------------
>  1 file changed, 78 insertions(+), 45 deletions(-)
>
> diff -puN arch/powerpc/mm/slice.c~mm-use-vm_unmapped_area-on-powerpc-architecture arch/powerpc/mm/slice.c
> --- a/arch/powerpc/mm/slice.c~mm-use-vm_unmapped_area-on-powerpc-architecture
> +++ a/arch/powerpc/mm/slice.c
> @@ -237,36 +237,69 @@ static void slice_convert(struct mm_stru
>  #endif
>  }
>
> +/*
> + * Compute which slice addr is part of;
> + * set *boundary_addr to the start or end boundary of that slice
> + * (depending on 'end' parameter);
> + * return boolean indicating if the slice is marked as available in the
> + * 'available' slice_mark.
> + */
> +static bool slice_scan_available(unsigned long addr,
> +				 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));
> +	} else {
> +		slice = GET_HIGH_SLICE_INDEX(addr);
> +		*boundary_addr = (slice + end) ?
> +			((slice + end) << SLICE_HIGH_SHIFT) : SLICE_LOW_TOP;
> +		return !!(available.high_slices & (1u << slice));
> +	}
> +}
> +

how about  ?

static bool slice_scan_available(unsigned long addr,
				 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));
	} else {
		slice = GET_HIGH_SLICE_INDEX(addr);
		if ((slice + end) >= SLICE_NUM_HIGH)
			/* loop back in the high slice */
			*boundary_addr = SLICE_LOW_TOP;
		else
			*boundary_addr = (slice + end) << SLICE_HIGH_SHIFT;
		return !!(available.high_slices & (1u << slice));
	}
}


-aneesh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture
  2013-03-18 11:12 ` Aneesh Kumar K.V
@ 2013-03-18 11:23   ` Michel Lespinasse
  2013-03-18 12:27     ` Aneesh Kumar K.V
  0 siblings, 1 reply; 6+ messages in thread
From: Michel Lespinasse @ 2013-03-18 11:23 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: akpm, paulus, linuxppc-dev

On Mon, Mar 18, 2013 at 4:12 AM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> how about  ?
>
> static bool slice_scan_available(unsigned long addr,
>                                  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));
>         } else {
>                 slice = GET_HIGH_SLICE_INDEX(addr);

>                 if ((slice + end) >= SLICE_NUM_HIGH)
>                         /* loop back in the high slice */
>                         *boundary_addr = SLICE_LOW_TOP;
>                 else
>                         *boundary_addr = (slice + end) << SLICE_HIGH_SHIFT;

I don't mind having this section as an if..else rather than ?:
statement. However, the condition would need to be if (slice == 0 &&
end == 0) or if (slice + end == 0)

This is because the beginning of high slice 0 is at SLICE_LOW_TOP and not at 0,
and the end of high slice 63 is at 64TB not at SLICE_LOW_TOP.

>                 return !!(available.high_slices & (1u << slice));
>         }
> }

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture
  2013-03-18 11:23   ` Michel Lespinasse
@ 2013-03-18 12:27     ` Aneesh Kumar K.V
  0 siblings, 0 replies; 6+ messages in thread
From: Aneesh Kumar K.V @ 2013-03-18 12:27 UTC (permalink / raw)
  To: Michel Lespinasse; +Cc: akpm, paulus, linuxppc-dev

Michel Lespinasse <walken@google.com> writes:

> On Mon, Mar 18, 2013 at 4:12 AM, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
>> how about  ?
>>
>> static bool slice_scan_available(unsigned long addr,
>>                                  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));
>>         } else {
>>                 slice = GET_HIGH_SLICE_INDEX(addr);
>
>>                 if ((slice + end) >= SLICE_NUM_HIGH)
>>                         /* loop back in the high slice */
>>                         *boundary_addr = SLICE_LOW_TOP;
>>                 else
>>                         *boundary_addr = (slice + end) << SLICE_HIGH_SHIFT;
>
> I don't mind having this section as an if..else rather than ?:
> statement. However, the condition would need to be if (slice == 0 &&
> end == 0) or if (slice + end == 0)
>
> This is because the beginning of high slice 0 is at SLICE_LOW_TOP and not at 0,
> and the end of high slice 63 is at 64TB not at SLICE_LOW_TOP.
>

aha, I missed the fact that it was to handle slice = 0 && end == 0
case. I was looking it as looping back to start slice in high slice
range once we reached 0xffffffff. Above slice 63 in high slice, we
should have available always unset, so beyond that we can ideally
loop back to SLICE_LOW_TOP right ?

-aneesh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture
  2013-02-21 23:05 [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture akpm
  2013-03-18 10:40 ` Aneesh Kumar K.V
  2013-03-18 11:12 ` Aneesh Kumar K.V
@ 2013-03-19  6:31 ` David Gibson
  2 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2013-03-19  6:31 UTC (permalink / raw)
  To: akpm; +Cc: walken, paulus, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Thu, Feb 21, 2013 at 03:05:58PM -0800, akpm@linux-foundation.org wrote:
> From: Michel Lespinasse <walken@google.com>
> Subject: mm: use vm_unmapped_area() on powerpc architecture
> 
> Update the powerpc slice_get_unmapped_area function to make use of
> vm_unmapped_area() instead of implementing a brute force search.
> 
> Signed-off-by: Michel Lespinasse <walken@google.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

A little unnecessarily cryptic in places, but the logic looks sound.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-03-19  6:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-21 23:05 [patch 2/2] mm: use vm_unmapped_area() on powerpc architecture akpm
2013-03-18 10:40 ` Aneesh Kumar K.V
2013-03-18 11:12 ` Aneesh Kumar K.V
2013-03-18 11:23   ` Michel Lespinasse
2013-03-18 12:27     ` Aneesh Kumar K.V
2013-03-19  6:31 ` David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).