[RFC/PATCH 0/15] Pass MAP_FIXED down to get_unmapped_area

[RFC/PATCH 0/15] Pass MAP_FIXED down to get_unmapped_area

[Benjamin Herrenschmidt]

!!! This is a first cut, and there are still cleanups to be done in various
areas touched by that code. I also haven't done descriptions yet for the
individual patches.

The current get_unmapped_area code calls the f_ops->get_unmapped_area or
the arch one (via the mm) only when MAP_FIXED is not passed. That makes
it impossible for archs to impose proper constraints on regions of the
virtual address space. To work around that, get_unmapped_area() then
calls some hugetlbfs specific hacks.

This cause several problems, among others:

 - It makes it impossible for a driver or filesystem to do the same thing
that hugetlbfs does (for example, to allow a driver to use larger page
sizes to map external hardware) if that requires applying a constraint
on the addresses (constraining that mapping in certain regions and other
mappings out of those regions).

 - Some archs like arm, mips, sparc, sparc64, sh and sh64 already want
MAP_FIXED to be passed down in order to deal with aliasing issues.
The code is there to handle it... but is never called.

This serie of patches moves the logic to handle MAP_FIXED down to the
various arch/driver get_unmapped_area() implementations, and then changes
the generic code to always call them. The hugetlbfs hacks then disappear
from the generic code.

Since I need to do some special 64K pages mappings for SPEs on cell, I need
to work around the first problem at least. I have further patches thus
implementing a "slices" layer that handles multiple page sizes through
slices of the address space for use by hugetlbfs, the SPE code, and possibly
others, but it requires that serie of patches first/

There is still a potential (but not practical) issue due to the fact that
filesystems/drivers implemeting g_u_a will effectively bypass all arch
checks. This is not an issue in practice as the only users of those are
actually doing so are doing it using arch hooks in the first place.

There is also a problem with mremap that will completely bypass all arch
checks. I'll try to address that separately mostly by making it not work
when the vma has a file whose f_ops has a get_unmapped_area callback,
and by making it use is_hugepage_only_range() before expanding into a
new area.

Also, I want to turn is_hugepage_only_range() into a more generic
is_normal_page_range() as that's really what it will end up meaning
when used in stack grow, brk grow and mremap.

[RFC/PATCH 1/15] get_unmapped_area handles MAP_FIXED on powerpc

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/powerpc/mm/hugetlbpage.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Index: linux-cell/arch/powerpc/mm/hugetlbpage.c
===================================================================
--- linux-cell.orig/arch/powerpc/mm/hugetlbpage.c	2007-03-22 14:52:07.000000000 +1100
+++ linux-cell/arch/powerpc/mm/hugetlbpage.c	2007-03-22 14:57:40.000000000 +1100
@@ -572,6 +572,13 @@ unsigned long arch_get_unmapped_area(str
 	if (len > TASK_SIZE)
 		return -ENOMEM;
 
+	/* handle fixed mapping: prevent overlap with huge pages */
+	if (flags & MAP_FIXED) {
+		if (is_hugepage_only_range(mm, addr, len))
+			return -EINVAL;
+		return addr;
+	}
+
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
 		vma = find_vma(mm, addr);
@@ -647,6 +654,13 @@ arch_get_unmapped_area_topdown(struct fi
 	if (len > TASK_SIZE)
 		return -ENOMEM;
 
+	/* handle fixed mapping: prevent overlap with huge pages */
+	if (flags & MAP_FIXED) {
+		if (is_hugepage_only_range(mm, addr, len))
+			return -EINVAL;
+		return addr;
+	}
+
 	/* dont allow allocations above current base */
 	if (mm->free_area_cache > base)
 		mm->free_area_cache = base;
@@ -829,6 +843,13 @@ unsigned long hugetlb_get_unmapped_area(
 	/* Paranoia, caller should have dealt with this */
 	BUG_ON((addr + len)  < addr);
 
+	/* Handle MAP_FIXED */
+	if (flags & MAP_FIXED) {
+		if (prepare_hugepage_range(addr, len, pgoff))
+			return -EINVAL;
+		return addr;
+	}
+
 	if (test_thread_flag(TIF_32BIT)) {
 		curareas = current->mm->context.low_htlb_areas;
 

[RFC/PATCH 2/15] get_unmapped_area handles MAP_FIXED on alpha

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/alpha/kernel/osf_sys.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-cell/arch/alpha/kernel/osf_sys.c
===================================================================
--- linux-cell.orig/arch/alpha/kernel/osf_sys.c	2007-03-22 14:58:33.000000000 +1100
+++ linux-cell/arch/alpha/kernel/osf_sys.c	2007-03-22 14:58:44.000000000 +1100
@@ -1267,6 +1267,9 @@ arch_get_unmapped_area(struct file *filp
 	if (len > limit)
 		return -ENOMEM;
 
+	if (flags & MAP_FIXED)
+		return addr;
+
 	/* First, see if the given suggestion fits.
 
 	   The OSF/1 loader (/sbin/loader) relies on us returning an

[RFC/PATCH 3/15] get_unmapped_area handles MAP_FIXED on arm

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/arm/mm/mmap.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-cell/arch/arm/mm/mmap.c
===================================================================
--- linux-cell.orig/arch/arm/mm/mmap.c	2007-03-22 14:59:51.000000000 +1100
+++ linux-cell/arch/arm/mm/mmap.c	2007-03-22 15:00:01.000000000 +1100
@@ -49,8 +49,7 @@ arch_get_unmapped_area(struct file *filp
 #endif
 
 	/*
-	 * We should enforce the MAP_FIXED case.  However, currently
-	 * the generic kernel code doesn't allow us to handle this.
+	 * We enforce the MAP_FIXED case.
 	 */
 	if (flags & MAP_FIXED) {
 		if (aliasing && flags & MAP_SHARED && addr & (SHMLBA - 1))

[RFC/PATCH 4/15] get_unmapped_area handles MAP_FIXED on frv

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/frv/mm/elf-fdpic.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-cell/arch/frv/mm/elf-fdpic.c
===================================================================
--- linux-cell.orig/arch/frv/mm/elf-fdpic.c	2007-03-22 15:00:50.000000000 +1100
+++ linux-cell/arch/frv/mm/elf-fdpic.c	2007-03-22 15:01:06.000000000 +1100
@@ -64,6 +64,10 @@ unsigned long arch_get_unmapped_area(str
 	if (len > TASK_SIZE)
 		return -ENOMEM;
 
+	/* handle MAP_FIXED */
+	if (flags & MAP_FIXED)
+		return addr;
+
 	/* only honour a hint if we're not going to clobber something doing so */
 	if (addr) {
 		addr = PAGE_ALIGN(addr);

[RFC/PATCH 5/15] get_unmapped_area handles MAP_FIXED on i386

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/i386/mm/hugetlbpage.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-cell/arch/i386/mm/hugetlbpage.c
===================================================================
--- linux-cell.orig/arch/i386/mm/hugetlbpage.c	2007-03-22 16:08:12.000000000 +1100
+++ linux-cell/arch/i386/mm/hugetlbpage.c	2007-03-22 16:14:19.000000000 +1100
@@ -367,6 +367,12 @@ hugetlb_get_unmapped_area(struct file *f
 	if (len > TASK_SIZE)
 		return -ENOMEM;
 
+	if (flags & MAP_FIXED) {
+		if (prepare_hugepage_range(addr, len, pgoff))
+			return -EINVAL;
+		return addr;
+	}
+
 	if (addr) {
 		addr = ALIGN(addr, HPAGE_SIZE);
 		vma = find_vma(mm, addr);

[RFC/PATCH 6/15] get_unmapped_area handles MAP_FIXED on ia64

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/ia64/kernel/sys_ia64.c |    7 +++++++
 arch/ia64/mm/hugetlbpage.c  |    8 ++++++++
 2 files changed, 15 insertions(+)

Index: linux-cell/arch/ia64/kernel/sys_ia64.c
===================================================================
--- linux-cell.orig/arch/ia64/kernel/sys_ia64.c	2007-03-22 15:10:45.000000000 +1100
+++ linux-cell/arch/ia64/kernel/sys_ia64.c	2007-03-22 15:10:47.000000000 +1100
@@ -33,6 +33,13 @@ arch_get_unmapped_area (struct file *fil
 	if (len > RGN_MAP_LIMIT)
 		return -ENOMEM;
 
+	/* handle fixed mapping: prevent overlap with huge pages */
+	if (flags & MAP_FIXED) {
+		if (is_hugepage_only_range(mm, addr, len))
+			return -EINVAL;
+		return addr;
+	}
+
 #ifdef CONFIG_HUGETLB_PAGE
 	if (REGION_NUMBER(addr) == RGN_HPAGE)
 		addr = 0;
Index: linux-cell/arch/ia64/mm/hugetlbpage.c
===================================================================
--- linux-cell.orig/arch/ia64/mm/hugetlbpage.c	2007-03-22 15:12:32.000000000 +1100
+++ linux-cell/arch/ia64/mm/hugetlbpage.c	2007-03-22 15:12:39.000000000 +1100
@@ -148,6 +148,14 @@ unsigned long hugetlb_get_unmapped_area(
 		return -ENOMEM;
 	if (len & ~HPAGE_MASK)
 		return -EINVAL;
+
+	/* Handle MAP_FIXED */
+	if (flags & MAP_FIXED) {
+		if (prepare_hugepage_range(addr, len, pgoff))
+			return -EINVAL;
+		return addr;
+	}
+
 	/* This code assumes that RGN_HPAGE != 0. */
 	if ((REGION_NUMBER(addr) != RGN_HPAGE) || (addr & (HPAGE_SIZE - 1)))
 		addr = HPAGE_REGION_BASE;
 

[RFC/PATCH 7/15] get_unmapped_area handles MAP_FIXED on parisc

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/parisc/kernel/sys_parisc.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-cell/arch/parisc/kernel/sys_parisc.c
===================================================================
--- linux-cell.orig/arch/parisc/kernel/sys_parisc.c	2007-03-22 15:28:05.000000000 +1100
+++ linux-cell/arch/parisc/kernel/sys_parisc.c	2007-03-22 15:29:08.000000000 +1100
@@ -106,6 +106,11 @@ unsigned long arch_get_unmapped_area(str
 {
 	if (len > TASK_SIZE)
 		return -ENOMEM;
+	/* Might want to check for cache aliasing issues for MAP_FIXED case
+	 * like ARM or MIPS ??? --BenH.
+	 */
+	if (flags & MAP_FIXED)
+		return addr;
 	if (!addr)
 		addr = TASK_UNMAPPED_BASE;
 

[RFC/PATCH 8/15] get_unmapped_area handles MAP_FIXED on sparc64

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/sparc64/mm/hugetlbpage.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-cell/arch/sparc64/mm/hugetlbpage.c
===================================================================
--- linux-cell.orig/arch/sparc64/mm/hugetlbpage.c	2007-03-22 16:12:57.000000000 +1100
+++ linux-cell/arch/sparc64/mm/hugetlbpage.c	2007-03-22 16:15:33.000000000 +1100
@@ -175,6 +175,12 @@ hugetlb_get_unmapped_area(struct file *f
 	if (len > task_size)
 		return -ENOMEM;
 
+	if (flags & MAP_FIXED) {
+		if (prepare_hugepage_range(addr, len, pgoff))
+			return -EINVAL;
+		return addr;
+	}
+
 	if (addr) {
 		addr = ALIGN(addr, HPAGE_SIZE);
 		vma = find_vma(mm, addr);

[RFC/PATCH 9/15] get_unmapped_area handles MAP_FIXED on x86_64

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/x86_64/kernel/sys_x86_64.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-cell/arch/x86_64/kernel/sys_x86_64.c
===================================================================
--- linux-cell.orig/arch/x86_64/kernel/sys_x86_64.c	2007-03-22 16:10:10.000000000 +1100
+++ linux-cell/arch/x86_64/kernel/sys_x86_64.c	2007-03-22 16:11:06.000000000 +1100
@@ -93,6 +93,9 @@ arch_get_unmapped_area(struct file *filp
 	unsigned long start_addr;
 	unsigned long begin, end;
 	
+	if (flags & MAP_FIXED)
+		return addr;
+
 	find_start_end(flags, &begin, &end); 
 
 	if (len > end)

[RFC/PATCH 10/15] get_unmapped_area handles MAP_FIXED in hugetlbfs

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 fs/hugetlbfs/inode.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-cell/fs/hugetlbfs/inode.c
===================================================================
--- linux-cell.orig/fs/hugetlbfs/inode.c	2007-03-22 16:12:56.000000000 +1100
+++ linux-cell/fs/hugetlbfs/inode.c	2007-03-22 16:16:02.000000000 +1100
@@ -115,6 +115,12 @@ hugetlb_get_unmapped_area(struct file *f
 	if (len > TASK_SIZE)
 		return -ENOMEM;
 
+	if (flags & MAP_FIXED) {
+		if (prepare_hugepage_range(addr, len, pgoff))
+			return -EINVAL;
+		return addr;
+	}
+
 	if (addr) {
 		addr = ALIGN(addr, HPAGE_SIZE);
 		vma = find_vma(mm, addr);

[RFC/PATCH 11/15] get_unmapped_area handles MAP_FIXED on ramfs (nommu)

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 fs/ramfs/file-nommu.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-cell/fs/ramfs/file-nommu.c
===================================================================
--- linux-cell.orig/fs/ramfs/file-nommu.c	2007-03-22 16:18:27.000000000 +1100
+++ linux-cell/fs/ramfs/file-nommu.c	2007-03-22 16:20:14.000000000 +1100
@@ -238,7 +238,10 @@ unsigned long ramfs_nommu_get_unmapped_a
 	struct page **pages = NULL, **ptr, *page;
 	loff_t isize;
 
-	if (!(flags & MAP_SHARED))
+	/* Deal with MAP_FIXED differently ? Forbid it ? Need help from some nommu
+	 * folks there... --BenH.
+	 */
+	if ((flags & MAP_FIXED) || !(flags & MAP_SHARED))
 		return addr;
 
 	/* the mapping mustn't extend beyond the EOF */

[RFC/PATCH 12/15] get_unmapped_area handles MAP_FIXED in ffb DRM

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 drivers/char/drm/ffb_drv.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-cell/drivers/char/drm/ffb_drv.c
===================================================================
--- linux-cell.orig/drivers/char/drm/ffb_drv.c	2007-03-22 16:21:22.000000000 +1100
+++ linux-cell/drivers/char/drm/ffb_drv.c	2007-03-22 16:23:13.000000000 +1100
@@ -191,6 +191,12 @@ unsigned long ffb_get_unmapped_area(stru
 			if ((kvirt & (SHMLBA - 1)) != (addr & (SHMLBA - 1))) {
 				unsigned long koff, aoff;
 
+				/* Address needs adjusting which can't be done
+				 * for MAP_FIXED
+				 */
+				if (flags & MAP_FIXED)
+					return -EINVAL;
+
 				koff = kvirt & (SHMLBA - 1);
 				aoff = addr & (SHMLBA - 1);
 				if (koff < aoff)

[RFC/PATCH 13/15] get_unmapped_area handles MAP_FIXED in /dev/mem (nommu)

[Benjamin Herrenschmidt]

This also fixes a bug, I think, it used to return a pgoff (pfn)
instead of an address. (To split ?)

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 drivers/char/mem.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-cell/drivers/char/mem.c
===================================================================
--- linux-cell.orig/drivers/char/mem.c	2007-03-22 16:24:04.000000000 +1100
+++ linux-cell/drivers/char/mem.c	2007-03-22 16:26:30.000000000 +1100
@@ -246,9 +246,12 @@ static unsigned long get_unmapped_area_m
 					   unsigned long pgoff,
 					   unsigned long flags)
 {
+	if (flags & MAP_FIXED)
+		if ((addr >> PAGE_SHIFT) != pgoff)
+			return (unsigned long) -EINVAL;
 	if (!valid_mmap_phys_addr_range(pgoff, len))
 		return (unsigned long) -EINVAL;
-	return pgoff;
+	return pgoff << PAGE_SHIFT;
 }
 
 /* can't do an in-place private mapping if there's no MMU */

[RFC/PATCH 14/15] get_unmapped_area handles MAP_FIXED in generic code

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 mm/mmap.c |   25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

Index: linux-cell/mm/mmap.c
===================================================================
--- linux-cell.orig/mm/mmap.c	2007-03-22 16:29:22.000000000 +1100
+++ linux-cell/mm/mmap.c	2007-03-22 16:30:06.000000000 +1100
@@ -1199,6 +1199,9 @@ arch_get_unmapped_area(struct file *filp
 	if (len > TASK_SIZE)
 		return -ENOMEM;
 
+	if (flags & MAP_FIXED)
+		return addr;
+
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
 		vma = find_vma(mm, addr);
@@ -1272,6 +1275,9 @@ arch_get_unmapped_area_topdown(struct fi
 	if (len > TASK_SIZE)
 		return -ENOMEM;
 
+	if (flags & MAP_FIXED)
+		return addr;
+
 	/* requesting a specific address */
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
@@ -1360,22 +1366,21 @@ get_unmapped_area(struct file *file, uns
 		unsigned long pgoff, unsigned long flags)
 {
 	unsigned long ret;
+	unsigned long (*get_area)(struct file *, unsigned long,
+				  unsigned long, unsigned long, unsigned long);
 
-	if (!(flags & MAP_FIXED)) {
-		unsigned long (*get_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
-
-		get_area = current->mm->get_unmapped_area;
-		if (file && file->f_op && file->f_op->get_unmapped_area)
-			get_area = file->f_op->get_unmapped_area;
-		addr = get_area(file, addr, len, pgoff, flags);
-		if (IS_ERR_VALUE(addr))
-			return addr;
-	}
+	get_area = current->mm->get_unmapped_area;
+	if (file && file->f_op && file->f_op->get_unmapped_area)
+		get_area = file->f_op->get_unmapped_area;
+	addr = get_area(file, addr, len, pgoff, flags);
+	if (IS_ERR_VALUE(addr))
+		return addr;
 
 	if (addr > TASK_SIZE - len)
 		return -ENOMEM;
 	if (addr & ~PAGE_MASK)
 		return -EINVAL;
+
 	if (file && is_file_hugepages(file))  {
 		/*
 		 * Check if the given range is hugepage aligned, and

[RFC/PATCH 15/15] get_unmapped_area doesn't need hugetlbfs hacks anymore

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 mm/mmap.c |   16 ----------------
 1 file changed, 16 deletions(-)

Index: linux-cell/mm/mmap.c
===================================================================
--- linux-cell.orig/mm/mmap.c	2007-03-22 16:30:24.000000000 +1100
+++ linux-cell/mm/mmap.c	2007-03-22 16:30:48.000000000 +1100
@@ -1381,22 +1381,6 @@ get_unmapped_area(struct file *file, uns
 	if (addr & ~PAGE_MASK)
 		return -EINVAL;
 
-	if (file && is_file_hugepages(file))  {
-		/*
-		 * Check if the given range is hugepage aligned, and
-		 * can be made suitable for hugepages.
-		 */
-		ret = prepare_hugepage_range(addr, len, pgoff);
-	} else {
-		/*
-		 * Ensure that a normal request is not falling in a
-		 * reserved hugepage range.  For some archs like IA-64,
-		 * there is a separate region for hugepages.
-		 */
-		ret = is_hugepage_only_range(current->mm, addr, len);
-	}
-	if (ret)
-		return -EINVAL;
 	return addr;
 }
 

Re: [RFC/PATCH 0/15] Pass MAP_FIXED down to get_unmapped_area

[Nick Piggin]

Benjamin Herrenschmidt wrote:
> !!! This is a first cut, and there are still cleanups to be done in various
> areas touched by that code. I also haven't done descriptions yet for the
> individual patches.
> 
> The current get_unmapped_area code calls the f_ops->get_unmapped_area or
> the arch one (via the mm) only when MAP_FIXED is not passed. That makes
> it impossible for archs to impose proper constraints on regions of the
> virtual address space. To work around that, get_unmapped_area() then
> calls some hugetlbfs specific hacks.
> 
> This cause several problems, among others:
> 
>  - It makes it impossible for a driver or filesystem to do the same thing
> that hugetlbfs does (for example, to allow a driver to use larger page
> sizes to map external hardware) if that requires applying a constraint
> on the addresses (constraining that mapping in certain regions and other
> mappings out of those regions).
> 
>  - Some archs like arm, mips, sparc, sparc64, sh and sh64 already want
> MAP_FIXED to be passed down in order to deal with aliasing issues.
> The code is there to handle it... but is never called.
> 
> This serie of patches moves the logic to handle MAP_FIXED down to the
> various arch/driver get_unmapped_area() implementations, and then changes
> the generic code to always call them. The hugetlbfs hacks then disappear
> from the generic code.
> 
> Since I need to do some special 64K pages mappings for SPEs on cell, I need
> to work around the first problem at least. I have further patches thus
> implementing a "slices" layer that handles multiple page sizes through
> slices of the address space for use by hugetlbfs, the SPE code, and possibly
> others, but it requires that serie of patches first/
> 
> There is still a potential (but not practical) issue due to the fact that
> filesystems/drivers implemeting g_u_a will effectively bypass all arch
> checks. This is not an issue in practice as the only users of those are
> actually doing so are doing it using arch hooks in the first place.
> 
> There is also a problem with mremap that will completely bypass all arch
> checks. I'll try to address that separately mostly by making it not work
> when the vma has a file whose f_ops has a get_unmapped_area callback,
> and by making it use is_hugepage_only_range() before expanding into a
> new area.
> 
> Also, I want to turn is_hugepage_only_range() into a more generic
> is_normal_page_range() as that's really what it will end up meaning
> when used in stack grow, brk grow and mremap.

Great, this is long overdue for a cleanup.

I haven't looked at all users of this, but does it make sense to switch
to an API that takes an address range and modifies / filters it? Perhaps
also filling in some other annotations (eg. alignment, topdown/bottom up).
This way you could stack as many arch and driver callbacks as you need,
while hopefully also having just a single generic allocator.

OTOH, that might end up being too inefficient or simply over engineered.
Did you have any other thoughts about how to do this more generically?

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [RFC/PATCH 0/15] Pass MAP_FIXED down to get_unmapped_area

[Benjamin Herrenschmidt]

> Great, this is long overdue for a cleanup.

Indeed... lots of redundant checks, dead code, etc...

> I haven't looked at all users of this, but does it make sense to switch
> to an API that takes an address range and modifies / filters it? Perhaps
> also filling in some other annotations (eg. alignment, topdown/bottom up).
> This way you could stack as many arch and driver callbacks as you need,
> while hopefully also having just a single generic allocator.
> 
> OTOH, that might end up being too inefficient or simply over engineered.
> Did you have any other thoughts about how to do this more generically?

I haven't quite managed to think about something that would fit
everybody...

The main problem is that the requirements are fairly arch specific in
nature... from page sizes, page attributes, to things like nommu who
want in some case to enfore virt=phys or to cache aliasing issues.

I think we can find a subset of "parameters" that represent the various
constraints... pgprot, page size, mostly. Then, we could stack up the
driver/fs g_u_a on top of the arch one, but it's not simple to do that
right. Who get's to pick an address first ? If the driver picks one but
the arch rejects it, the driver need a chance to pick another one ...

Or do we decide that only the arch knows, and thus the driver only
provides more detailed informations to the arch to pick something up...

That would be useful.. for example, some archs could really use knowing
at g_u_a time wether the mapping is to be cacheable or non cacheable.

I'm still thinking about it. I think my first patch set is a good step
to give some more flexibility to archs but is by no mean something to
settle on.

I need to simmer that more until my neurons manage to cough up some
nicer & generic abstraction/api to deal with that in a better way.

Ben.

Re: [RFC/PATCH 0/15] Pass MAP_FIXED down to get_unmapped_area

[Nick Piggin]

Benjamin Herrenschmidt wrote:
>>Great, this is long overdue for a cleanup.
> 
> 
> Indeed... lots of redundant checks, dead code, etc...
> 
> 
>>I haven't looked at all users of this, but does it make sense to switch
>>to an API that takes an address range and modifies / filters it? Perhaps
>>also filling in some other annotations (eg. alignment, topdown/bottom up).
>>This way you could stack as many arch and driver callbacks as you need,
>>while hopefully also having just a single generic allocator.
>>
>>OTOH, that might end up being too inefficient or simply over engineered.
>>Did you have any other thoughts about how to do this more generically?
> 
> 
> I haven't quite managed to think about something that would fit
> everybody...
> 
> The main problem is that the requirements are fairly arch specific in
> nature... from page sizes, page attributes, to things like nommu who
> want in some case to enfore virt=phys or to cache aliasing issues.
> 
> I think we can find a subset of "parameters" that represent the various
> constraints... pgprot, page size, mostly. Then, we could stack up the
> driver/fs g_u_a on top of the arch one, but it's not simple to do that
> right. Who get's to pick an address first ? If the driver picks one but
> the arch rejects it, the driver need a chance to pick another one ...
> 
> Or do we decide that only the arch knows, and thus the driver only
> provides more detailed informations to the arch to pick something up...

Well if we use a set of valid ranges, then we can start with generic code
that will set up ranges allowed by the syscall semantics.

Then the arch code could be called with that set of ranges, and perform
its modifications to that set.

Then driver/fs code could be called and perform its modifications.

Note that each call would only ever exclude ranges of virtual addresses,
increase alignment, and decrease permissions (or fail). So nobody steps
on anybody's toes. When the final set of ranges come out the other end,
you can run a generic allocator on them.

A range could be something like

struct {
	unsigned long start, end;
	pgprot prot;
	unsigned int align_bits;
	unsigned int other_flags;	
};

The problem I see with this is that it could be computationally too
expensive to have each layer process and validate/modify them. And
simply might just be overengineered for the job.


> That would be useful.. for example, some archs could really use knowing
> at g_u_a time wether the mapping is to be cacheable or non cacheable.

This would add some constraint onto the ordering of whether driver or
arch gets called first, but still wouldn't invalidate the above...


> I'm still thinking about it. I think my first patch set is a good step
> to give some more flexibility to archs but is by no mean something to
> settle on.

Yeah definitely a nice start. And of course things should be done
incrementally rather than thinking about a complete rewrite first ;)

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [RFC/PATCH 0/15] Pass MAP_FIXED down to get_unmapped_area

[Benjamin Herrenschmidt]

> Well if we use a set of valid ranges, then we can start with generic code
> that will set up ranges allowed by the syscall semantics.
> 
> Then the arch code could be called with that set of ranges, and perform
> its modifications to that set.

A bit complicated in practice... "set of ranges" can be big... depend on
what hthe restrictions actually are.

On powerpc, for example, the fs/driver don't even know. With my multiple
page size thingy for example, the only thing I want to rely on is the
page size of a given mapping. The arch code can open/close segments for
different page sizes depending if they have already some VMAs in them or
not. Thus providing ranges isn't a good idea.

With my slices patch (you can find it on linuxppc-dev, at least an older
version not base on that serie, I can post a newer one if you want), you
can see that hugetlbfs g_u_a basically turns into a single slice request
for a given page size and the arch code will automatically try to find a
segment already converted for that page size if any, if not, will try to
convert one that has no other VMAs in it.

Another counter example to you proposal is cacheable vs. cache
invalidate. A driver like /dev/mem or /proc/bus/pci only knows "cache
invalidate" in most case (and currently has no way to tell that to the
arch). It doesn't and shouldn't have to know what kind of rstrictions
the arch might have on such a mapping. Thus it shouldn't have to pick up
ranges. It doesn't care.
 
> This would add some constraint onto the ordering of whether driver or
> arch gets called first, but still wouldn't invalidate the above...

Problem is as I said above, driver doesn't really know about "ranges".
Driver really knows about constraints
> 
> > I'm still thinking about it. I think my first patch set is a good step
> > to give some more flexibility to archs but is by no mean something to
> > settle on.
> 
> Yeah definitely a nice start. And of course things should be done
> incrementally rather than thinking about a complete rewrite first ;)

Yeah...

Ben.