Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 10 May 2014 17:03:33 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r265843 - head/sys/vm
Message-ID:  <201405101703.s4AH3Xi9028568@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Sat May 10 17:03:33 2014
New Revision: 265843
URL: http://svnweb.freebsd.org/changeset/base/265843

Log:
  For the upgrade case in vm_fault_copy_entry(), when the entry does not
  need COW and is writeable (i.e. becoming writeable due to the
  mprotect(2) operation), do not create a new backing object for the
  entry.  The caller of the function is vm_map_protect(), the call is
  made to ensure that wired entry has all pages resident and wired in
  the top level object and to enable the write.  We might need to copy
  read-only page from some backing objects into the top object or remap
  the page with the write allowed.
  
  This fixes the issue with mishandling of the swap accounting when
  read-only wired mapping is upgraded to write-enabled after fork.  The
  previous code path did not accounted the new object, but it creation
  is redundand anyway and the change provides an optimization for the
  non-common situation.
  
  Reported by:	markj
  Suggested and reviewed by:	alc (previous version)
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/sys/vm/vm_fault.c

Modified: head/sys/vm/vm_fault.c
==============================================================================
--- head/sys/vm/vm_fault.c	Sat May 10 16:59:15 2014	(r265842)
+++ head/sys/vm/vm_fault.c	Sat May 10 17:03:33 2014	(r265843)
@@ -1247,38 +1247,48 @@ vm_fault_copy_entry(vm_map_t dst_map, vm
 #endif	/* lint */
 
 	upgrade = src_entry == dst_entry;
+	access = prot = dst_entry->protection;
 
 	src_object = src_entry->object.vm_object;
 	src_pindex = OFF_TO_IDX(src_entry->offset);
 
-	/*
-	 * Create the top-level object for the destination entry. (Doesn't
-	 * actually shadow anything - we copy the pages directly.)
-	 */
-	dst_object = vm_object_allocate(OBJT_DEFAULT,
-	    OFF_TO_IDX(dst_entry->end - dst_entry->start));
+	if (upgrade && (dst_entry->eflags & MAP_ENTRY_NEEDS_COPY) == 0) {
+		dst_object = src_object;
+		vm_object_reference(dst_object);
+	} else {
+		/*
+		 * Create the top-level object for the destination entry. (Doesn't
+		 * actually shadow anything - we copy the pages directly.)
+		 */
+		dst_object = vm_object_allocate(OBJT_DEFAULT,
+		    OFF_TO_IDX(dst_entry->end - dst_entry->start));
 #if VM_NRESERVLEVEL > 0
-	dst_object->flags |= OBJ_COLORED;
-	dst_object->pg_color = atop(dst_entry->start);
+		dst_object->flags |= OBJ_COLORED;
+		dst_object->pg_color = atop(dst_entry->start);
 #endif
+	}
 
 	VM_OBJECT_WLOCK(dst_object);
 	KASSERT(upgrade || dst_entry->object.vm_object == NULL,
 	    ("vm_fault_copy_entry: vm_object not NULL"));
-	dst_entry->object.vm_object = dst_object;
-	dst_entry->offset = 0;
-	dst_object->charge = dst_entry->end - dst_entry->start;
+	if (src_object != dst_object) {
+		dst_entry->object.vm_object = dst_object;
+		dst_entry->offset = 0;
+		dst_object->charge = dst_entry->end - dst_entry->start;
+	}
 	if (fork_charge != NULL) {
 		KASSERT(dst_entry->cred == NULL,
 		    ("vm_fault_copy_entry: leaked swp charge"));
 		dst_object->cred = curthread->td_ucred;
 		crhold(dst_object->cred);
 		*fork_charge += dst_object->charge;
-	} else {
+	} else if (dst_object->cred == NULL) {
+		KASSERT(dst_entry->cred != NULL, ("no cred for entry %p",
+		    dst_entry));
 		dst_object->cred = dst_entry->cred;
 		dst_entry->cred = NULL;
 	}
-	access = prot = dst_entry->protection;
+
 	/*
 	 * If not an upgrade, then enter the mappings in the pmap as
 	 * read and/or execute accesses.  Otherwise, enter them as
@@ -1304,26 +1314,14 @@ vm_fault_copy_entry(vm_map_t dst_map, vm
 	for (vaddr = dst_entry->start, dst_pindex = 0;
 	    vaddr < dst_entry->end;
 	    vaddr += PAGE_SIZE, dst_pindex++) {
-
-		/*
-		 * Allocate a page in the destination object.
-		 */
-		do {
-			dst_m = vm_page_alloc(dst_object, dst_pindex,
-			    VM_ALLOC_NORMAL);
-			if (dst_m == NULL) {
-				VM_OBJECT_WUNLOCK(dst_object);
-				VM_WAIT;
-				VM_OBJECT_WLOCK(dst_object);
-			}
-		} while (dst_m == NULL);
-
+again:
 		/*
 		 * Find the page in the source object, and copy it in.
 		 * Because the source is wired down, the page will be
 		 * in memory.
 		 */
-		VM_OBJECT_RLOCK(src_object);
+		if (src_object != dst_object)
+			VM_OBJECT_RLOCK(src_object);
 		object = src_object;
 		pindex = src_pindex + dst_pindex;
 		while ((src_m = vm_page_lookup(object, pindex)) == NULL &&
@@ -1343,14 +1341,39 @@ vm_fault_copy_entry(vm_map_t dst_map, vm
 
 			VM_OBJECT_RLOCK(backing_object);
 			pindex += OFF_TO_IDX(object->backing_object_offset);
-			VM_OBJECT_RUNLOCK(object);
+			if (object != dst_object)
+				VM_OBJECT_RUNLOCK(object);
 			object = backing_object;
 		}
 		KASSERT(src_m != NULL, ("vm_fault_copy_entry: page missing"));
-		pmap_copy_page(src_m, dst_m);
-		VM_OBJECT_RUNLOCK(object);
-		dst_m->valid = VM_PAGE_BITS_ALL;
-		dst_m->dirty = VM_PAGE_BITS_ALL;
+
+		if (object != dst_object) {
+			/*
+			 * Allocate a page in the destination object.
+			 */
+			do {
+				dst_m = vm_page_alloc(dst_object,
+				    (src_object == dst_object ? src_pindex :
+				    0) + dst_pindex, VM_ALLOC_NORMAL);
+				if (dst_m == NULL) {
+					VM_OBJECT_WUNLOCK(dst_object);
+					VM_OBJECT_RUNLOCK(object);
+					VM_WAIT;
+					goto again;
+				}
+			} while (dst_m == NULL);
+			pmap_copy_page(src_m, dst_m);
+			VM_OBJECT_RUNLOCK(object);
+			dst_m->valid = VM_PAGE_BITS_ALL;
+			dst_m->dirty = VM_PAGE_BITS_ALL;
+		} else {
+			dst_m = src_m;
+			if (vm_page_sleep_if_busy(dst_m, "fltupg"))
+				goto again;
+			vm_page_xbusy(dst_m);
+			KASSERT(dst_m->valid == VM_PAGE_BITS_ALL,
+			    ("invalid dst page %p", dst_m));
+		}
 		VM_OBJECT_WUNLOCK(dst_object);
 
 		/*
@@ -1366,13 +1389,17 @@ vm_fault_copy_entry(vm_map_t dst_map, vm
 		VM_OBJECT_WLOCK(dst_object);
 		
 		if (upgrade) {
-			vm_page_lock(src_m);
-			vm_page_unwire(src_m, 0);
-			vm_page_unlock(src_m);
-
-			vm_page_lock(dst_m);
-			vm_page_wire(dst_m);
-			vm_page_unlock(dst_m);
+			if (src_m != dst_m) {
+				vm_page_lock(src_m);
+				vm_page_unwire(src_m, 0);
+				vm_page_unlock(src_m);
+				vm_page_lock(dst_m);
+				vm_page_wire(dst_m);
+				vm_page_unlock(dst_m);
+			} else {
+				KASSERT(dst_m->wire_count > 0,
+				    ("dst_m %p is not wired", dst_m));
+			}
 		} else {
 			vm_page_lock(dst_m);
 			vm_page_activate(dst_m);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201405101703.s4AH3Xi9028568>