Push down page queues lock acquisition in pmap_enter_object() and

pmap_is_referenced().  Eliminate the corresponding page queues lock
acquisitions from vm_map_pmap_enter() and mincore(), respectively.  In
mincore(), this allows some additional cases to complete without ever
acquiring the page queues lock.

Assert that the page is managed in pmap_is_referenced().

On powerpc/aim, push down the page queues lock acquisition from
moea*_is_modified() and moea*_is_referenced() into moea*_query_bit().
Again, this will allow some additional cases to complete without ever
acquiring the page queues lock.

Reorder a few statements in vm_page_dontneed() so that a race can't lead
to an old reference persisting.  This scenario is described in detail by a
comment.

Correct a spelling error in vm_page_dontneed().

Assert that the object is locked in vm_page_clear_dirty(), and restrict the
page queues lock assertion to just those cases in which the page is
currently writeable.

Add object locking to vnode_pager_generic_putpages().  This was the one
and only place where vm_page_clear_dirty() was being called without the
object being locked.

Eliminate an unnecessary vm_page_lock() around vnode_pager_setsize()'s call
to vm_page_clear_dirty().

Change vnode_pager_generic_putpages() to the modern-style of function
definition.  Also, change the name of one of the parameters to follow
virtual memory system naming conventions.

Reviewed by:	kib

4 files changed, 45 insertions, 38 deletions

diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index 2a57e33b6e50..3ef93a371123 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
@@ -1726,7 +1726,6 @@ vm_map_pmap_enter(vm_map_t map, vm_offset_t addr, vm_prot_t prot,
 	vm_offset_t start;
 	vm_page_t p, p_start;
 	vm_pindex_t psize, tmpidx;
-	boolean_t are_queues_locked;
 
 	if ((prot & (VM_PROT_READ | VM_PROT_EXECUTE)) == 0 || object == NULL)
 		return;
@@ -1748,7 +1747,6 @@ vm_map_pmap_enter(vm_map_t map, vm_offset_t addr, vm_prot_t prot,
 		psize = object->size - pindex;
 	}
 
-	are_queues_locked = FALSE;
 	start = 0;
 	p_start = NULL;
 
@@ -1782,25 +1780,14 @@ vm_map_pmap_enter(vm_map_t map, vm_offset_t addr, vm_prot_t prot,
 				p_start = p;
 			}
 		} else if (p_start != NULL) {
-			if (!are_queues_locked) {
-				are_queues_locked = TRUE;
-				vm_page_lock_queues();
-			}
 			pmap_enter_object(map->pmap, start, addr +
 			    ptoa(tmpidx), p_start, prot);
 			p_start = NULL;
 		}
 	}
-	if (p_start != NULL) {
-		if (!are_queues_locked) {
-			are_queues_locked = TRUE;
-			vm_page_lock_queues();
-		}
+	if (p_start != NULL)
 		pmap_enter_object(map->pmap, start, addr + ptoa(psize),
 		    p_start, prot);
-	}
-	if (are_queues_locked)
-		vm_page_unlock_queues();
 unlock_return:
 	VM_OBJECT_UNLOCK(object);
 }
diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c
index 99d505907493..3d72123caddf 100644
--- a/sys/vm/vm_mmap.c
+++ b/sys/vm/vm_mmap.c
@@ -912,11 +912,18 @@ RestartScan:
 					vm_page_dirty(m);
 				if (m->dirty != 0)
 					mincoreinfo |= MINCORE_MODIFIED_OTHER;
-				vm_page_lock_queues();
+				/*
+				 * The first test for PG_REFERENCED is an
+				 * optimization.  The second test is
+				 * required because a concurrent pmap
+				 * operation could clear the last reference
+				 * and set PG_REFERENCED before the call to
+				 * pmap_is_referenced(). 
+				 */
 				if ((m->flags & PG_REFERENCED) != 0 ||
-				    pmap_is_referenced(m))
+				    pmap_is_referenced(m) ||
+				    (m->flags & PG_REFERENCED) != 0)
 					mincoreinfo |= MINCORE_REFERENCED_OTHER;
-				vm_page_unlock_queues();
 			}
 			if (object != NULL)
 				VM_OBJECT_UNLOCK(object);
diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c
index 060827ae85e9..1ff371afb8c0 100644
--- a/sys/vm/vm_page.c
+++ b/sys/vm/vm_page.c
@@ -1894,7 +1894,7 @@ vm_page_dontneed(vm_page_t m)
 	PCPU_INC(dnweight);
 
 	/*
-	 * occassionally leave the page alone
+	 * Occasionally leave the page alone.
 	 */
 	if ((dnw & 0x01F0) == 0 ||
 	    VM_PAGE_INQUEUE2(m, PQ_INACTIVE)) {
@@ -1906,11 +1906,18 @@ vm_page_dontneed(vm_page_t m)
 	/*
 	 * Clear any references to the page.  Otherwise, the page daemon will
 	 * immediately reactivate the page.
+	 *
+	 * Perform the pmap_clear_reference() first.  Otherwise, a concurrent
+	 * pmap operation, such as pmap_remove(), could clear a reference in
+	 * the pmap and set PG_REFERENCED on the page before the
+	 * pmap_clear_reference() had completed.  Consequently, the page would
+	 * appear referenced based upon an old reference that occurred before
+	 * this function ran.
 	 */
+	pmap_clear_reference(m);
 	vm_page_lock_queues();
 	vm_page_flag_clear(m, PG_REFERENCED);
 	vm_page_unlock_queues();
-	pmap_clear_reference(m);
 
 	if (m->dirty == 0 && pmap_is_modified(m))
 		vm_page_dirty(m);
@@ -2142,7 +2149,9 @@ void
 vm_page_clear_dirty(vm_page_t m, int base, int size)
 {
 
-	mtx_assert(&vm_page_queue_mtx, MA_OWNED);
+	VM_OBJECT_LOCK_ASSERT(m->object, MA_OWNED);
+	if ((m->flags & PG_WRITEABLE) != 0)
+		mtx_assert(&vm_page_queue_mtx, MA_OWNED);
 	m->dirty &= ~vm_page_bits(base, size);
 }
 
diff --git a/sys/vm/vnode_pager.c b/sys/vm/vnode_pager.c
index cfe9f994e0e3..c1f8cff5867a 100644
--- a/sys/vm/vnode_pager.c
+++ b/sys/vm/vnode_pager.c
@@ -429,11 +429,9 @@ vnode_pager_setsize(vp, nsize)
 			 * bits.  This would prevent bogus_page
 			 * replacement from working properly.
 			 */
-			vm_page_lock(m);
 			vm_page_lock_queues();
 			vm_page_clear_dirty(m, base, PAGE_SIZE - base);
 			vm_page_unlock_queues();
-			vm_page_unlock(m);
 		} else if ((nsize & PAGE_MASK) &&
 		    __predict_false(object->cache != NULL)) {
 			vm_page_cache_free(object, OFF_TO_IDX(nsize),
@@ -1071,15 +1069,12 @@ vnode_pager_putpages(object, m, count, sync, rtvals)
  * then delayed.
  */
 int
-vnode_pager_generic_putpages(vp, m, bytecount, flags, rtvals)
-	struct vnode *vp;
-	vm_page_t *m;
-	int bytecount;
-	int flags;
-	int *rtvals;
+vnode_pager_generic_putpages(struct vnode *vp, vm_page_t *ma, int bytecount,
+    int flags, int *rtvals)
 {
 	int i;
 	vm_object_t object;
+	vm_page_t m;
 	int count;
 
 	int maxsize, ncount;
@@ -1098,9 +1093,9 @@ vnode_pager_generic_putpages(vp, m, bytecount, flags, rtvals)
 	for (i = 0; i < count; i++)
 		rtvals[i] = VM_PAGER_AGAIN;
 
-	if ((int64_t)m[0]->pindex < 0) {
+	if ((int64_t)ma[0]->pindex < 0) {
 		printf("vnode_pager_putpages: attempt to write meta-data!!! -- 0x%lx(%lx)\n",
-			(long)m[0]->pindex, (u_long)m[0]->dirty);
+		    (long)ma[0]->pindex, (u_long)ma[0]->dirty);
 		rtvals[0] = VM_PAGER_BAD;
 		return VM_PAGER_BAD;
 	}
@@ -1108,7 +1103,7 @@ vnode_pager_generic_putpages(vp, m, bytecount, flags, rtvals)
 	maxsize = count * PAGE_SIZE;
 	ncount = count;
 
-	poffset = IDX_TO_OFF(m[0]->pindex);
+	poffset = IDX_TO_OFF(ma[0]->pindex);
 
 	/*
 	 * If the page-aligned write is larger then the actual file we
@@ -1122,6 +1117,7 @@ vnode_pager_generic_putpages(vp, m, bytecount, flags, rtvals)
 	 * We do not under any circumstances truncate the valid bits, as
 	 * this will screw up bogus page replacement.
 	 */
+	VM_OBJECT_LOCK(object);
 	if (maxsize + poffset > object->un_pager.vnp.vnp_size) {
 		if (object->un_pager.vnp.vnp_size > poffset) {
 			int pgoff;
@@ -1129,12 +1125,19 @@ vnode_pager_generic_putpages(vp, m, bytecount, flags, rtvals)
 			maxsize = object->un_pager.vnp.vnp_size - poffset;
 			ncount = btoc(maxsize);
 			if ((pgoff = (int)maxsize & PAGE_MASK) != 0) {
-				vm_page_lock(m[ncount - 1]);
-				vm_page_lock_queues();
-				vm_page_clear_dirty(m[ncount - 1], pgoff,
-					PAGE_SIZE - pgoff);
-				vm_page_unlock_queues();
-				vm_page_unlock(m[ncount - 1]);
+				/*
+				 * If the object is locked and the following
+				 * conditions hold, then the page's dirty
+				 * field cannot be concurrently changed by a
+				 * pmap operation.
+				 */
+				m = ma[ncount - 1];
+				KASSERT(m->busy > 0,
+		("vnode_pager_generic_putpages: page %p is not busy", m));
+				KASSERT((m->flags & PG_WRITEABLE) == 0,
+		("vnode_pager_generic_putpages: page %p is not read-only", m));
+				vm_page_clear_dirty(m, pgoff, PAGE_SIZE -
+				    pgoff);
 			}
 		} else {
 			maxsize = 0;
@@ -1146,6 +1149,7 @@ vnode_pager_generic_putpages(vp, m, bytecount, flags, rtvals)
 			}
 		}
 	}
+	VM_OBJECT_UNLOCK(object);
 
 	/*
 	 * pageouts are already clustered, use IO_ASYNC t o force a bawrite()
@@ -1182,7 +1186,7 @@ vnode_pager_generic_putpages(vp, m, bytecount, flags, rtvals)
 	if (auio.uio_resid) {
 		if (ppscheck || ppsratecheck(&lastfail, &curfail, 1))
 			printf("vnode_pager_putpages: residual I/O %zd at %lu\n",
-			    auio.uio_resid, (u_long)m[0]->pindex);
+			    auio.uio_resid, (u_long)ma[0]->pindex);
 	}
 	for (i = 0; i < ncount; i++) {
 		rtvals[i] = VM_PAGER_OK;