[PATCH] mm: move sys_mmap_pgoff from util.c

[PATCH] mm: move sys_mmap_pgoff from util.c

[Hugh Dickins]

Move sys_mmap_pgoff() from mm/util.c to mm/mmap.c and mm/nommu.c,
where we'd expect to find such code: especially now that it contains
the MAP_HUGETLB handling.  Revert mm/util.c to how it was in 2.6.32.

This patch just ignores MAP_HUGETLB in the nommu case, as in 2.6.32,
whereas 2.6.33-rc2 reported -ENOSYS.  Perhaps validate_mmap_request()
should reject it with -EINVAL?  Add that later if necessary.

Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
---

 mm/mmap.c  |   40 ++++++++++++++++++++++++++++++++++++++++
 mm/nommu.c |   25 +++++++++++++++++++++++++
 mm/util.c  |   44 --------------------------------------------
 3 files changed, 65 insertions(+), 44 deletions(-)

--- 2.6.33-rc2/mm/mmap.c	2009-12-18 11:42:54.000000000 +0000
+++ linux/mm/mmap.c	2009-12-26 18:28:30.000000000 +0000
@@ -1043,6 +1043,46 @@ unsigned long do_mmap_pgoff(struct file
 }
 EXPORT_SYMBOL(do_mmap_pgoff);
 
+SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
+		unsigned long, prot, unsigned long, flags,
+		unsigned long, fd, unsigned long, pgoff)
+{
+	struct file *file = NULL;
+	unsigned long retval = -EBADF;
+
+	if (!(flags & MAP_ANONYMOUS)) {
+		if (unlikely(flags & MAP_HUGETLB))
+			return -EINVAL;
+		file = fget(fd);
+		if (!file)
+			goto out;
+	} else if (flags & MAP_HUGETLB) {
+		struct user_struct *user = NULL;
+		/*
+		 * VM_NORESERVE is used because the reservations will be
+		 * taken when vm_ops->mmap() is called
+		 * A dummy user value is used because we are not locking
+		 * memory so no accounting is necessary
+		 */
+		len = ALIGN(len, huge_page_size(&default_hstate));
+		file = hugetlb_file_setup(HUGETLB_ANON_FILE, len, VM_NORESERVE,
+						&user, HUGETLB_ANONHUGE_INODE);
+		if (IS_ERR(file))
+			return PTR_ERR(file);
+	}
+
+	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
+
+	down_write(&current->mm->mmap_sem);
+	retval = do_mmap_pgoff(file, addr, len, prot, flags, pgoff);
+	up_write(&current->mm->mmap_sem);
+
+	if (file)
+		fput(file);
+out:
+	return retval;
+}
+
 /*
  * Some shared mappigns will want the pages marked read-only
  * to track write events. If so, we'll downgrade vm_page_prot
--- 2.6.33-rc2/mm/nommu.c	2009-12-18 11:42:54.000000000 +0000
+++ linux/mm/nommu.c	2009-12-26 18:28:30.000000000 +0000
@@ -1398,6 +1398,31 @@ error_getting_region:
 }
 EXPORT_SYMBOL(do_mmap_pgoff);
 
+SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
+		unsigned long, prot, unsigned long, flags,
+		unsigned long, fd, unsigned long, pgoff)
+{
+	struct file *file = NULL;
+	unsigned long retval = -EBADF;
+
+	if (!(flags & MAP_ANONYMOUS)) {
+		file = fget(fd);
+		if (!file)
+			goto out;
+	}
+
+	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
+
+	down_write(&current->mm->mmap_sem);
+	retval = do_mmap_pgoff(file, addr, len, prot, flags, pgoff);
+	up_write(&current->mm->mmap_sem);
+
+	if (file)
+		fput(file);
+out:
+	return retval;
+}
+
 /*
  * split a vma into two pieces at address 'addr', a new vma is allocated either
  * for the first part or the tail.
--- 2.6.33-rc2/mm/util.c	2009-12-18 11:42:55.000000000 +0000
+++ linux/mm/util.c	2009-12-26 18:28:30.000000000 +0000
@@ -4,10 +4,6 @@
 #include <linux/module.h>
 #include <linux/err.h>
 #include <linux/sched.h>
-#include <linux/hugetlb.h>
-#include <linux/syscalls.h>
-#include <linux/mman.h>
-#include <linux/file.h>
 #include <asm/uaccess.h>
 
 #define CREATE_TRACE_POINTS
@@ -272,46 +268,6 @@ int __attribute__((weak)) get_user_pages
 }
 EXPORT_SYMBOL_GPL(get_user_pages_fast);
 
-SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
-		unsigned long, prot, unsigned long, flags,
-		unsigned long, fd, unsigned long, pgoff)
-{
-	struct file * file = NULL;
-	unsigned long retval = -EBADF;
-
-	if (!(flags & MAP_ANONYMOUS)) {
-		if (unlikely(flags & MAP_HUGETLB))
-			return -EINVAL;
-		file = fget(fd);
-		if (!file)
-			goto out;
-	} else if (flags & MAP_HUGETLB) {
-		struct user_struct *user = NULL;
-		/*
-		 * VM_NORESERVE is used because the reservations will be
-		 * taken when vm_ops->mmap() is called
-		 * A dummy user value is used because we are not locking
-		 * memory so no accounting is necessary
-		 */
-		len = ALIGN(len, huge_page_size(&default_hstate));
-		file = hugetlb_file_setup(HUGETLB_ANON_FILE, len, VM_NORESERVE,
-						&user, HUGETLB_ANONHUGE_INODE);
-		if (IS_ERR(file))
-			return PTR_ERR(file);
-	}
-
-	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
-
-	down_write(&current->mm->mmap_sem);
-	retval = do_mmap_pgoff(file, addr, len, prot, flags, pgoff);
-	up_write(&current->mm->mmap_sem);
-
-	if (file)
-		fput(file);
-out:
-	return retval;
-}
-
 /* Tracepoints definitions. */
 EXPORT_TRACEPOINT_SYMBOL(kmalloc);
 EXPORT_TRACEPOINT_SYMBOL(kmem_cache_alloc);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: move sys_mmap_pgoff from util.c

[Eric B Munson]

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

On Wed, 30 Dec 2009, Hugh Dickins wrote:

> Move sys_mmap_pgoff() from mm/util.c to mm/mmap.c and mm/nommu.c,
> where we'd expect to find such code: especially now that it contains
> the MAP_HUGETLB handling.  Revert mm/util.c to how it was in 2.6.32.
> 
> This patch just ignores MAP_HUGETLB in the nommu case, as in 2.6.32,
> whereas 2.6.33-rc2 reported -ENOSYS.  Perhaps validate_mmap_request()
> should reject it with -EINVAL?  Add that later if necessary.
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

I think that -ENOSYS is the correcet response in the nommu case, but
I that can be added in a later patch.

Acked-by: Eric B Munson <ebmunson@us.ibm.com>

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

Re: [PATCH] mm: move sys_mmap_pgoff from util.c

[Hugh Dickins]

On Mon, 4 Jan 2010, Eric B Munson wrote:
> On Wed, 30 Dec 2009, Hugh Dickins wrote:
> 
> > Move sys_mmap_pgoff() from mm/util.c to mm/mmap.c and mm/nommu.c,
> > where we'd expect to find such code: especially now that it contains
> > the MAP_HUGETLB handling.  Revert mm/util.c to how it was in 2.6.32.
> > 
> > This patch just ignores MAP_HUGETLB in the nommu case, as in 2.6.32,
> > whereas 2.6.33-rc2 reported -ENOSYS.  Perhaps validate_mmap_request()
> > should reject it with -EINVAL?  Add that later if necessary.
> > 
> > Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> 
> I think that -ENOSYS is the correcet response in the nommu case, but
> I that can be added in a later patch.
> 
> Acked-by: Eric B Munson <ebmunson@us.ibm.com>

Thanks.  I had believed -ENOSYS was solely for unsupported system calls,
so thought it inappropriate here; but we seem to have quite a few places
which are using it indeed for "Function not implemented", and -EINVAL is
always so very overloaded that an alternative can be a lot more helpful.
Okay, I'll send a patch to give -ENOSYS for MAP_HUGETLB on nommu, which
will be consistent with mmu.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] nommu: reject MAP_HUGETLB

[Hugh Dickins]

We've agreed to restore the rejection of MAP_HUGETLB to nommu.
Mimic what happens with mmu when hugetlb is not configured in:
say -ENOSYS, but -EINVAL if MAP_ANONYMOUS was not given too.

Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
---

 mm/nommu.c |    8 ++++++++
 1 file changed, 8 insertions(+)

--- 2.6.33-rc2-git/mm/nommu.c	2009-12-31 08:08:16.000000000 +0000
+++ linux/mm/nommu.c	2010-01-05 12:08:01.000000000 +0000
@@ -1405,6 +1405,14 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned lon
 	struct file *file = NULL;
 	unsigned long retval = -EBADF;
 
+	if (unlikely(flags & MAP_HUGETLB)) {
+		if (flags & MAP_ANONYMOUS)
+			retval = -ENOSYS;	/* like hugetlb_file_setup */
+		else
+			retval = -EINVAL;
+		goto out;
+	}
+
 	if (!(flags & MAP_ANONYMOUS)) {
 		file = fget(fd);
 		if (!file)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] nommu: reject MAP_HUGETLB

[Eric B Munson]

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

On Tue, 05 Jan 2010, Hugh Dickins wrote:

> We've agreed to restore the rejection of MAP_HUGETLB to nommu.
> Mimic what happens with mmu when hugetlb is not configured in:
> say -ENOSYS, but -EINVAL if MAP_ANONYMOUS was not given too.
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Acked-by: Eric B Munson <ebmunson@us.ibm.com>

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

Re: [PATCH] nommu: reject MAP_HUGETLB

[David Howells]

Hugh Dickins <hugh.dickins@tiscali.co.uk> wrote:

> We've agreed to restore the rejection of MAP_HUGETLB to nommu.
> Mimic what happens with mmu when hugetlb is not configured in:
> say -ENOSYS, but -EINVAL if MAP_ANONYMOUS was not given too.

On the other hand, why not just ignore the MAP_HUGETLB flag on NOMMU?

David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] nommu: reject MAP_HUGETLB

[Hugh Dickins]

On Tue, 5 Jan 2010, David Howells wrote:
> Hugh Dickins <hugh.dickins@tiscali.co.uk> wrote:
> 
> > We've agreed to restore the rejection of MAP_HUGETLB to nommu.
> > Mimic what happens with mmu when hugetlb is not configured in:
> > say -ENOSYS, but -EINVAL if MAP_ANONYMOUS was not given too.
> 
> On the other hand, why not just ignore the MAP_HUGETLB flag on NOMMU?

I don't care very much either way: originally it was ignored,
then it became an -ENOSYS when Al moved the MAP_HUGETLB handling
into util.c, then it was ignored again when I moved that back into
mmap.c and nommu.c, now this patch makes it -ENOSYS on nommu again
- which Eric preferred.

I'd say this patch is _correct_; but I'm perfectly happy to have
you NAK it, or Linus ignore it, with the observation that nommu is
more likely to want to cut bloat than to be pedantically correct -
pedantic because I'd expect the nommu mmap() to work fine with the
MAP_HUGETLB flag there, just wouldn't be using any huge pages.

Okay with me whichever way it goes.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>