The use of uvm.pagedaemon_lock is incredibly inconsistent. only a

fraction of the wakeups and sleeps involved here actually grab that
lock. The remainder, on the other hand, always have the fpageq_lock
locked.

So, make this locking correct by switching the other users over to
fpageq_lock, too.

This would probably be better off being a semaphore, but for now at
least it's correct.

"ok, unless you want to implement semaphores" art@

4 files changed, 22 insertions, 30 deletions

diff --git a/sys/uvm/uvm.h b/sys/uvm/uvm.h
index 22e9323d15d..cb447f87f88 100644
--- a/sys/uvm/uvm.h
+++ b/sys/uvm/uvm.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm.h,v 1.29 2009/04/13 22:17:54 oga Exp $	*/
+/*	$OpenBSD: uvm.h,v 1.30 2009/04/14 20:12:05 oga Exp $	*/
 /*	$NetBSD: uvm.h,v 1.24 2000/11/27 08:40:02 chs Exp $	*/
 
 /*
@@ -81,8 +81,8 @@ struct uvm {
 	struct pglist page_inactive_swp;/* pages inactive (reclaim or free) */
 	struct pglist page_inactive_obj;/* pages inactive (reclaim or free) */
 	/* Lock order: object lock,  pageqlock, then fpageqlock. */
-	struct mutex pageqlock;	/* lock for active/inactive page q */
-	struct mutex fpageqlock;	/* lock for free page q */
+	struct mutex pageqlock;		/* lock for active/inactive page q */
+	struct mutex fpageqlock;	/* lock for free page q  + pdaemon */
 	boolean_t page_init_done;	/* TRUE if uvm_page_init() finished */
 	boolean_t page_idle_zero;	/* TRUE if we should try to zero
 					   pages in the idle loop */
@@ -90,7 +90,6 @@ struct uvm {
 		/* page daemon trigger */
 	int pagedaemon;			/* daemon sleeps on this */
 	struct proc *pagedaemon_proc;	/* daemon's pid */
-	simple_lock_data_t pagedaemon_lock;
 
 		/* aiodone daemon trigger */
 	int aiodoned;			/* daemon sleeps on this */
diff --git a/sys/uvm/uvm_page.c b/sys/uvm/uvm_page.c
index 3b2ea4c3ba4..d00119383b7 100644
--- a/sys/uvm/uvm_page.c
+++ b/sys/uvm/uvm_page.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_page.c,v 1.74 2009/04/13 22:17:54 oga Exp $	*/
+/*	$OpenBSD: uvm_page.c,v 1.75 2009/04/14 20:12:05 oga Exp $	*/
 /*	$NetBSD: uvm_page.c,v 1.44 2000/11/27 08:40:04 chs Exp $	*/
 
 /* 
@@ -337,8 +337,6 @@ uvm_page_init(vaddr_t *kvm_startp, vaddr_t *kvm_endp)
 	/*
 	 * init locks for kernel threads
 	 */
-
-	simple_lock_init(&uvm.pagedaemon_lock);
 	mtx_init(&uvm.aiodoned_lock, IPL_BIO);
 
 	/*
diff --git a/sys/uvm/uvm_pdaemon.c b/sys/uvm/uvm_pdaemon.c
index bcfde9be6df..c64530bcac2 100644
--- a/sys/uvm/uvm_pdaemon.c
+++ b/sys/uvm/uvm_pdaemon.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_pdaemon.c,v 1.39 2009/04/13 22:17:54 oga Exp $	*/
+/*	$OpenBSD: uvm_pdaemon.c,v 1.40 2009/04/14 20:12:05 oga Exp $	*/
 /*	$NetBSD: uvm_pdaemon.c,v 1.23 2000/08/20 10:24:14 bjh21 Exp $	*/
 
 /* 
@@ -110,8 +110,7 @@ static void		uvmpd_tune(void);
 void
 uvm_wait(const char *wmsg)
 {
-	int timo = 0;
-	int s = splbio();
+	int	timo = 0;
 
 	/*
 	 * check for page daemon going to sleep (waiting for itself)
@@ -143,12 +142,9 @@ uvm_wait(const char *wmsg)
 #endif
 	}
 
-	simple_lock(&uvm.pagedaemon_lock);
+	uvm_lock_fpageq();
 	wakeup(&uvm.pagedaemon);		/* wake the daemon! */
-	UVM_UNLOCK_AND_WAIT(&uvmexp.free, &uvm.pagedaemon_lock, FALSE, wmsg,
-	    timo);
-
-	splx(s);
+	msleep(&uvmexp.free, &uvm.fpageqlock, PVM | PNORELOCK, wmsg, timo);
 }
 
 
@@ -216,11 +212,10 @@ uvm_pageout(void *arg)
 	 */
 
 	for (;;) {
-		simple_lock(&uvm.pagedaemon_lock);
-
+		uvm_lock_fpageq();
 		UVMHIST_LOG(pdhist,"  <<SLEEPING>>",0,0,0,0);
-		UVM_UNLOCK_AND_WAIT(&uvm.pagedaemon,
-		    &uvm.pagedaemon_lock, FALSE, "pgdaemon", 0);
+		msleep(&uvm.pagedaemon, &uvm.fpageqlock, PVM | PNORELOCK,
+		    "pgdaemon", 0);
 		uvmexp.pdwoke++;
 		UVMHIST_LOG(pdhist,"  <<WOKE UP>>",0,0,0,0);
 
@@ -255,11 +250,12 @@ uvm_pageout(void *arg)
 		 * if there's any free memory to be had,
 		 * wake up any waiters.
 		 */
-
+		uvm_lock_fpageq();
 		if (uvmexp.free > uvmexp.reserve_kernel ||
 		    uvmexp.paging == 0) {
 			wakeup(&uvmexp.free);
 		}
+		uvm_unlock_fpageq();
 
 		/*
 		 * scan done.  unlock page queues (the only lock we are holding)
@@ -313,15 +309,10 @@ uvm_aiodone_daemon(void *arg)
 			splx(s);
 			bp = nbp;
 		}
-		if (free <= uvmexp.reserve_kernel) {
-			uvm_lock_fpageq();
-			wakeup(&uvm.pagedaemon);
-			uvm_unlock_fpageq();
-		} else {
-			simple_lock(&uvm.pagedaemon_lock);
-			wakeup(&uvmexp.free);
-			simple_unlock(&uvm.pagedaemon_lock);
-		}
+		uvm_lock_fpageq();
+		wakeup(free <= uvmexp.reserve_kernel ? &uvm.pagedaemon :
+		    &uvmexp.free);
+		uvm_unlock_fpageq();
 	}
 }
 
diff --git a/sys/uvm/uvm_vnode.c b/sys/uvm/uvm_vnode.c
index 1e06956e93e..1f6cfd157bf 100644
--- a/sys/uvm/uvm_vnode.c
+++ b/sys/uvm/uvm_vnode.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_vnode.c,v 1.55 2009/04/13 22:17:54 oga Exp $	*/
+/*	$OpenBSD: uvm_vnode.c,v 1.56 2009/04/14 20:12:05 oga Exp $	*/
 /*	$NetBSD: uvm_vnode.c,v 1.36 2000/11/24 20:34:01 chs Exp $	*/
 
 /*
@@ -1164,6 +1164,10 @@ ReTry:
 						atomic_setbits_int(
 						    &ptmp->pg_flags,
 						    PG_RELEASED);
+				/*
+				 * XXX if ! busy, io is already done. shouldn't
+				 * XXX we free the pages ourselves?
+				 */
 				} else {
 					if (result != VM_PAGER_OK) {
 						printf("uvn_flush: obj=%p, "