Re: [PATCH] NOMMU: Separate out VMAs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



David Howells wrote:
David Howells <[email protected]> wrote:

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
--
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
diff --width=180 -x '*~' -x '.*' -x 'Sys*' -dru linux-2.6.x-daves-baseline/mm/nommu.c linux-2.6.x/mm/nommu.c
--- linux-2.6.x-daves-baseline/mm/nommu.c	2007-08-14 21:20:43.000000000 +0200
+++ linux-2.6.x/mm/nommu.c	2007-08-16 19:11:29.000000000 +0200
@@ -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);

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux