: Unbreak no-mmu mmap

PATCH In review Bernd Schmidt 8 participants 24 messages Jun 8, 2007 – Aug 20, 2007

Subsystems (9)

Memory management VFS MM core Scheduler +5 more subsystems

lore

Share & tools

lore

Bernd Schmidt

author

2007-06-08 13:54 UTC #0 lore raw

PATCH 1/1 : Unbreak no-mmu mmap

Here's a patch to move nommu mmap/munmap ever so slightly closer to mmu behaviour. The motivation for this is to be able to deselect uClibc's UCLIBC_UCLINUX_BROKEN_MUNMAP config option, which speeds up malloc a fair bit. I'm interested in comments whether this is a good direction to go. The patch is against Linus' tree as of a few minutes ago.

Bernd

Signature · 4 lines

This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

This changes nommu mmap/munmap in the following ways:
1. munmap can now unmap subparts of previously allocated blocks. This makes behaviour more consistent with mmu Linux, and allows us to simplify and speed up the uClibc malloc implementation.
2. It is no longer possible to get blocks smaller than a page through mmap. This behaviour was used by simplemalloc, which is an insane way of implementing malloc on nommu systems and hopefully not used by anyone anymore.
3. mmap can now be asked not to round up the allocation to the next power of 2 page size. Excess pages will be freed if MAP_SPLIT_PAGES is passed to mmap.
The latter is done for binfmt_flat and binfmt_elf_fdpic to save memory when loading executables.
If this flag is used, more memory is kept available, but fragmentation may (or may not) be higher.

Every VMA can be in two states: either it manages a power-of-2 sized compound page, or (if VM_SPLIT_PAGES) is set, a set of single pages exactly covering the area between vm_start and vm_end.

Signed-off-by Bernd Schmidt <[email protected]>

 fs/binfmt_elf_fdpic.c                       |   17 +-
 fs/binfmt_flat.c                            |   40 ++---
 fs/proc/task_nommu.c                        |   14 +-
 include/asm-blackfin/mman.h                 |    1 +
 include/linux/mm.h                          |    2 +
 mm/nommu.c                                  |  294 ++++++++++++++++++++-------
 mm/page_alloc.c                             |   10 +
 7 files changed, 263 insertions(+), 115 deletions(-)

fs/binfmt_elf_fdpic.c +5 ~~-12~~

index 9d62fba..4449412 100644
@@ -167,9 +167,6 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 	struct elf_fdpic_params exec_params, interp_params;
 	struct elf_phdr *phdr;
 	unsigned long stack_size, entryaddr;
-#ifndef CONFIG_MMU
-	unsigned long fullsize;
-#endif
 #ifdef ELF_FDPIC_PLAT_INIT
 	unsigned long dynaddr;
 #endif

@@ -373,8 +370,8 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 	down_write(&current->mm->mmap_sem);
 	current->mm->start_brk = do_mmap(NULL, 0, stack_size,
 					 PROT_READ | PROT_WRITE | PROT_EXEC,
-					 MAP_PRIVATE | MAP_ANONYMOUS | MAP_GROWSDOWN,
-					 0);
+					 MAP_PRIVATE | MAP_ANONYMOUS
+					 | MAP_GROWSDOWN | MAP_SPLIT_PAGES, 0);
 
 	if (IS_ERR_VALUE(current->mm->start_brk)) {
 		up_write(&current->mm->mmap_sem);

@@ -383,11 +380,6 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 		goto error_kill;
 	}
 
-	/* expand the stack mapping to use up the entire allocation granule */
-	fullsize = ksize((char *) current->mm->start_brk);
-	if (!IS_ERR_VALUE(do_mremap(current->mm->start_brk, stack_size,
-				    fullsize, 0, 0)))
-		stack_size = fullsize;
 	up_write(&current->mm->mmap_sem);
 
 	current->mm->brk = current->mm->start_brk;

@@ -906,7 +898,8 @@ static int elf_fdpic_map_file_constdisp_on_uclinux(
 
 	down_write(&mm->mmap_sem);
 	maddr = do_mmap(NULL, load_addr, top - base,
-			PROT_READ | PROT_WRITE | PROT_EXEC, mflags, 0);
+			PROT_READ | PROT_WRITE | PROT_EXEC,
+			mflags | MAP_SPLIT_PAGES, 0);
 	up_write(&mm->mmap_sem);
 	if (IS_ERR_VALUE(maddr))
 		return (int) maddr;

@@ -1006,7 +999,7 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
 		if (phdr->p_flags & PF_W) prot |= PROT_WRITE;
 		if (phdr->p_flags & PF_X) prot |= PROT_EXEC;
 
-		flags = MAP_PRIVATE | MAP_DENYWRITE;
+		flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_SPLIT_PAGES;
 		if (params->flags & ELF_FDPIC_FLAG_EXECUTABLE)
 			flags |= MAP_EXECUTABLE;

fs/binfmt_flat.c +14 ~~-26~~

index 7b0265d..b77f765 100644
@@ -419,8 +419,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 	unsigned long textpos = 0, datapos = 0, result;
 	unsigned long realdatastart = 0;
 	unsigned long text_len, data_len, bss_len, stack_len, flags;
-	unsigned long len, reallen, memp = 0;
-	unsigned long extra, rlim;
+	unsigned long len, memp = 0;
+	unsigned long memp_size, extra, rlim;
 	unsigned long *reloc = 0, *rp;
 	struct inode *inode;
 	int i, rev, relocs = 0;

@@ -530,7 +530,9 @@ static int load_flat_file(struct linux_binprm * bprm,
 		DBG_FLT("BINFMT_FLAT: ROM mapping of file (we hope)\n");
 
 		down_write(&current->mm->mmap_sem);
-		textpos = do_mmap(bprm->file, 0, text_len, PROT_READ|PROT_EXEC, MAP_PRIVATE, 0);
+		textpos = do_mmap(bprm->file, 0, text_len,
+				  PROT_READ | PROT_EXEC,
+				  MAP_PRIVATE|MAP_SPLIT_PAGES, 0);
 		up_write(&current->mm->mmap_sem);
 		if (!textpos  || textpos >= (unsigned long) -4096) {
 			if (!textpos)

@@ -543,15 +545,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 		len = data_len + extra + MAX_SHARED_LIBS * sizeof(unsigned long);
 		down_write(&current->mm->mmap_sem);
 		realdatastart = do_mmap(0, 0, len,
-			PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, 0);
-		/* Remap to use all availabe slack region space */
-		if (realdatastart && (realdatastart < (unsigned long)-4096)) {
-			reallen = ksize(realdatastart);
-			if (reallen > len) {
-				realdatastart = do_mremap(realdatastart, len,
-					reallen, MREMAP_FIXED, realdatastart);
-			}
-		}
+					PROT_READ|PROT_WRITE|PROT_EXEC,
+					MAP_PRIVATE|MAP_SPLIT_PAGES, 0);
 		up_write(&current->mm->mmap_sem);
 
 		if (realdatastart == 0 || realdatastart >= (unsigned long)-4096) {

@@ -589,21 +584,14 @@ static int load_flat_file(struct linux_binprm * bprm,
 
 		reloc = (unsigned long *) (datapos+(ntohl(hdr->reloc_start)-text_len));
 		memp = realdatastart;
-
+		memp_size = len;
 	} else {
 
 		len = text_len + data_len + extra + MAX_SHARED_LIBS * sizeof(unsigned long);
 		down_write(&current->mm->mmap_sem);
 		textpos = do_mmap(0, 0, len,
-			PROT_READ | PROT_EXEC | PROT_WRITE, MAP_PRIVATE, 0);
-		/* Remap to use all availabe slack region space */
-		if (textpos && (textpos < (unsigned long) -4096)) {
-			reallen = ksize(textpos);
-			if (reallen > len) {
-				textpos = do_mremap(textpos, len, reallen,
-					MREMAP_FIXED, textpos);
-			}
-		}
+				  PROT_READ | PROT_EXEC | PROT_WRITE,
+				  MAP_PRIVATE | MAP_SPLIT_PAGES, 0);
 		up_write(&current->mm->mmap_sem);
 
 		if (!textpos  || textpos >= (unsigned long) -4096) {

@@ -620,7 +608,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 		reloc = (unsigned long *) (textpos + ntohl(hdr->reloc_start) +
 				MAX_SHARED_LIBS * sizeof(unsigned long));
 		memp = textpos;
-
+		memp_size = len;
 #ifdef CONFIG_BINFMT_ZFLAT
 		/*
 		 * load it all in and treat it like a RAM load from now on

@@ -681,7 +669,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 		 */
 		current->mm->start_brk = datapos + data_len + bss_len;
 		current->mm->brk = (current->mm->start_brk + 3) & ~3;
-		current->mm->context.end_brk = memp + ksize((void *) memp) - stack_len;
+		current->mm->context.end_brk = memp + memp_size - stack_len;
 	}
 
 	if (flags & FLAT_FLAG_KTRACE)

@@ -784,8 +772,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 
 	/* zero the BSS,  BRK and stack areas */
 	memset((void*)(datapos + data_len), 0, bss_len + 
-			(memp + ksize((void *) memp) - stack_len -	/* end brk */
-			libinfo->lib_list[id].start_brk) +		/* start brk */
+			(memp + memp_size - stack_len -		/* end brk */
+			libinfo->lib_list[id].start_brk) +	/* start brk */
 			stack_len);
 
 	return 0;

fs/proc/task_nommu.c +10 -4

index d8b8c71..4b465ce 100644
@@ -19,20 +19,26 @@ char *task_mem(struct mm_struct *mm, char *buffer)
         
 	down_read(&mm->mmap_sem);
 	for (vml = mm->context.vmlist; vml; vml = vml->next) {
+		unsigned long size, len;
+
 		if (!vml->vma)
 			continue;
 
 		bytes += kobjsize(vml);
+		len = vml->vma->vm_end - vml->vma->vm_start;
+		if (!(vml->vma->vm_flags & VM_SPLIT_PAGES))
+			size = PAGE_SIZE << get_order(len);
+		else
+			size = len;
 		if (atomic_read(&mm->mm_count) > 1 ||
 		    atomic_read(&vml->vma->vm_usage) > 1
 		    ) {
-			sbytes += kobjsize((void *) vml->vma->vm_start);
+			sbytes += size;
 			sbytes += kobjsize(vml->vma);
 		} else {
-			bytes += kobjsize((void *) vml->vma->vm_start);
+			bytes += size;
 			bytes += kobjsize(vml->vma);
-			slack += kobjsize((void *) vml->vma->vm_start) -
-				(vml->vma->vm_end - vml->vma->vm_start);
+			slack += size - len;
 		}
 	}

include/asm-blackfin/mman.h +1 -0

index 4d504f9..07057b6 100644
@@ -24,6 +24,7 @@
 #define MAP_NONBLOCK	0x10000	/* do not block on IO */
 #define MAP_UNINITIALIZE 0x4000000  /* For anonymous mmap, memory could
                                     be uninitialized. */
+#define MAP_SPLIT_PAGES  0x8000000  /* Conserve memory (nommu only). */
 
 #define MS_ASYNC	1	/* sync memory asynchronously */
 #define MS_INVALIDATE	2	/* invalidate the caches */

include/linux/mm.h +2 -0

index e4183c6..49a1a8b 100644
@@ -169,6 +169,7 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_MAPPED_COPY	0x01000000	/* T if mapped copy of data (nommu mmap) */
 #define VM_INSERTPAGE	0x02000000	/* The vma has had "vm_insert_page()" done on it */
 #define VM_ALWAYSDUMP	0x04000000	/* Always include in core dumps */
+#define VM_SPLIT_PAGES	0x08000000	/* T if split_page was used (nommu mmap) */
 
 #ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
 #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS

@@ -305,6 +306,7 @@ void put_page(struct page *page);
 void put_pages_list(struct list_head *pages);
 
 void split_page(struct page *page, unsigned int order);
+void split_compound_page(struct page *page, unsigned int order);
 
 /*
  * Compound pages have a destructor function.  Provide a

mm/nommu.c +221 ~~-73~~

426 lines not on this page · raw message

mm/page_alloc.c +10 -0

index bd8e335..e992537 100644
@@ -833,6 +833,16 @@ void split_page(struct page *page, unsigned int order)
 }
 
 /*
+ * Like split_page, but calls destroy_compound_page first
+ */
+void split_compound_page(struct page *page, unsigned int order)
+{
+	VM_BUG_ON(!PageCompound(page));
+	destroy_compound_page(page, order);
+	split_page(page, order);
+}
+
+/*
  * Really, prep_compound_page() should be called from __rmqueue_bulk().  But
  * we cheat by calling it from here, in the order > 0 path.  Saves a branch
  * or two.

Matt Mackall

2007-06-09 19:10 UTC #1 lore raw

Quoted 4 lines · Bernd Schmidt

2. It is no longer possible to get blocks smaller than a page through mmap. This behaviour was used by simplemalloc, which is an insane way of implementing malloc on nommu systems and hopefully not used by anyone anymore.

That's worrisome. Breaking existing apps/libraries seems like a bad idea.

Signature · 1 line

Mathematics is the supreme nostalgia of our time.

Robin Getz

2007-06-11 21:05 UTC #2 lore raw

On Sat 9 Jun 2007 15:10, Matt Mackall pondered:

Quoted 8 lines · Bernd Schmidt

Quoted 4 lines
2. It is no longer possible to get blocks smaller than a page through mmap. This behaviour was used by simplemalloc, which is an insane way of implementing malloc on nommu systems and hopefully not used by anyone anymore.
That's worrisome. Breaking existing apps/libraries seems like a bad idea.

It is a bad idea - but on noMMU - breaking existing apps/libraries is (unfortunately) a pretty common thing...
- the standard distribution - uClinux - rebuilds all things : kernel, apps, libc, shell, etc which run on the target - everytime you type 'make'
- the standard library - uClibc - has little concept of binary compatibility, and has none in their years of releasing things. (which is OK, since everything is re-built with a simple 'make' anyway)

Plus - what Bernd talked about - uClibc's simple malloc, isn't used that often (at all that I am aware of), on any modern uClibc systems. The overhead of calling the kernel to manage memory kills performance.

As a user of the kernel in a noMMU environment - I would rather have Bernd's patch - which makes things closer to a MMU environment - and allows standard applications to work better (at all) - rather than backwards binary compatibility.

-Robin

Mike Frysinger

2007-06-11 22:09 UTC #3 lore raw

In reply to Matt Mackall

Quoted 8 lines · Matt Mackall

Quoted 4 lines · Bernd Schmidt
2. It is no longer possible to get blocks smaller than a page through mmap. This behaviour was used by simplemalloc, which is an insane way of implementing malloc on nommu systems and hopefully not used by anyone anymore.
That's worrisome. Breaking existing apps/libraries seems like a bad idea.

it isnt breaking anything ... simplemalloc() will continue to execute in newer kernels
-mike

Bernd Schmidt

author

2007-06-11 23:04 UTC #4 lore raw

Quoted 7 lines · Mike Frysinger

Quoted 6 lines · Matt Mackall
Quoted 4 lines · Bernd Schmidt
2. It is no longer possible to get blocks smaller than a page through mmap. This behaviour was used by simplemalloc, which is an insane way of implementing malloc on nommu systems and hopefully not used by anyone anymore.

Quoted 5 lines

Quoted 2 lines
That's worrisome. Breaking existing apps/libraries seems like a bad idea.
it isnt breaking anything ... simplemalloc() will continue to execute in newer kernels

While that's true, it'll have an even bigger memory overhead than it already does (simplemalloc, by trapping into the kernel and creating vm_area/vm_list structures for every malloc call, has huge overheads in both time and space).
I've posted this as an RFD to get a feeling for whether we can change behaviour here - I do expect that nommu embedded systems are less constrained by backwards compatibility considerations, but I'd like to hear from other embedded users whether this is a change they'd welcome.

Bernd

Signature · 4 lines

This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

Mike Frysinger

2007-06-11 23:22 UTC #5 lore raw

Quoted 11 lines · Bernd Schmidt

Quoted 10 lines · Mike Frysinger
Quoted 8 lines · Matt Mackall
Quoted 4 lines · Bernd Schmidt
2. It is no longer possible to get blocks smaller than a page through mmap. This behaviour was used by simplemalloc, which is an insane way of implementing malloc on nommu systems and hopefully not used by anyone anymore.
That's worrisome. Breaking existing apps/libraries seems like a bad idea.

Quoted 7 lines

Quoted 2 lines
it isnt breaking anything ... simplemalloc() will continue to execute in newer kernels
While that's true, it'll have an even bigger memory overhead than it already does (simplemalloc, by trapping into the kernel and creating vm_area/vm_list structures for every malloc call, has huge overheads in both time and space).

yes, it does increase the runtime overhead, but the simplemalloc implementation is already tagged as crappy, so i dont think it's that big of a deal ... especially in light of all of the advantages the other malloc implementation gets us nommu peeps
-mike

Robin Getz

2007-06-19 23:23 UTC #6 lore raw

In reply to Bernd Schmidt

On Fri 8 Jun 2007 09:53, Bernd Schmidt pondered:

Quoted 5 lines

Here's a patch to move nommu mmap/munmap ever so slightly closer to mmu behaviour. The motivation for this is to be able to deselect uClibc's UCLIBC_UCLINUX_BROKEN_MUNMAP config option, which speeds up malloc a fair bit. I'm interested in comments whether this is a good direction to go. The patch is against Linus' tree as of a few minutes ago.

I'm assuming that since no one had any large objections, that this is OK, and we should send to Andrew to live in -mm for awhile?

-Robin

Bryan Wu

2007-06-20 02:34 UTC #7 lore raw

Quoted 11 lines · Robin Getz

On Fri 8 Jun 2007 09:53, Bernd Schmidt pondered:
Quoted 5 lines
Here's a patch to move nommu mmap/munmap ever so slightly closer to mmu behaviour. The motivation for this is to be able to deselect uClibc's UCLIBC_UCLINUX_BROKEN_MUNMAP config option, which speeds up malloc a fair bit. I'm interested in comments whether this is a good direction to go. The patch is against Linus' tree as of a few minutes ago.
I'm assuming that since no one had any large objections, that this is OK, and we should send to Andrew to live in -mm for awhile?
-Robin

Yes, IMO it is fine for kernel and uclibc. Is there any comments from David and Greg?

Thanks
- Bryan

Paul Mundt

2007-06-20 03:02 UTC #8 lore raw

In reply to Robin Getz

Quoted 4 lines · Bernd Schmidt

mm/nommu.c +0 -0
index 2b16b00..7480a95 100644

[snip]

mm/nommu.c static int do_mmap_private(struct vm_area_struct *vma, unsigned long len) · line 838

Quoted 7 lines

mm/nommu.c +7 -0

+	/*
+	 * Must always set the VM_SPLIT_PAGES flag for single-page allocations,
+	 * to avoid trying to get the order of the compound page later on.
+	 */
+	if (len == PAGE_SIZE)
+		vma->vm_flags |= VM_SPLIT_PAGES;
+	else if (flags & MAP_SPLIT_PAGES

And now you've just broken every non-blackfin nommu platform, as you've only defined MAP_SPLIT_PAGES in asm-blackfin/mman.h.

mm/nommu.c static int do_mmap_private(struct vm_area_struct *vma, unsigned long len) · line 841

Quoted 3 lines

mm/nommu.c +3 -0

+#ifdef CONFIG_NP2
+	    || len < total_len
+#endif

And what is this? It only shows up in the blackfin defconfig. This is not the place to be putting board-specific hacks.

Quoted 3 lines · Robin Getz

I'm assuming that since no one had any large objections, that this is OK, and we should send to Andrew to live in -mm for awhile?

No real objections to the approach, but it would be nice if these sorts of things were test compiled for at least one platform that isn't yours, so the obviously broken stuff is fixed before it's posted and someone else has to find out about it later.

Bryan Wu

2007-06-20 03:14 UTC #9 lore raw

Quoted 17 lines · Paul Mundt

Quoted 4 lines · Bernd Schmidt
mm/nommu.c +0 -0
index 2b16b00..7480a95 100644
[snip]
mm/nommu.c static int do_mmap_private(struct vm_area_struct *vma, unsigned long len) · line 838
Quoted 7 lines
mm/nommu.c +7 -0
+	/*
+	 * Must always set the VM_SPLIT_PAGES flag for single-page allocations,
+	 * to avoid trying to get the order of the compound page later on.
+	 */
+	if (len == PAGE_SIZE)
+		vma->vm_flags |= VM_SPLIT_PAGES;
+	else if (flags & MAP_SPLIT_PAGES
And now you've just broken every non-blackfin nommu platform, as you've only defined MAP_SPLIT_PAGES in asm-blackfin/mman.h.

Quoted 6 lines

mm/nommu.c static int do_mmap_private(struct vm_area_struct *vma, unsigned long len) · line 841
Quoted 3 lines
mm/nommu.c +3 -0
+#ifdef CONFIG_NP2
+	    || len < total_len
+#endif
And what is this? It only shows up in the blackfin defconfig. This is not the place to be putting board-specific hacks.

Yes, it is our own NP2 memory allocator option. I think Bernd will fix it.

Quoted 9 lines · Robin Getz

Quoted 3 lines
I'm assuming that since no one had any large objections, that this is OK, and we should send to Andrew to live in -mm for awhile?
No real objections to the approach, but it would be nice if these sorts of things were test compiled for at least one platform that isn't yours, so the obviously broken stuff is fixed before it's posted and someone else has to find out about it later.

Exactly, Could please do some simple test on your SH-NOMMU platform? And we are waiting for some feedback from other nommu arch maintainers.

David and Grep could you please help on this? Maybe Robin got some m68k nommu by hand which can be used for testing, I only have Blackfin, -:))

Thanks, Paul.
Regards,
- Bryan

Quoted 5 lines

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Greg Ungerer

2007-06-27 05:51 UTC #10 lore raw

Quoted 17 lines · Bryan Wu

Quoted 16 lines · Paul Mundt
Quoted 4 lines · Bernd Schmidt
mm/nommu.c +0 -0
index 2b16b00..7480a95 100644
[snip]
mm/nommu.c static int do_mmap_private(struct vm_area_struct *vma, unsigned long len) · line 838
Quoted 7 lines
mm/nommu.c +7 -0
+	/*
+	 * Must always set the VM_SPLIT_PAGES flag for single-page allocations,
+	 * to avoid trying to get the order of the compound page later on.
+	 */
+	if (len == PAGE_SIZE)
+		vma->vm_flags |= VM_SPLIT_PAGES;
+	else if (flags & MAP_SPLIT_PAGES
And now you've just broken every non-blackfin nommu platform, as you've only defined MAP_SPLIT_PAGES in asm-blackfin/mman.h.

Quoted 8 lines

Quoted 5 lines
mm/nommu.c static int do_mmap_private(struct vm_area_struct *vma, unsigned long len) · line 841
Quoted 3 lines
mm/nommu.c +3 -0
+#ifdef CONFIG_NP2
+	    || len < total_len
+#endif
And what is this? It only shows up in the blackfin defconfig. This is not the place to be putting board-specific hacks.
Yes, it is our own NP2 memory allocator option. I think Bernd will fix it.

Theres no reason you can't add the MAP_SPLIT_PAGES define in all the necessary places too.

Quoted 8 lines

Quoted 7 lines · Robin Getz
Quoted 2 lines
I'm assuming that since no one had any large objections, that this is OK, and we should send to Andrew to live in -mm for awhile?
No real objections to the approach, but it would be nice if these sorts of things were test compiled for at least one platform that isn't yours, so the obviously broken stuff is fixed before it's posted and someone else has to find out about it later.

Quoted 5 lines

Exactly, Could please do some simple test on your SH-NOMMU platform? And we are waiting for some feedback from other nommu arch maintainers.
David and Grep could you please help on this? Maybe Robin got some m68k nommu by hand which can be used for testing, I only have Blackfin, -:))

I have compiled the patch on m68knommu (after adding a MAP_SPLIT_PAGES define). And it seems to work ok with simple testing.

I don't have a problem with the change, though please do add that MAP_SPLIT_PAGES define in the appropriate mman.h includes. And like Paul said there is no place for CONFIG_NP2 in it currently. Please take that out.

Regards
Greg

------------------------------------------------------------------------

Greg Ungerer  --  Chief Software Dude       EMAIL:     [email protected]
Secure Computing Corporation                PHONE:       +61 7 3435 2888
825 Stanley St,                             FAX:         +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia         WEB: http://www.SnapGear.com

David Howells

2007-06-22 12:59 UTC #11 lore raw

In reply to Bernd Schmidt

fs/binfmt_elf_fdpic.c static int load_elf_fdpic_binary(struct linux_binprm *bprm, · line 374

Quoted 2 lines · Bernd Schmidt

fs/binfmt_elf_fdpic.c +2 -0

+					 MAP_PRIVATE | MAP_ANONYMOUS
+					 | MAP_GROWSDOWN | MAP_SPLIT_PAGES, 0);

The OR-operator should end the first line.

include/asm-blackfin/mman.h line 27

Quoted 9 lines

include/asm-blackfin/mman.h +1 -0

index 4d504f9..07057b6 100644
@@ -24,6 +24,7 @@
 #define MAP_NONBLOCK	0x10000	/* do not block on IO */
 #define MAP_UNINITIALIZE 0x4000000  /* For anonymous mmap, memory could
                                     be uninitialized. */
+#define MAP_SPLIT_PAGES  0x8000000  /* Conserve memory (nommu only). */

Blackfin should really use asm-generic/mman.h if it can.

I'll produce my own version of the patch and pass it back to you once I've got FRV compiling with it.

David

David Howells

2007-06-22 13:36 UTC #12 lore raw

In reply to Bernd Schmidt

Quoted 1 line · Bernd Schmidt

Excess pages will be freed if MAP_SPLIT_PAGES is passed to mmap.

How about MAP_TRIM_EXCESS instead of MAP_SPLIT_PAGES? I think it fits the function better.

David

David Howells

2007-06-22 14:29 UTC #13 lore raw

PATCH 1/1 NOMMU: Unbreak no-mmu mmap

In reply to Bernd Schmidt

From: Bernd Schmidt <[email protected]>

Every VMA can be in two states: either it manages a power-of-2 sized compound page, or (if VM_SPLIT_PAGES) is set, a set of single pages exactly covering the area between vm_start and vm_end.

Signed-off-by Bernd Schmidt <[email protected]>

Signed-off-by David Howells <[email protected]>

---

 fs/binfmt_elf_fdpic.c       |   17 +-
 fs/binfmt_flat.c            |   40 ++----
 fs/proc/task_nommu.c        |   14 +-
 include/asm-blackfin/mman.h |    1 
 include/asm-frv/mman.h      |    3 
 include/linux/mm.h          |    2 
 mm/nommu.c                  |  294 ++++++++++++++++++++++++++++++++-----------
 mm/page_alloc.c             |   10 +
 8 files changed, 266 insertions(+), 115 deletions(-)

fs/binfmt_elf_fdpic.c +5 ~~-12~~

index 9d62fba..bea3be0 100644
@@ -167,9 +167,6 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 	struct elf_fdpic_params exec_params, interp_params;
 	struct elf_phdr *phdr;
 	unsigned long stack_size, entryaddr;
-#ifndef CONFIG_MMU
-	unsigned long fullsize;
-#endif
 #ifdef ELF_FDPIC_PLAT_INIT
 	unsigned long dynaddr;
 #endif

@@ -373,8 +370,8 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 	down_write(&current->mm->mmap_sem);
 	current->mm->start_brk = do_mmap(NULL, 0, stack_size,
 					 PROT_READ | PROT_WRITE | PROT_EXEC,
-					 MAP_PRIVATE | MAP_ANONYMOUS | MAP_GROWSDOWN,
-					 0);
+					 MAP_PRIVATE | MAP_ANONYMOUS |
+					 MAP_GROWSDOWN | MAP_SPLIT_PAGES, 0);
 
 	if (IS_ERR_VALUE(current->mm->start_brk)) {
 		up_write(&current->mm->mmap_sem);

@@ -383,11 +380,6 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 		goto error_kill;
 	}
 
-	/* expand the stack mapping to use up the entire allocation granule */
-	fullsize = ksize((char *) current->mm->start_brk);
-	if (!IS_ERR_VALUE(do_mremap(current->mm->start_brk, stack_size,
-				    fullsize, 0, 0)))
-		stack_size = fullsize;
 	up_write(&current->mm->mmap_sem);
 
 	current->mm->brk = current->mm->start_brk;

@@ -906,7 +898,8 @@ static int elf_fdpic_map_file_constdisp_on_uclinux(
 
 	down_write(&mm->mmap_sem);
 	maddr = do_mmap(NULL, load_addr, top - base,
-			PROT_READ | PROT_WRITE | PROT_EXEC, mflags, 0);
+			PROT_READ | PROT_WRITE | PROT_EXEC,
+			mflags | MAP_SPLIT_PAGES, 0);
 	up_write(&mm->mmap_sem);
 	if (IS_ERR_VALUE(maddr))
 		return (int) maddr;

@@ -1006,7 +999,7 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
 		if (phdr->p_flags & PF_W) prot |= PROT_WRITE;
 		if (phdr->p_flags & PF_X) prot |= PROT_EXEC;
 
-		flags = MAP_PRIVATE | MAP_DENYWRITE;
+		flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_SPLIT_PAGES;
 		if (params->flags & ELF_FDPIC_FLAG_EXECUTABLE)
 			flags |= MAP_EXECUTABLE;

fs/binfmt_flat.c +14 ~~-26~~

index 861141b..63da3fb 100644
@@ -419,8 +419,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 	unsigned long textpos = 0, datapos = 0, result;
 	unsigned long realdatastart = 0;
 	unsigned long text_len, data_len, bss_len, stack_len, flags;
-	unsigned long len, reallen, memp = 0;
-	unsigned long extra, rlim;
+	unsigned long len, memp = 0;
+	unsigned long memp_size, extra, rlim;
 	unsigned long *reloc = 0, *rp;
 	struct inode *inode;
 	int i, rev, relocs = 0;

@@ -530,7 +530,9 @@ static int load_flat_file(struct linux_binprm * bprm,
 		DBG_FLT("BINFMT_FLAT: ROM mapping of file (we hope)\n");
 
 		down_write(&current->mm->mmap_sem);
-		textpos = do_mmap(bprm->file, 0, text_len, PROT_READ|PROT_EXEC, MAP_PRIVATE, 0);
+		textpos = do_mmap(bprm->file, 0, text_len,
+				  PROT_READ | PROT_EXEC,
+				  MAP_PRIVATE|MAP_SPLIT_PAGES, 0);
 		up_write(&current->mm->mmap_sem);
 		if (!textpos  || textpos >= (unsigned long) -4096) {
 			if (!textpos)

@@ -543,15 +545,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 		len = data_len + extra + MAX_SHARED_LIBS * sizeof(unsigned long);
 		down_write(&current->mm->mmap_sem);
 		realdatastart = do_mmap(0, 0, len,
-			PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, 0);
-		/* Remap to use all availabe slack region space */
-		if (realdatastart && (realdatastart < (unsigned long)-4096)) {
-			reallen = ksize(realdatastart);
-			if (reallen > len) {
-				realdatastart = do_mremap(realdatastart, len,
-					reallen, MREMAP_FIXED, realdatastart);
-			}
-		}
+					PROT_READ|PROT_WRITE|PROT_EXEC,
+					MAP_PRIVATE|MAP_SPLIT_PAGES, 0);
 		up_write(&current->mm->mmap_sem);
 
 		if (realdatastart == 0 || realdatastart >= (unsigned long)-4096) {

@@ -589,21 +584,14 @@ static int load_flat_file(struct linux_binprm * bprm,
 
 		reloc = (unsigned long *) (datapos+(ntohl(hdr->reloc_start)-text_len));
 		memp = realdatastart;
-
+		memp_size = len;
 	} else {
 
 		len = text_len + data_len + extra + MAX_SHARED_LIBS * sizeof(unsigned long);
 		down_write(&current->mm->mmap_sem);
 		textpos = do_mmap(0, 0, len,
-			PROT_READ | PROT_EXEC | PROT_WRITE, MAP_PRIVATE, 0);
-		/* Remap to use all availabe slack region space */
-		if (textpos && (textpos < (unsigned long) -4096)) {
-			reallen = ksize(textpos);
-			if (reallen > len) {
-				textpos = do_mremap(textpos, len, reallen,
-					MREMAP_FIXED, textpos);
-			}
-		}
+				  PROT_READ | PROT_EXEC | PROT_WRITE,
+				  MAP_PRIVATE | MAP_SPLIT_PAGES, 0);
 		up_write(&current->mm->mmap_sem);
 
 		if (!textpos  || textpos >= (unsigned long) -4096) {

@@ -620,7 +608,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 		reloc = (unsigned long *) (textpos + ntohl(hdr->reloc_start) +
 				MAX_SHARED_LIBS * sizeof(unsigned long));
 		memp = textpos;
-
+		memp_size = len;
 #ifdef CONFIG_BINFMT_ZFLAT
 		/*
 		 * load it all in and treat it like a RAM load from now on

@@ -681,7 +669,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 		 */
 		current->mm->start_brk = datapos + data_len + bss_len;
 		current->mm->brk = (current->mm->start_brk + 3) & ~3;
-		current->mm->context.end_brk = memp + ksize((void *) memp) - stack_len;
+		current->mm->context.end_brk = memp + memp_size - stack_len;
 	}
 
 	if (flags & FLAT_FLAG_KTRACE)

@@ -784,8 +772,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 
 	/* zero the BSS,  BRK and stack areas */
 	memset((void*)(datapos + data_len), 0, bss_len + 
-			(memp + ksize((void *) memp) - stack_len -	/* end brk */
-			libinfo->lib_list[id].start_brk) +		/* start brk */
+			(memp + memp_size - stack_len -		/* end brk */
+			libinfo->lib_list[id].start_brk) +	/* start brk */
 			stack_len);
 
 	return 0;

fs/proc/task_nommu.c +10 -4

index d8b8c71..4b465ce 100644
@@ -19,20 +19,26 @@ char *task_mem(struct mm_struct *mm, char *buffer)
         
 	down_read(&mm->mmap_sem);
 	for (vml = mm->context.vmlist; vml; vml = vml->next) {
+		unsigned long size, len;
+
 		if (!vml->vma)
 			continue;
 
 		bytes += kobjsize(vml);
+		len = vml->vma->vm_end - vml->vma->vm_start;
+		if (!(vml->vma->vm_flags & VM_SPLIT_PAGES))
+			size = PAGE_SIZE << get_order(len);
+		else
+			size = len;
 		if (atomic_read(&mm->mm_count) > 1 ||
 		    atomic_read(&vml->vma->vm_usage) > 1
 		    ) {
-			sbytes += kobjsize((void *) vml->vma->vm_start);
+			sbytes += size;
 			sbytes += kobjsize(vml->vma);
 		} else {
-			bytes += kobjsize((void *) vml->vma->vm_start);
+			bytes += size;
 			bytes += kobjsize(vml->vma);
-			slack += kobjsize((void *) vml->vma->vm_start) -
-				(vml->vma->vm_end - vml->vma->vm_start);
+			slack += size - len;
 		}
 	}

include/asm-blackfin/mman.h +1 -0

index 4d504f9..07057b6 100644
@@ -24,6 +24,7 @@
 #define MAP_NONBLOCK	0x10000	/* do not block on IO */
 #define MAP_UNINITIALIZE 0x4000000  /* For anonymous mmap, memory could
                                     be uninitialized. */
+#define MAP_SPLIT_PAGES  0x8000000  /* Conserve memory (nommu only). */
 
 #define MS_ASYNC	1	/* sync memory asynchronously */
 #define MS_INVALIDATE	2	/* invalidate the caches */

include/asm-frv/mman.h +3 -0

index b4371e9..d5f4a2a 100644
@@ -10,6 +10,9 @@
 #define MAP_NORESERVE	0x4000		/* don't check for reservations */
 #define MAP_POPULATE	0x8000		/* populate (prefault) pagetables */
 #define MAP_NONBLOCK	0x10000		/* do not block on IO */
+#define MAP_UNINITIALIZE 0x4000000	/* For anonymous mmap, memory could
+					   be uninitialized. */
+#define MAP_SPLIT_PAGES	0x8000000	/* Conserve memory (nommu only). */
 
 #define MCL_CURRENT	1		/* lock all current mappings */
 #define MCL_FUTURE	2		/* lock all future mappings */

include/linux/mm.h +2 -0

index 1c12074..80d946c 100644
@@ -169,6 +169,7 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_MAPPED_COPY	0x01000000	/* T if mapped copy of data (nommu mmap) */
 #define VM_INSERTPAGE	0x02000000	/* The vma has had "vm_insert_page()" done on it */
 #define VM_ALWAYSDUMP	0x04000000	/* Always include in core dumps */
+#define VM_SPLIT_PAGES	0x08000000	/* T if split_page was used (nommu mmap) */
 
 #ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
 #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS

@@ -305,6 +306,7 @@ void put_page(struct page *page);
 void put_pages_list(struct list_head *pages);
 
 void split_page(struct page *page, unsigned int order);
+void split_compound_page(struct page *page, unsigned int order);
 
 /*
  * Compound pages have a destructor function.  Provide a

mm/nommu.c +221 ~~-73~~

426 lines not on this page · raw message

mm/page_alloc.c +10 -0

index 05ace44..fcc8e88 100644
@@ -833,6 +833,16 @@ void split_page(struct page *page, unsigned int order)
 }
 
 /*
+ * Like split_page, but calls destroy_compound_page first
+ */
+void split_compound_page(struct page *page, unsigned int order)
+{
+	VM_BUG_ON(!PageCompound(page));
+	destroy_compound_page(page, order);
+	split_page(page, order);
+}
+
+/*
  * Really, prep_compound_page() should be called from __rmqueue_bulk().  But
  * we cheat by calling it from here, in the order > 0 path.  Saves a branch
  * or two.

David Howells

2007-08-01 12:29 UTC #14 lore raw

In reply to Bernd Schmidt

Quoted 3 lines · Bernd Schmidt

1. munmap can now unmap subparts of previously allocated blocks. This makes behaviour more consistent with mmu Linux, and allows us to simplify and speed up the uClibc malloc implementation.

There's a problem with your alteration to do_munmap() to split VMAs. What happens if an area of a file is mmap'd twice in a process? Consider the situation where two independent parts of a process (say two different libraries) each map an area of the same file. do_mmap_pgoff() will attempt to share these regions if it can.

This can lead to you splitting the wrong VMA when someone comes along later to do an apparent partial unmap.

I'm currently working on something to give each NOMMU process its own list VMAs rather than holding them in common. This is so that I can add VMA reservation (and more to the point, unreservation) which'll make some stuff easier.

David

David Howells

2007-08-03 14:03 UTC #15 lore raw

PATCH 1/1 NOMMU: Separate out VMAs

In reply to Bernd Schmidt

Here's a preview of my patch to give each process a separate list of VMAs under NOMMU mode, just as under MMU mode. Could you have a look over it please?

Could you also see if you get a memory leak on the blackfin CPU? I see a leak when I use this patch, but I'm not sure whether it's this patch, or whether it's something else in the arch that is suppressed without this patch.

As far as I can tell by page counting there shouldn't be a leak.

I've taken some of Bernd's patch into this one, and I'll take a bit more when I've done.

David

[PATCH] NOMMU: Make VMAs per MM as for MMU-mode linux

From: David Howells <[email protected]>

Make VMAs per mm_struct as for MMU-mode linux. This solves the nattch problem for SYSV SHM where nattch for a segment does not reflect the number of shmat's (and forks) done.

Signed-off-by David Howells <[email protected]>

---

 arch/frv/Makefile        |    2 
 arch/frv/kernel/ptrace.c |   11 -
 fs/binfmt_elf_fdpic.c    |   29 --
 fs/proc/internal.h       |    2 
 fs/proc/nommu.c          |   71 ++--
 fs/proc/task_nommu.c     |  135 +++++--
 include/asm-frv/mmu.h    |    1 
 include/linux/mm.h       |   43 +-
 ipc/shm.c                |   12 +
 mm/nommu.c               |  852 +++++++++++++++++++++++++++++++---------------
 10 files changed, 740 insertions(+), 418 deletions(-)

arch/frv/Makefile +1 -1

index 9bf7345..038e3a8 100644
@@ -88,7 +88,7 @@ ASFLAGS		+= -mno-fdpic
 # make sure the .S files get compiled with debug info
 # and disable optimisations that are unhelpful whilst debugging
 ifdef CONFIG_DEBUG_INFO
-#CFLAGS		+= -O1
+CFLAGS		+= -O1
 AFLAGS		+= -Wa,--gdwarf2
 ASFLAGS		+= -Wa,--gdwarf2
 endif

arch/frv/kernel/ptrace.c +6 -5

index 709e9bd..e9af8de 100644
@@ -69,7 +69,8 @@ static inline int put_reg(struct task_struct *task, int regno,
 }
 
 /*
- * check that an address falls within the bounds of the target process's memory mappings
+ * check that an address falls within the bounds of the target process's memory
+ * mappings
  */
 static inline int is_user_addr_valid(struct task_struct *child,
 				     unsigned long start, unsigned long len)

@@ -79,11 +80,11 @@ static inline int is_user_addr_valid(struct task_struct *child,
 		return -EIO;
 	return 0;
 #else
-	struct vm_list_struct *vml;
+	struct vm_area_struct *vma;
 
-	for (vml = child->mm->context.vmlist; vml; vml = vml->next)
-		if (start >= vml->vma->vm_start && start + len <= vml->vma->vm_end)
-			return 0;
+	vma = find_vma(child->mm, start);
+	if (start >= vma->vm_start && start + len <= vma->vm_end)
+		return 0;
 
 	return -EIO;
 #endif

fs/binfmt_elf_fdpic.c +4 ~~-25~~

index 2f5d8db..c76070f 100644
@@ -384,7 +384,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 	}
 
 	/* expand the stack mapping to use up the entire allocation granule */
-	fullsize = ksize((char *) current->mm->start_brk);
+	fullsize = PAGE_ALIGN(current->mm->start_brk);
 	if (!IS_ERR_VALUE(do_mremap(current->mm->start_brk, stack_size,
 				    fullsize, 0, 0)))
 		stack_size = fullsize;

@@ -1524,11 +1524,9 @@ end_coredump:
 static int elf_fdpic_dump_segments(struct file *file, size_t *size,
 			   unsigned long *limit, unsigned long mm_flags)
 {
-	struct vm_list_struct *vml;
-
-	for (vml = current->mm->context.vmlist; vml; vml = vml->next) {
-	struct vm_area_struct *vma = vml->vma;
+	struct vm_area_struct *vma;
 
+	for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
 		if (!maydump(vma, mm_flags))
 			continue;

@@ -1576,9 +1574,6 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	elf_fpxregset_t *xfpu = NULL;
 #endif
 	int thread_status_size = 0;
-#ifndef CONFIG_MMU
-	struct vm_list_struct *vml;
-#endif
 	elf_addr_t *auxv;
 	unsigned long mm_flags;

@@ -1645,13 +1640,7 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	fill_prstatus(prstatus, current, signr);
 	elf_core_copy_regs(&prstatus->pr_reg, regs);
 
-#ifdef CONFIG_MMU
 	segs = current->mm->map_count;
-#else
-	segs = 0;
-	for (vml = current->mm->context.vmlist; vml; vml = vml->next)
-	    segs++;
-#endif
 #ifdef ELF_CORE_EXTRA_PHDRS
 	segs += ELF_CORE_EXTRA_PHDRS;
 #endif

@@ -1726,20 +1715,10 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	mm_flags = current->mm->flags;
 
 	/* write program headers for segments dump */
-	for (
-#ifdef CONFIG_MMU
-		vma = current->mm->mmap; vma; vma = vma->vm_next
-#else
-			vml = current->mm->context.vmlist; vml; vml = vml->next
-#endif
-	     ) {
+	for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
 		struct elf_phdr phdr;
 		size_t sz;
 
-#ifndef CONFIG_MMU
-		vma = vml->vma;
-#endif
-
 		sz = vma->vm_end - vma->vm_start;
 
 		phdr.p_type = PT_LOAD;

fs/proc/internal.h +0 -2

index b215c35..e8aa8d3 100644
@@ -33,8 +33,6 @@ do {						\
 	(vmi)->used = 0;			\
 	(vmi)->largest_chunk = 0;		\
 } while(0)
-
-extern int nommu_vma_show(struct seq_file *, struct vm_area_struct *);
 #endif
 
 extern int maps_protect;

fs/proc/nommu.c +32 ~~-39~~

133 lines not on this page · raw message

fs/proc/task_nommu.c +84 ~~-51~~

216 lines not on this page · raw message

include/asm-frv/mmu.h +0 -1

index 22c0371..86ca0e8 100644
@@ -22,7 +22,6 @@ typedef struct {
 	unsigned long	dtlb_ptd_mapping;	/* [DAMR5] PTD mapping for dtlb cached PGE */
 
 #else
-	struct vm_list_struct	*vmlist;
 	unsigned long		end_brk;
 
 #endif

include/linux/mm.h +25 ~~-18~~

61 lines not on this page · raw message

ipc/shm.c +12 -0

index a86a3a5..d0f1bf8 100644
@@ -1033,6 +1033,7 @@ asmlinkage long sys_shmdt(char __user *shmaddr)
 	 */
 	vma = find_vma(mm, addr);
 
+#ifdef CONFIG_MMU
 	while (vma) {
 		next = vma->vm_next;

@@ -1077,6 +1078,17 @@ asmlinkage long sys_shmdt(char __user *shmaddr)
 		vma = next;
 	}
 
+#else /* CONFIG_MMU */
+	/* under NOMMU conditions, the exact address to be destroyed must be
+	 * given */
+	retval = -EINVAL;
+	if (vma->vm_start == addr && vma->vm_ops == &shm_vm_ops) {
+		do_munmap(mm, vma->vm_start, vma->vm_end - vma->vm_start);
+		retval = 0;
+	}
+
+#endif
+
 	up_write(&mm->mmap_sem);
 	return retval;
 }

mm/nommu.c +576 ~~-276~~

1,133 lines not on this page · raw message

Bernd Schmidt

author

2007-08-07 13:40 UTC #16 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

Quoted 3 lines · David Howells

Here's a preview of my patch to give each process a separate list of VMAs under NOMMU mode, just as under MMU mode. Could you have a look over it please?

I've managed to apply it to our Blackfin tree and started looking at it.

Quoted 5 lines

Could you also see if you get a memory leak on the blackfin CPU? I see a leak when I use this patch, but I'm not sure whether it's this patch, or whether it's something else in the arch that is suppressed without this patch.
As far as I can tell by page counting there shouldn't be a leak.

There is a leak:

root:~> while true; do

Quoted 3 lines

cat /proc/buddyinfo
sleep 1
done

Node 0, zone      DMA     20      1      1      1      0      1      1
    1      0      0      0      1      2      0
Node 0, zone      DMA     32      1      1      0      0      0      1
    0      0      0      0      1      2      0
Node 0, zone      DMA     47      1      1      1      1      0      0
    1      1      1      1      0      2      0
Node 0, zone      DMA     62      1      1      0      1      1      1
    1      0      1      1      0      2      0
Node 0, zone      DMA     77      1      1      1      0      0      1
    0      0      1      1      0      2      0
Node 0, zone      DMA     92      1      1      0      0      1      0
    1      1      0      1      0      2      0
Node 0, zone      DMA    107      1      1      1      1      1      1
    1      0      0      1      0      2      0
Node 0, zone      DMA    122      1      1      0      1      0      1
    0      0      0      1      0      2      0
Node 0, zone      DMA    137      1      1      1      0      1      0
    1      1      1      0      0      2      0

... and so on. It's a strange pattern of fragmentation, as if it keeps allocating 8k pages and freeing one half of them.

Will play with this some more. Thanks!

Bernd

David Howells

2007-08-07 14:23 UTC #17 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

Quoted 1 line · Bernd Schmidt

There is a leak:

Yes. I found the major leak this morning. There may be a minor leak, but I'm not convinced it's in the mmap stuff. See revised patch.

The leak was due to the fact that I wasn't setting the usage count on the 2nd+ pages when I made a >0 order allocation.

David

---
[PATCH] NOMMU: Make VMAs per MM as for MMU-mode linux

From: David Howells <[email protected]>

Make VMAs per mm_struct as for MMU-mode linux. This solves the nattch problem for SYSV SHM where nattch for a segment does not reflect the number of shmat's (and forks) done.

Signed-off-by David Howells <[email protected]>

---

 arch/frv/Makefile        |    2 
 arch/frv/kernel/ptrace.c |   11 -
 fs/binfmt_elf_fdpic.c    |   29 --
 fs/proc/internal.h       |    2 
 fs/proc/nommu.c          |   71 ++--
 fs/proc/task_nommu.c     |  135 +++++--
 include/asm-frv/mmu.h    |    1 
 include/linux/mm.h       |   43 +-
 ipc/shm.c                |   12 +
 mm/nommu.c               |  852 +++++++++++++++++++++++++++++++---------------
 10 files changed, 740 insertions(+), 418 deletions(-)

arch/frv/Makefile +1 -1

index 9bf7345..038e3a8 100644
@@ -88,7 +88,7 @@ ASFLAGS		+= -mno-fdpic
 # make sure the .S files get compiled with debug info
 # and disable optimisations that are unhelpful whilst debugging
 ifdef CONFIG_DEBUG_INFO
-#CFLAGS		+= -O1
+CFLAGS		+= -O1
 AFLAGS		+= -Wa,--gdwarf2
 ASFLAGS		+= -Wa,--gdwarf2
 endif

arch/frv/kernel/ptrace.c +6 -5

index 709e9bd..e9af8de 100644
@@ -69,7 +69,8 @@ static inline int put_reg(struct task_struct *task, int regno,
 }
 
 /*
- * check that an address falls within the bounds of the target process's memory mappings
+ * check that an address falls within the bounds of the target process's memory
+ * mappings
  */
 static inline int is_user_addr_valid(struct task_struct *child,
 				     unsigned long start, unsigned long len)

@@ -79,11 +80,11 @@ static inline int is_user_addr_valid(struct task_struct *child,
 		return -EIO;
 	return 0;
 #else
-	struct vm_list_struct *vml;
+	struct vm_area_struct *vma;
 
-	for (vml = child->mm->context.vmlist; vml; vml = vml->next)
-		if (start >= vml->vma->vm_start && start + len <= vml->vma->vm_end)
-			return 0;
+	vma = find_vma(child->mm, start);
+	if (start >= vma->vm_start && start + len <= vma->vm_end)
+		return 0;
 
 	return -EIO;
 #endif

fs/binfmt_elf_fdpic.c +4 ~~-25~~

index 2f5d8db..c76070f 100644
@@ -384,7 +384,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 	}
 
 	/* expand the stack mapping to use up the entire allocation granule */
-	fullsize = ksize((char *) current->mm->start_brk);
+	fullsize = PAGE_ALIGN(current->mm->start_brk);
 	if (!IS_ERR_VALUE(do_mremap(current->mm->start_brk, stack_size,
 				    fullsize, 0, 0)))
 		stack_size = fullsize;

@@ -1524,11 +1524,9 @@ end_coredump:
 static int elf_fdpic_dump_segments(struct file *file, size_t *size,
 			   unsigned long *limit, unsigned long mm_flags)
 {
-	struct vm_list_struct *vml;
-
-	for (vml = current->mm->context.vmlist; vml; vml = vml->next) {
-	struct vm_area_struct *vma = vml->vma;
+	struct vm_area_struct *vma;
 
+	for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
 		if (!maydump(vma, mm_flags))
 			continue;

@@ -1576,9 +1574,6 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	elf_fpxregset_t *xfpu = NULL;
 #endif
 	int thread_status_size = 0;
-#ifndef CONFIG_MMU
-	struct vm_list_struct *vml;
-#endif
 	elf_addr_t *auxv;
 	unsigned long mm_flags;

@@ -1645,13 +1640,7 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	fill_prstatus(prstatus, current, signr);
 	elf_core_copy_regs(&prstatus->pr_reg, regs);
 
-#ifdef CONFIG_MMU
 	segs = current->mm->map_count;
-#else
-	segs = 0;
-	for (vml = current->mm->context.vmlist; vml; vml = vml->next)
-	    segs++;
-#endif
 #ifdef ELF_CORE_EXTRA_PHDRS
 	segs += ELF_CORE_EXTRA_PHDRS;
 #endif

@@ -1726,20 +1715,10 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	mm_flags = current->mm->flags;
 
 	/* write program headers for segments dump */
-	for (
-#ifdef CONFIG_MMU
-		vma = current->mm->mmap; vma; vma = vma->vm_next
-#else
-			vml = current->mm->context.vmlist; vml; vml = vml->next
-#endif
-	     ) {
+	for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
 		struct elf_phdr phdr;
 		size_t sz;
 
-#ifndef CONFIG_MMU
-		vma = vml->vma;
-#endif
-
 		sz = vma->vm_end - vma->vm_start;
 
 		phdr.p_type = PT_LOAD;

fs/proc/internal.h +0 -2

index b215c35..e8aa8d3 100644
@@ -33,8 +33,6 @@ do {						\
 	(vmi)->used = 0;			\
 	(vmi)->largest_chunk = 0;		\
 } while(0)
-
-extern int nommu_vma_show(struct seq_file *, struct vm_area_struct *);
 #endif
 
 extern int maps_protect;

fs/proc/nommu.c +32 ~~-39~~

133 lines not on this page · raw message

fs/proc/task_nommu.c +84 ~~-51~~

216 lines not on this page · raw message

include/asm-frv/mmu.h +0 -1

index 22c0371..86ca0e8 100644
@@ -22,7 +22,6 @@ typedef struct {
 	unsigned long	dtlb_ptd_mapping;	/* [DAMR5] PTD mapping for dtlb cached PGE */
 
 #else
-	struct vm_list_struct	*vmlist;
 	unsigned long		end_brk;
 
 #endif

include/linux/mm.h +25 ~~-18~~

61 lines not on this page · raw message

ipc/shm.c +12 -0

index a86a3a5..d0f1bf8 100644
@@ -1033,6 +1033,7 @@ asmlinkage long sys_shmdt(char __user *shmaddr)
 	 */
 	vma = find_vma(mm, addr);
 
+#ifdef CONFIG_MMU
 	while (vma) {
 		next = vma->vm_next;

@@ -1077,6 +1078,17 @@ asmlinkage long sys_shmdt(char __user *shmaddr)
 		vma = next;
 	}
 
+#else /* CONFIG_MMU */
+	/* under NOMMU conditions, the exact address to be destroyed must be
+	 * given */
+	retval = -EINVAL;
+	if (vma->vm_start == addr && vma->vm_ops == &shm_vm_ops) {
+		do_munmap(mm, vma->vm_start, vma->vm_end - vma->vm_start);
+		retval = 0;
+	}
+
+#endif
+
 	up_write(&mm->mmap_sem);
 	return retval;
 }

mm/nommu.c +576 ~~-276~~

1,133 lines not on this page · raw message

David Howells

2007-08-07 13:22 UTC #18 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

Quoted 2 lines · David Howells

Yes. I found the major leak this morning. There may be a minor leak, but I'm not convinced it's in the mmap stuff. See revised patch.

Oops. That was the old patch. Try this one instead.

David

[PATCH] NOMMU: Make VMAs per MM as for MMU-mode linux

From: David Howells <[email protected]>

Make VMAs per mm_struct as for MMU-mode linux. This solves the nattch problem for SYSV SHM where nattch for a segment does not reflect the number of shmat's (and forks) done.

Signed-off-by David Howells <[email protected]>

---

 arch/frv/Makefile        |    2 
 arch/frv/kernel/ptrace.c |   11 -
 fs/binfmt_elf_fdpic.c    |   29 -
 fs/proc/internal.h       |    2 
 fs/proc/nommu.c          |   71 ++--
 fs/proc/proc_misc.c      |    6 
 fs/proc/task_nommu.c     |  135 ++++---
 include/asm-frv/mmu.h    |    1 
 include/linux/mm.h       |   47 +-
 ipc/shm.c                |   12 +
 kernel/fork.c            |    4 
 mm/mmap.c                |   10 +
 mm/nommu.c               |  900 +++++++++++++++++++++++++++++++---------------
 13 files changed, 784 insertions(+), 446 deletions(-)

arch/frv/Makefile +1 -1

index 9bf7345..038e3a8 100644
@@ -88,7 +88,7 @@ ASFLAGS		+= -mno-fdpic
 # make sure the .S files get compiled with debug info
 # and disable optimisations that are unhelpful whilst debugging
 ifdef CONFIG_DEBUG_INFO
-#CFLAGS		+= -O1
+CFLAGS		+= -O1
 AFLAGS		+= -Wa,--gdwarf2
 ASFLAGS		+= -Wa,--gdwarf2
 endif

arch/frv/kernel/ptrace.c +6 -5

index 709e9bd..e9af8de 100644
@@ -69,7 +69,8 @@ static inline int put_reg(struct task_struct *task, int regno,
 }
 
 /*
- * check that an address falls within the bounds of the target process's memory mappings
+ * check that an address falls within the bounds of the target process's memory
+ * mappings
  */
 static inline int is_user_addr_valid(struct task_struct *child,
 				     unsigned long start, unsigned long len)

@@ -79,11 +80,11 @@ static inline int is_user_addr_valid(struct task_struct *child,
 		return -EIO;
 	return 0;
 #else
-	struct vm_list_struct *vml;
+	struct vm_area_struct *vma;
 
-	for (vml = child->mm->context.vmlist; vml; vml = vml->next)
-		if (start >= vml->vma->vm_start && start + len <= vml->vma->vm_end)
-			return 0;
+	vma = find_vma(child->mm, start);
+	if (start >= vma->vm_start && start + len <= vma->vm_end)
+		return 0;
 
 	return -EIO;
 #endif

fs/binfmt_elf_fdpic.c +4 ~~-25~~

index 2f5d8db..c76070f 100644
@@ -384,7 +384,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
 	}
 
 	/* expand the stack mapping to use up the entire allocation granule */
-	fullsize = ksize((char *) current->mm->start_brk);
+	fullsize = PAGE_ALIGN(current->mm->start_brk);
 	if (!IS_ERR_VALUE(do_mremap(current->mm->start_brk, stack_size,
 				    fullsize, 0, 0)))
 		stack_size = fullsize;

@@ -1524,11 +1524,9 @@ end_coredump:
 static int elf_fdpic_dump_segments(struct file *file, size_t *size,
 			   unsigned long *limit, unsigned long mm_flags)
 {
-	struct vm_list_struct *vml;
-
-	for (vml = current->mm->context.vmlist; vml; vml = vml->next) {
-	struct vm_area_struct *vma = vml->vma;
+	struct vm_area_struct *vma;
 
+	for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
 		if (!maydump(vma, mm_flags))
 			continue;

@@ -1576,9 +1574,6 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	elf_fpxregset_t *xfpu = NULL;
 #endif
 	int thread_status_size = 0;
-#ifndef CONFIG_MMU
-	struct vm_list_struct *vml;
-#endif
 	elf_addr_t *auxv;
 	unsigned long mm_flags;

@@ -1645,13 +1640,7 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	fill_prstatus(prstatus, current, signr);
 	elf_core_copy_regs(&prstatus->pr_reg, regs);
 
-#ifdef CONFIG_MMU
 	segs = current->mm->map_count;
-#else
-	segs = 0;
-	for (vml = current->mm->context.vmlist; vml; vml = vml->next)
-	    segs++;
-#endif
 #ifdef ELF_CORE_EXTRA_PHDRS
 	segs += ELF_CORE_EXTRA_PHDRS;
 #endif

@@ -1726,20 +1715,10 @@ static int elf_fdpic_core_dump(long signr, struct pt_regs *regs,
 	mm_flags = current->mm->flags;
 
 	/* write program headers for segments dump */
-	for (
-#ifdef CONFIG_MMU
-		vma = current->mm->mmap; vma; vma = vma->vm_next
-#else
-			vml = current->mm->context.vmlist; vml; vml = vml->next
-#endif
-	     ) {
+	for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
 		struct elf_phdr phdr;
 		size_t sz;
 
-#ifndef CONFIG_MMU
-		vma = vml->vma;
-#endif
-
 		sz = vma->vm_end - vma->vm_start;
 
 		phdr.p_type = PT_LOAD;

fs/proc/internal.h +0 -2

index b215c35..e8aa8d3 100644
@@ -33,8 +33,6 @@ do {						\
 	(vmi)->used = 0;			\
 	(vmi)->largest_chunk = 0;		\
 } while(0)
-
-extern int nommu_vma_show(struct seq_file *, struct vm_area_struct *);
 #endif
 
 extern int maps_protect;

fs/proc/nommu.c +32 ~~-39~~

133 lines not on this page · raw message

fs/proc/proc_misc.c +6 -0

index bee251c..c29d23b 100644
@@ -160,6 +160,9 @@ static int meminfo_read_proc(char *page, char **start, off_t off,
 		"LowTotal:     %8lu kB\n"
 		"LowFree:      %8lu kB\n"
 #endif
+#ifndef CONFIG_MMU
+		"MmapCopy:     %8lu kB\n"
+#endif
 		"SwapTotal:    %8lu kB\n"
 		"SwapFree:     %8lu kB\n"
 		"Dirty:        %8lu kB\n"

@@ -190,6 +193,9 @@ static int meminfo_read_proc(char *page, char **start, off_t off,
 		K(i.totalram-i.totalhigh),
 		K(i.freeram-i.freehigh),
 #endif
+#ifndef CONFIG_MMU
+		K((unsigned long) atomic_read(&mmap_pages_allocated)),
+#endif
 		K(i.totalswap),
 		K(i.freeswap),
 		K(global_page_state(NR_FILE_DIRTY)),

fs/proc/task_nommu.c +84 ~~-51~~

216 lines not on this page · raw message

include/asm-frv/mmu.h +0 -1

index 22c0371..86ca0e8 100644
@@ -22,7 +22,6 @@ typedef struct {
 	unsigned long	dtlb_ptd_mapping;	/* [DAMR5] PTD mapping for dtlb cached PGE */
 
 #else
-	struct vm_list_struct	*vmlist;
 	unsigned long		end_brk;
 
 #endif

include/linux/mm.h +29 ~~-18~~

77 lines not on this page · raw message

ipc/shm.c +12 -0

index a86a3a5..d0f1bf8 100644
@@ -1033,6 +1033,7 @@ asmlinkage long sys_shmdt(char __user *shmaddr)
 	 */
 	vma = find_vma(mm, addr);
 
+#ifdef CONFIG_MMU
 	while (vma) {
 		next = vma->vm_next;

@@ -1077,6 +1078,17 @@ asmlinkage long sys_shmdt(char __user *shmaddr)
 		vma = next;
 	}
 
+#else /* CONFIG_MMU */
+	/* under NOMMU conditions, the exact address to be destroyed must be
+	 * given */
+	retval = -EINVAL;
+	if (vma->vm_start == addr && vma->vm_ops == &shm_vm_ops) {
+		do_munmap(mm, vma->vm_start, vma->vm_end - vma->vm_start);
+		retval = 0;
+	}
+
+#endif
+
 	up_write(&mm->mmap_sem);
 	return retval;
 }

kernel/fork.c +1 -3

index 7332e23..41fccde 100644
@@ -1456,12 +1456,10 @@ void __init proc_caches_init(void)
 	fs_cachep = kmem_cache_create("fs_cache",
 			sizeof(struct fs_struct), 0,
 			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
-	vm_area_cachep = kmem_cache_create("vm_area_struct",
-			sizeof(struct vm_area_struct), 0,
-			SLAB_PANIC, NULL);
 	mm_cachep = kmem_cache_create("mm_struct",
 			sizeof(struct mm_struct), ARCH_MIN_MMSTRUCT_ALIGN,
 			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+	mmap_init();
 }
 
 /*

mm/mmap.c +10 -0

index b653721..100694b 100644
@@ -2224,3 +2224,13 @@ int install_special_mapping(struct mm_struct *mm,
 
 	return 0;
 }
+
+/*
+ * initialise the VMA slab
+ */
+void __init mmap_init(void)
+{
+	vm_area_cachep = kmem_cache_create("vm_area_struct",
+			sizeof(struct vm_area_struct), 0,
+			SLAB_PANIC, NULL);
+}

mm/nommu.c +599 ~~-301~~

1,198 lines not on this page · raw message

Bernd Schmidt

author

2007-08-07 13:38 UTC #19 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

mm/nommu.c static int do_mmap_private(struct vm_area_struct *vma, unsigned long len) · line 951

Quoted 13 lines · David Howells

mm/nommu.c +13 -0

+	/* we allocated a power-of-2 sized page set, so we need to trim off the
+	 * excess */
+	total = 1 << order;
+	atomic_add(total, &mmap_pages_allocated);
+
+	point = len >> PAGE_SHIFT;
+	while (point < total) {
+		order = ilog2(total - point);
+		_debug("shave %u/%lu", 1 << order, total - point);
+		atomic_sub(1 << order, &mmap_pages_allocated);
+		__free_pages(pages + point, order);
+		point += 1 << order;
+	}

Note that we've had a similar change in our kernel, and we've had to revert it since the effect on fragmentation is just horrendous. Without this, we could run a complete gcc testsuite on the board; with it we'd run out of high-order pages somewhere in the middle.

This probably wants to be dependent on something like MAP_TRIM_EXCESS.

Bernd

Signature · 4 lines

This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

David Howells

2007-08-07 14:19 UTC #20 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

Quoted 1 line · Bernd Schmidt

This probably wants to be dependent on something like MAP_TRIM_EXCESS.

Hmmm... In the interests of not adding more flags, I wonder if mremap() should be used for that, with a global setting for mmaps made by binfmts as they can't really be shrunk afterwards if they're shareable.

David

Bernd Schmidt

author

2007-08-17 11:50 UTC #21 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

In reply to David Howells

Quoted 7 lines · David Howells

Quoted 2 lines · David Howells
Yes. I found the major leak this morning. There may be a minor leak, but I'm not convinced it's in the mmap stuff. See revised patch.
Oops. That was the old patch. Try this one instead.

Here are some changes to make it more stable on my system. In do_mmap_private, I've commented out the logic to free excess pages, as it fragments terribly and causes a simple
while true; do cat /proc/buddyinfo; done
loop to go oom. Also, I think you're freeing high-order pages unaligned to their order?
In shrink_vma, we must save the mm across calls to remove_vma_from_mm (oops when telnetting into the box).
In do_munmap, we can deal with freeing more than one vma. I've not touched the rb-tree logic in the shared file case, as I have no idea what it's trying to do given that only exact matches are allowed.

It still does not survive my mmap stress-tester, so I'll keep looking.

Why do we need vm_regions for anonymous memory? Wouldn't it be enough to just have a VMA?

Bernd

Signature · 4 lines

This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

mm/nommu.c +58 ~~-38~~

diff --width=180 -x '*~' -x '.*' -x 'Sys*' -dru linux-2.6.x-daves-baseline/mm/nommu.c linux-2.6.x/mm/nommu.c
@@ -922,19 +931,23 @@
 	total = 1 << order;
 	atomic_add(total, &mmap_pages_allocated);
 
-	point = len >> PAGE_SHIFT;
-	while (point < total) {
-		order = ilog2(total - point);
-		_debug("shave %u/%lu", 1 << order, total - point);
-		atomic_sub(1 << order, &mmap_pages_allocated);
-		__free_pages(pages + point, order);
-		point += 1 << order;
+	len = PAGE_SIZE << order;
+#if 0
+	while (total > (len >> PAGE_SHIFT)) {
+		unsigned long to_free;
+		order = ilog2(total - (len >> PAGE_SHIFT));
+		to_free = 1 << order;
+		_debug("shave %u/%lu", to_free, total - (len >> PAGE_SHIFT));
+		atomic_sub(to_free, &mmap_pages_allocated);
+		__free_pages(pages + total - to_free, order);
+		total -= to_free;
 	}
 
 	total = len >> PAGE_SHIFT;
 	for (point = 1; point < total; point++)
 		set_page_refcounted(&pages[point]);
-
+#endif
+	split_page(pages, order);
 	base = page_address(pages);
 	region->vm_start = vma->vm_start = (unsigned long) base;
 	region->vm_end   = vma->vm_end   = vma->vm_start + len;

@@ -1294,6 +1307,7 @@
 		      unsigned long from, unsigned long to)
 {
 	struct vm_region *region;
+	struct mm_struct *mm = vma->vm_mm;
 
 	_enter("");

@@ -1304,7 +1318,7 @@
 		vma->vm_end = from;
 	else
 		vma->vm_start = to;
-	add_vma_to_mm(vma->vm_mm, vma);
+	add_vma_to_mm(mm, vma);
 
 	/* cut the region down to size */
 	region = vma->vm_region;

@@ -1337,44 +1351,59 @@
 
 	_enter(",%lx,%zx", start, len);
 
-	/* find the first potentially overlapping VMA */
-	vma = find_vma(mm, start);
-	if (!vma) {
-		_leave(" = -EINVAL [no]");
-		return -EINVAL;
-	}
-
-	/* we're allowed to split an anonymous VMA but not a file-backed one */
-	if (vma->vm_file) {
-		do {
-			if (start > vma->vm_start) {
-				_leave(" = -EINVAL [miss]");
-				return -EINVAL;
-			}
-			if (end == vma->vm_end)
-				goto erase_whole_vma;
-			rb = rb_next(&vma->vm_rb);
-			vma = rb_entry(rb, struct vm_area_struct, vm_rb);
-		} while (rb);
-		_leave(" = -EINVAL [split file]");
-		return -EINVAL;
-	} else {
-		/* the region must be a subset of the VMA found */
-		if (start == vma->vm_start && end == vma->vm_end)
-			goto erase_whole_vma;
-		if (start < vma->vm_start || end > vma->vm_end) {
+	while (start < end) {
+		unsigned long this_end;
+		/* find the first potentially overlapping VMA */
+		vma = find_vma(mm, start);
+		if (!vma) {
+			_leave(" = -EINVAL [no]");
+			return -EINVAL;
+		}
+		if (start < vma->vm_start) {
 			_leave(" = -EINVAL [superset]");
 			return -EINVAL;
 		}
-		if (start != vma->vm_start && end != vma->vm_end) {
+
+		this_end = vma->vm_end;
+
+		/* See if we can delete the entire VMA.  */
+		if (start == vma->vm_start && end >= this_end) {
+			delete_vma_from_mm(vma);
+			put_vma(vma);
+			start = this_end;
+			continue;
+		}
+
+		/* we're allowed to split an anonymous VMA but not a file-backed one */
+		if (vma->vm_file) {
+			do {
+				if (start > vma->vm_start) {
+					_leave(" = -EINVAL [miss]");
+					return -EINVAL;
+				}
+				if (end == vma->vm_end)
+					goto erase_whole_vma;
+				rb = rb_next(&vma->vm_rb);
+				vma = rb_entry(rb, struct vm_area_struct, vm_rb);
+			} while (rb);
+			_leave(" = -EINVAL [split file]");
+			return -EINVAL;
+		} else {
+			if (start > vma->vm_start && end >= this_end) {
+				shrink_vma(vma, start, this_end);
+				start = this_end;
+			} else if (start == vma->vm_start && end < this_end) {
+				shrink_vma(vma, start, end);
+				break;
+			}
 			ret = split_vma(mm, vma, start, 1);
 			if (ret < 0) {
 				_leave(" = %d [split]", ret);
 				return ret;
-			}
+			}
 		}
-		return shrink_vma(vma, start, end);
 	}
+	return 0;
 
 erase_whole_vma:
 	delete_vma_from_mm(vma);

David Howells

2007-08-20 15:13 UTC #22 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

Quoted 2 lines · Bernd Schmidt

In do_mmap_private, I've commented out the logic to free excess pages, as it fragments terribly

I wonder if there's a good heuristic for this. The problem is that whilst not releasing excess pages _may_ seem like a good idea, if your system is something like a single persistent app, then it really is not.

For instance, if such an app allocates a byte over 16MB (perhaps implicitly in the binfmt driver), then you'd completely waste a large chunk of RAM. In the 16MB+1 case, the wastage would be a byte less than 16MB.

Quoted 3 lines

and causes a simple
while true; do cat /proc/buddyinfo; done
loop to go oom.

Are you sure it's not just another leak?

Quoted 2 lines

Also, I think you're freeing high-order pages unaligned to their order?

Yeah, but some of the pages might still be in use when we want to release them.

Quoted 2 lines

In shrink_vma, we must save the mm across calls to remove_vma_from_mm (oops when telnetting into the box).

I'll have a look, but I don't see that.

Quoted 3 lines

In do_munmap, we can deal with freeing more than one vma. I've not touched the rb-tree logic in the shared file case, as I have no idea what it's trying to do given that only exact matches are allowed.

I'd generally rather not do this. You can't use MAP_FIXED to request adjacent regions, so why should you anticipate there would be any?

Quoted 1 line

It still does not survive my mmap stress-tester, so I'll keep looking.

Thanks.

Quoted 2 lines

Why do we need vm_regions for anonymous memory? Wouldn't it be enough to just have a VMA?

It makes it simpler to have a common way of allocating memory for both anon regions and file regions.

David

Bernd Schmidt

author

2007-08-20 16:03 UTC #23 lore raw

Re: [PATCH] NOMMU: Separate out VMAs

Quoted 5 lines · David Howells

Quoted 2 lines · Bernd Schmidt
In do_mmap_private, I've commented out the logic to free excess pages, as it fragments terribly

Quoted 7 lines

I wonder if there's a good heuristic for this. The problem is that whilst not releasing excess pages _may_ seem like a good idea, if your system is something like a single persistent app, then it really is not.
For instance, if such an app allocates a byte over 16MB (perhaps implicitly in the binfmt driver), then you'd completely waste a large chunk of RAM. In the 16MB+1 case, the wastage would be a byte less than 16MB.

I think it would be good to have a mechanism to group free pages by purpose - so that if we break up a high-order page in order to allocate memory for process A, then the remaining pages remain in a special pool that the allocator will prefer to hand out only to process A.

Quoted 5 lines

Quoted 2 lines
Also, I think you're freeing high-order pages unaligned to their order?
Yeah, but some of the pages might still be in use when we want to release them.

Not following you here. Is it valid to free an order-2 page that's not aligned at order-2?

Quoted 6 lines

Quoted 3 lines
In do_munmap, we can deal with freeing more than one vma. I've not touched the rb-tree logic in the shared file case, as I have no idea what it's trying to do given that only exact matches are allowed.
I'd generally rather not do this. You can't use MAP_FIXED to request adjacent regions, so why should you anticipate there would be any?

Adjacent regions can happen by accident, and the uClibc malloc will try to merge free areas when they are adjacent - there's a lot of special-case code in there to prevent this on uClinux systems by essentially duplicating VMA tracking. That's something we want to avoid, because it eats performance (especially in threaded apps due to additional locking).

Bernd

Signature · 4 lines

This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif