Replaced the page queue mutexes by spinlocks. The critical sections are very

short and quite hot, so mutexes just cause more overhead due to frequent
rescheduling than waiting for the spinlocks does. The free and clear queues
are additionally protected by a R/W lock, which is mostly read-locked, save
for rare cases like allocating page runs.

The total -j8 Haiku image build speedup is marginal. The kernel time drops
about 8%, though.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@35004 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/system/kernel/vm/VMPageQueue.cpp

@@ -7,11 +7,16 @@
 #include "VMPageQueue.h"
 
 
+// #pragma mark - VMPageQueue
+
+
 void
-VMPageQueue::Init(const char* name, int lockingOrder)
+VMPageQueue::Init(const char* name)
 {
+	new(&fPages) PageList;
+
+	B_INITIALIZE_SPINLOCK(&fLock);
+
 	fName = name;
-	fLockingOrder = lockingOrder;
 	fCount = 0;
-	mutex_init(&fLock, fName);
 }

src/system/kernel/vm/VMPageQueue.h

@@ -13,6 +13,8 @@
 #include <util/DoublyLinkedList.h>
 
 #include <lock.h>
+#include <int.h>
+#include <util/AutoLock.h>
 #include <vm/vm_types.h>
 
 
@@ -24,15 +26,9 @@ public:
 	typedef PageList::ConstIterator Iterator;
 
 public:
-			void				Init(const char* name, int lockingOrder);
+			void				Init(const char* name);
 
 			const char*			Name() const	{ return fName; }
-			int					LockingOrder() const { return fLockingOrder; }
-
-	inline	bool				Lock();
-	inline	void				Unlock();
-
-	inline	void				LockMultiple(VMPageQueue* other);
 
 	inline	void				Append(vm_page* page);
 	inline	void				Prepend(vm_page* page);
@@ -40,6 +36,13 @@ public:
 									vm_page* page);
 	inline	void				Remove(vm_page* page);
 	inline	vm_page*			RemoveHead();
+	inline	void				Requeue(vm_page* page, bool tail);
+
+	inline	void				AppendUnlocked(vm_page* page);
+	inline	void				PrependUnlocked(vm_page* page);
+	inline	void				RemoveUnlocked(vm_page* page);
+	inline	vm_page*			RemoveHeadUnlocked();
+	inline	void				RequeueUnlocked(vm_page* page, bool tail);
 
 	inline	vm_page*			Head() const;
 	inline	vm_page*			Tail() const;
@@ -50,40 +53,17 @@ public:
 
 	inline	Iterator			GetIterator() const;
 
-private:
+	inline	spinlock&			GetLock()	{ return fLock; }
+
+protected:
 			const char*			fName;
-			int					fLockingOrder;
+			spinlock			fLock;
-			mutex				fLock;
 			uint32				fCount;
 			PageList			fPages;
 };
 
 
-bool
+// #pragma mark - VMPageQueue
-VMPageQueue::Lock()
-{
-	return mutex_lock(&fLock) == B_OK;
-}
-
-
-void
-VMPageQueue::Unlock()
-{
-	mutex_unlock(&fLock);
-}
-
-
-void
-VMPageQueue::LockMultiple(VMPageQueue* other)
-{
-	if (fLockingOrder < other->fLockingOrder) {
-		Lock();
-		other->Lock();
-	} else {
-		other->Lock();
-		Lock();
-	}
-}
 
 
 void
@@ -183,6 +163,61 @@ VMPageQueue::RemoveHead()
 }
 
 
+void
+VMPageQueue::Requeue(vm_page* page, bool tail)
+{
+#if DEBUG_PAGE_QUEUE
+	if (page->queue != this) {
+		panic("%p->VMPageQueue::Requeue(): page %p thinks it is in "
+			"queue %p", this, page, page->queue);
+	}
+#endif
+
+	fPages.Remove(page);
+	fPages.Add(page, tail);
+}
+
+
+void
+VMPageQueue::AppendUnlocked(vm_page* page)
+{
+	InterruptsSpinLocker locker(fLock);
+	Append(page);
+}
+
+
+void
+VMPageQueue::PrependUnlocked(vm_page* page)
+{
+	InterruptsSpinLocker locker(fLock);
+	Prepend(page);
+}
+
+
+void
+VMPageQueue::RemoveUnlocked(vm_page* page)
+{
+	InterruptsSpinLocker locker(fLock);
+	return Remove(page);
+}
+
+
+vm_page*
+VMPageQueue::RemoveHeadUnlocked()
+{
+	InterruptsSpinLocker locker(fLock);
+	return RemoveHead();
+}
+
+
+void
+VMPageQueue::RequeueUnlocked(vm_page* page, bool tail)
+{
+	InterruptsSpinLocker locker(fLock);
+	Requeue(page, tail);
+}
+
+
 vm_page*
 VMPageQueue::Head() const
 {
@@ -218,87 +253,4 @@ VMPageQueue::GetIterator() const
 }
 
 
-// #pragma mark - VMPageQueuePairLocker
-
-
-struct VMPageQueuePairLocker {
-	VMPageQueuePairLocker()
-		:
-		fQueue1(NULL),
-		fQueue2(NULL)
-	{
-	}
-
-	VMPageQueuePairLocker(VMPageQueue& queue1, VMPageQueue& queue2)
-		:
-		fQueue1(&queue1),
-		fQueue2(&queue2)
-	{
-		_Lock();
-	}
-
-	~VMPageQueuePairLocker()
-	{
-		_Unlock();
-	}
-
-	void SetTo(VMPageQueue* queue1, VMPageQueue* queue2)
-	{
-		_Unlock();
-		fQueue1 = queue1;
-		fQueue2 = queue2;
-		_Lock();
-	}
-
-	void Unlock()
-	{
-		if (fQueue1 != NULL) {
-			fQueue1->Unlock();
-			fQueue1 = NULL;
-		}
-
-		if (fQueue2 != NULL) {
-			fQueue2->Unlock();
-			fQueue2 = NULL;
-		}
-	}
-
-private:
-	void _Lock()
-	{
-		if (fQueue1 == fQueue2) {
-			if (fQueue1 == NULL)
-				return;
-			fQueue1->Lock();
-			fQueue2 = NULL;
-		} else {
-			if (fQueue1 == NULL) {
-				fQueue2->Lock();
-			} else if (fQueue2 == NULL) {
-				fQueue1->Lock();
-			} else if (fQueue1->LockingOrder() < fQueue2->LockingOrder()) {
-				fQueue1->Lock();
-				fQueue2->Lock();
-			} else {
-				fQueue2->Lock();
-				fQueue1->Lock();
-			}
-		}
-	}
-
-	void _Unlock()
-	{
-		if (fQueue1 != NULL)
-			fQueue1->Unlock();
-
-		if (fQueue2 != NULL)
-			fQueue2->Unlock();
-	}
-
-private:
-	VMPageQueue*	fQueue1;
-	VMPageQueue*	fQueue2;
-};
-
-
 #endif	// VM_PAGE_QUEUE_H

src/system/kernel/vm/vm_page.cpp

@@ -77,6 +77,9 @@ static vint32 sModifiedTemporaryPages;
 static ConditionVariable sFreePageCondition;
 static mutex sPageDeficitLock = MUTEX_INITIALIZER("page deficit");
 
+static rw_lock sFreePageQueuesLock
+	= RW_LOCK_INITIALIZER("free/clear page queues");
+
 static sem_id sWriterWaitSem;
 
 
@@ -575,22 +578,13 @@ dump_page_stats(int argc, char **argv)
 }
 
 
-/*!	The caller must make sure that no-one else tries to change the page's state
-	while the function is called. If the page has a cache, this can be done by
-	locking the cache.
-*/
 static void
-set_page_state(vm_page *page, int pageState, bool queuesLocked)
+free_page(vm_page* page, bool clear)
 {
 	DEBUG_PAGE_ACCESS_CHECK(page);
 
-	if (pageState == page->state)
-		return;
-
 	VMPageQueue* fromQueue;
 
-	int32 freeCountDiff = 0;
-
 	switch (page->state) {
 		case PAGE_STATE_BUSY:
 		case PAGE_STATE_ACTIVE:
@@ -603,26 +597,90 @@ set_page_state(vm_page *page, int pageState, bool queuesLocked)
 			fromQueue = &sModifiedPageQueue;
 			break;
 		case PAGE_STATE_FREE:
-			fromQueue = &sFreePageQueue;
-			freeCountDiff = -1;
-			break;
 		case PAGE_STATE_CLEAR:
-			fromQueue = &sClearPageQueue;
+			panic("free_page(): page %p already free", page);
-			freeCountDiff = -1;
+			return;
-			break;
 		case PAGE_STATE_WIRED:
 		case PAGE_STATE_UNUSED:
 			fromQueue = NULL;
 			break;
 		default:
-			panic("set_page_state: vm_page %p in invalid state %d\n",
+			panic("free_page(): page %p in invalid state %d",
 				page, page->state);
 			return;
 	}
 
-	if (page->state == PAGE_STATE_CLEAR || page->state == PAGE_STATE_FREE) {
+	if (page->cache != NULL)
-		if (page->cache != NULL)
+		panic("to be freed page %p has cache", page);
-			panic("free page %p has cache", page);
+	if (!page->mappings.IsEmpty() || page->wired_count > 0)
+		panic("to be freed page %p has mappings", page);
+
+	if (sPageDeficit > 0) {
+		MutexLocker pageDeficitLocker(sPageDeficitLock);
+		if (sPageDeficit > 0)
+			sFreePageCondition.NotifyOne();
+	}
+
+	if (fromQueue != NULL)
+		fromQueue->RemoveUnlocked(page);
+
+	T(FreePage());
+
+	ReadLocker locker(sFreePageQueuesLock);
+
+	DEBUG_PAGE_ACCESS_END(page);
+
+	if (clear) {
+		page->state = PAGE_STATE_CLEAR;
+		sClearPageQueue.PrependUnlocked(page);
+	} else {
+		page->state = PAGE_STATE_FREE;
+		sFreePageQueue.PrependUnlocked(page);
+	}
+
+	locker.Unlock();
+
+	atomic_add(&sUnreservedFreePages, 1);
+}
+
+
+/*!	The caller must make sure that no-one else tries to change the page's state
+	while the function is called. If the page has a cache, this can be done by
+	locking the cache.
+*/
+static void
+set_page_state(vm_page *page, int pageState)
+{
+	DEBUG_PAGE_ACCESS_CHECK(page);
+
+	if (pageState == page->state)
+		return;
+
+	VMPageQueue* fromQueue;
+
+	switch (page->state) {
+		case PAGE_STATE_BUSY:
+		case PAGE_STATE_ACTIVE:
+			fromQueue = &sActivePageQueue;
+			break;
+		case PAGE_STATE_INACTIVE:
+			fromQueue = &sInactivePageQueue;
+			break;
+		case PAGE_STATE_MODIFIED:
+			fromQueue = &sModifiedPageQueue;
+			break;
+		case PAGE_STATE_FREE:
+		case PAGE_STATE_CLEAR:
+			panic("set_page_state(): page %p is free/clear", page);
+			return;
+		case PAGE_STATE_WIRED:
+		case PAGE_STATE_UNUSED:
+			fromQueue = NULL;
+			break;
+		default:
+			panic("set_page_state(): page %p in invalid state %d",
+				page, page->state);
+			return;
 	}
 
 	VMPageQueue* toQueue;
@@ -639,37 +697,24 @@ set_page_state(vm_page *page, int pageState, bool queuesLocked)
 			toQueue = &sModifiedPageQueue;
 			break;
 		case PAGE_STATE_FREE:
-			toQueue = &sFreePageQueue;
-			freeCountDiff++;
-			break;
 		case PAGE_STATE_CLEAR:
-			toQueue = &sClearPageQueue;
+			panic("set_page_state(): target state is free/clear");
-			freeCountDiff++;
+			return;
-			break;
 		case PAGE_STATE_WIRED:
 		case PAGE_STATE_UNUSED:
 			toQueue = NULL;
 			break;
 		default:
-			panic("set_page_state: invalid target state %d\n", pageState);
+			panic("set_page_state(): invalid target state %d", pageState);
 			return;
 	}
 
-	if (pageState == PAGE_STATE_CLEAR || pageState == PAGE_STATE_FREE
+	if (pageState == PAGE_STATE_INACTIVE && sPageDeficit > 0) {
-		|| pageState == PAGE_STATE_INACTIVE) {
+		MutexLocker pageDeficitLocker(sPageDeficitLock);
-		if (sPageDeficit > 0) {
+		if (sPageDeficit > 0)
-			MutexLocker pageDeficitLocker(sPageDeficitLock);
+			sFreePageCondition.NotifyOne();
-			if (sPageDeficit > 0)
-				sFreePageCondition.NotifyOne();
-		}
-
-		if (pageState != PAGE_STATE_INACTIVE) {
-			if (page->cache != NULL)
-				panic("to be freed page %p has cache", page);
-			if (!page->mappings.IsEmpty() || page->wired_count > 0)
-				panic("to be freed page %p has mappings", page);
-		}
 	}
+
 	if (page->cache != NULL && page->cache->temporary) {
 		if (pageState == PAGE_STATE_MODIFIED)
 			atomic_add(&sModifiedTemporaryPages, 1);
@@ -677,13 +722,6 @@ set_page_state(vm_page *page, int pageState, bool queuesLocked)
 			atomic_add(&sModifiedTemporaryPages, -1);
 	}
 
-#ifdef PAGE_ALLOCATION_TRACING
-	if ((pageState == PAGE_STATE_CLEAR || pageState == PAGE_STATE_FREE)
-		&& page->state != PAGE_STATE_CLEAR && page->state != PAGE_STATE_FREE) {
-		T(FreePage());
-	}
-#endif	// PAGE_ALLOCATION_TRACING
-
 	// move the page
 	if (toQueue == fromQueue) {
 		// Note: Theoretically we are required to lock when changing the page
@@ -696,28 +734,14 @@ set_page_state(vm_page *page, int pageState, bool queuesLocked)
 		page->cache->AssertLocked();
 		page->state = pageState;
 	} else {
-		VMPageQueuePairLocker locker;
-		if (!queuesLocked)
-			locker.SetTo(fromQueue, toQueue);
-
 		if (fromQueue != NULL)
-			fromQueue->Remove(page);
+			fromQueue->RemoveUnlocked(page);
 
 		page->state = pageState;
 
-		if (toQueue != NULL) {
+		if (toQueue != NULL)
-			if (pageState == PAGE_STATE_CLEAR || pageState == PAGE_STATE_FREE) {
+			toQueue->AppendUnlocked(page);
-				DEBUG_PAGE_ACCESS_END(page);
-				// prepend free/clear pages to be more cache friendly.
-				toQueue->Prepend(page);
-			} else
-				toQueue->Append(page);
-
-		}
 	}
-
-	if (freeCountDiff != 0)
-		atomic_add(&sUnreservedFreePages, freeCountDiff);
 }
 
 
@@ -742,13 +766,11 @@ move_page_to_active_or_inactive_queue(vm_page *page, bool dequeued)
 		page->state = state;
 		VMPageQueue& queue = state == PAGE_STATE_ACTIVE
 			? sActivePageQueue : sInactivePageQueue;
-		queue.Lock();
+		queue.AppendUnlocked(page);
-		queue.Append(page);
-		queue.Unlock();
 		if (page->cache->temporary)
 			atomic_add(&sModifiedTemporaryPages, -1);
 	} else
-		set_page_state(page, state, false);
+		set_page_state(page, state);
 }
 
 
@@ -788,12 +810,12 @@ page_scrubber(void *unused)
 			continue;
 
 		// get some pages from the free queue
-		AutoLocker<VMPageQueue> freeQueueLocker(sFreePageQueue);
+		ReadLocker locker(sFreePageQueuesLock);
 
 		vm_page *page[SCRUB_SIZE];
 		int32 scrubCount = 0;
 		for (int32 i = 0; i < SCRUB_SIZE; i++) {
-			page[i] = sFreePageQueue.RemoveHead();
+			page[i] = sFreePageQueue.RemoveHeadUnlocked();
 			if (page[i] == NULL)
 				break;
 
@@ -803,7 +825,7 @@ page_scrubber(void *unused)
 			scrubCount++;
 		}
 
-		freeQueueLocker.Unlock();
+		locker.Unlock();
 
 		if (scrubCount == 0) {
 			vm_page_unreserve_pages(SCRUB_SIZE);
@@ -816,16 +838,16 @@ page_scrubber(void *unused)
 		for (int32 i = 0; i < scrubCount; i++)
 			clear_page(page[i]);
 
-		AutoLocker<VMPageQueue> clearQueueLocker(sClearPageQueue);
+		locker.Lock();
 
 		// and put them into the clear queue
 		for (int32 i = 0; i < scrubCount; i++) {
 			page[i]->state = PAGE_STATE_CLEAR;
-			sClearPageQueue.Append(page[i]);
+			sClearPageQueue.PrependUnlocked(page[i]);
 			DEBUG_PAGE_ACCESS_END(page[i]);
 		}
 
-		clearQueueLocker.Unlock();
+		locker.Unlock();
 
 		vm_page_unreserve_pages(SCRUB_SIZE);
 
@@ -868,9 +890,7 @@ remove_page_marker(struct vm_page &marker)
 			return;
 	}
 
-	queue->Lock();
+	queue->RemoveUnlocked(&marker);
-	queue->Remove(&marker);
-	queue->Unlock();
 
 	marker.state = PAGE_STATE_UNUSED;
 }
@@ -879,7 +899,7 @@ remove_page_marker(struct vm_page &marker)
 static vm_page *
 next_modified_page(struct vm_page &marker)
 {
-	AutoLocker<VMPageQueue> locker(sModifiedPageQueue);
+	InterruptsSpinLocker locker(sModifiedPageQueue.GetLock());
 	vm_page *page;
 
 	DEBUG_PAGE_ACCESS_CHECK(&marker);
@@ -993,7 +1013,6 @@ PageWriteWrapper::~PageWriteWrapper()
 
 
 /*!	The page's cache must be locked.
-	The modified queue must be locked.
 */
 void
 PageWriteWrapper::SetTo(vm_page* page, bool dequeuedPage)
@@ -1071,12 +1090,10 @@ PageWriteWrapper::Done(status_t result)
 		// explicitly, which will take care of moving between the queues.
 		if (fDequeuedPage) {
 			fPage->state = PAGE_STATE_MODIFIED;
-			sModifiedPageQueue.Lock();
+			sModifiedPageQueue.AppendUnlocked(fPage);
-			sModifiedPageQueue.Append(fPage);
-			sModifiedPageQueue.Unlock();
 		} else {
 			fPage->state = fOldPageState;
-			set_page_state(fPage, PAGE_STATE_MODIFIED, false);
+			set_page_state(fPage, PAGE_STATE_MODIFIED);
 		}
 
 		if (!fPage->busy_writing) {
@@ -1089,7 +1106,7 @@ PageWriteWrapper::Done(status_t result)
 				atomic_add(&sModifiedTemporaryPages, -1);
 
 			// free the page
-			set_page_state(fPage, PAGE_STATE_FREE, false);
+			free_page(fPage, false);
 		} else {
 			fPage->busy_writing = false;
 			DEBUG_PAGE_ACCESS_END(fPage);
@@ -1246,7 +1263,6 @@ PageWriterRun::PrepareNextRun()
 
 
 /*!	The page's cache must be locked.
-	The modified queue must be locked.
 */
 void
 PageWriterRun::AddPage(vm_page* page)
@@ -1443,13 +1459,9 @@ page_writer(void* /*unused*/)
 				continue;
 			}
 
-			sModifiedPageQueue.Lock();
+			sModifiedPageQueue.RemoveUnlocked(page);
-
-			sModifiedPageQueue.Remove(page);
 			run.AddPage(page);
 
-			sModifiedPageQueue.Unlock();
-
 			//dprintf("write page %p, cache %p (%ld)\n", page, page->cache, page->cache->ref_count);
 			TPW(WritePage(page));
 
@@ -1493,7 +1505,7 @@ find_page_candidate(struct vm_page &marker)
 {
 	DEBUG_PAGE_ACCESS_CHECK(&marker);
 
-	AutoLocker<VMPageQueue> locker(sInactivePageQueue);
+	InterruptsSpinLocker locker(sInactivePageQueue.GetLock());
 	vm_page *page;
 
 	if (marker.state == PAGE_STATE_UNUSED) {
@@ -1547,12 +1559,12 @@ steal_page(vm_page *page)
 	vm_remove_all_page_mappings(page, &flags);
 	if ((flags & PAGE_MODIFIED) != 0) {
 		// page was modified, don't steal it
-		set_page_state(page, PAGE_STATE_MODIFIED, false);
+		set_page_state(page, PAGE_STATE_MODIFIED);
 		DEBUG_PAGE_ACCESS_END(page);
 		return false;
 	} else if ((flags & PAGE_ACCESSED) != 0) {
 		// page is in active use, don't steal it
-		set_page_state(page, PAGE_STATE_ACTIVE, false);
+		set_page_state(page, PAGE_STATE_ACTIVE);
 		DEBUG_PAGE_ACCESS_END(page);
 		return false;
 	}
@@ -1564,8 +1576,7 @@ steal_page(vm_page *page)
 
 	page->cache->RemovePage(page);
 
-	AutoLocker<VMPageQueue> _(sInactivePageQueue);
+	sInactivePageQueue.RemoveUnlocked(page);
-	sInactivePageQueue.Remove(page);
 	return true;
 }
 
@@ -1594,9 +1605,9 @@ steal_pages(vm_page **pages, size_t count)
 				break;
 
 			if (steal_page(page)) {
-				AutoLocker<VMPageQueue> locker(sFreePageQueue);
+				ReadLocker locker(sFreePageQueuesLock);
-				sFreePageQueue.Append(page);
 				page->state = PAGE_STATE_FREE;
+				sFreePageQueue.PrependUnlocked(page);
 				DEBUG_PAGE_ACCESS_END(page);
 				locker.Unlock();
 
@@ -1710,8 +1721,7 @@ vm_page_write_modified_page_range(struct VMCache* cache, uint32 firstPage,
 		if (page != NULL) {
 			if (page->state == PAGE_STATE_MODIFIED) {
 				DEBUG_PAGE_ACCESS_START(page);
-				AutoLocker<VMPageQueue> locker(sModifiedPageQueue);
+				sModifiedPageQueue.RemoveUnlocked(page);
-				sModifiedPageQueue.Remove(page);
 				dequeuedPage = true;
 			} else if (page->state == PAGE_STATE_BUSY
 					|| !vm_test_map_modification(page)) {
@@ -1858,11 +1868,11 @@ vm_page_init(kernel_args *args)
 	TRACE(("vm_page_init: entry\n"));
 
 	// init page queues
-	sModifiedPageQueue.Init("modified pages queue", 5);
+	sModifiedPageQueue.Init("modified pages queue");
-	sInactivePageQueue.Init("inactive pages queue", 4);
+	sInactivePageQueue.Init("inactive pages queue");
-	sActivePageQueue.Init("active pages queue", 3);
+	sActivePageQueue.Init("active pages queue");
-	sFreePageQueue.Init("free pages queue", 2);
+	sFreePageQueue.Init("free pages queue");
-	sClearPageQueue.Init("clear pages queue", 1);
+	sClearPageQueue.Init("clear pages queue");
 
 	// map in the new free page table
 	sPages = (vm_page *)vm_allocate_early(args, sNumPages * sizeof(vm_page),
@@ -1986,20 +1996,26 @@ vm_mark_page_range_inuse(addr_t startPage, addr_t length)
 		return B_BAD_VALUE;
 	}
 
-	VMPageQueuePairLocker locker(sFreePageQueue, sClearPageQueue);
+	WriteLocker locker(sFreePageQueuesLock);
 
 	for (addr_t i = 0; i < length; i++) {
 		vm_page *page = &sPages[startPage + i];
 		switch (page->state) {
 			case PAGE_STATE_FREE:
 			case PAGE_STATE_CLEAR:
+			{
 // TODO: This violates the page reservation policy, since we remove pages from
 // the free/clear queues without having reserved them before. This should happen
 // in the early boot process only, though.
 				DEBUG_PAGE_ACCESS_START(page);
-				set_page_state(page, PAGE_STATE_UNUSED, true);
+				VMPageQueue& queue = page->state == PAGE_STATE_FREE
+					? sFreePageQueue : sClearPageQueue;
+				queue.Remove(page);
+				page->state = PAGE_STATE_UNUSED;
+				atomic_add(&sUnreservedFreePages, -1);
 				DEBUG_PAGE_ACCESS_END(page);
 				break;
+			}
 			case PAGE_STATE_WIRED:
 				break;
 			case PAGE_STATE_ACTIVE:
@@ -2095,8 +2111,8 @@ vm_page_try_reserve_pages(uint32 count)
 vm_page *
 vm_page_allocate_page(int pageState)
 {
-	VMPageQueue *queue;
+	VMPageQueue* queue;
-	VMPageQueue *otherQueue;
+	VMPageQueue* otherQueue;
 
 	switch (pageState) {
 		case PAGE_STATE_FREE:
@@ -2115,21 +2131,33 @@ vm_page_allocate_page(int pageState)
 
 	T(AllocatePage());
 
-	VMPageQueuePairLocker freeClearQueuelocker(sFreePageQueue, sClearPageQueue);
+	ReadLocker locker(sFreePageQueuesLock);
 
-	vm_page *page = queue->RemoveHead();
+	vm_page* page = queue->RemoveHeadUnlocked();
 	if (page == NULL) {
-#if DEBUG_PAGE_QUEUE
-		if (queue->Count() != 0)
-			panic("queue %p corrupted, count = %ld\n", queue, queue->Count());
-#endif
-
 		// if the primary queue was empty, grab the page from the
 		// secondary queue
-		page = otherQueue->RemoveHead();
+		page = otherQueue->RemoveHeadUnlocked();
 
-		if (page == NULL)
+		if (page == NULL) {
-			panic("Had reserved page, but there is none!");
+			// Unlikely, but possible: the page we have reserved has moved
+			// between the queues after we checked the first queue. Grab the
+			// write locker to make sure this doesn't happen again.
+			locker.Unlock();
+			WriteLocker writeLocker(sFreePageQueuesLock);
+
+			page = queue->RemoveHead();
+			if (page == NULL)
+				otherQueue->RemoveHead();
+
+			if (page == NULL) {
+				panic("Had reserved page, but there is none!");
+				return NULL;
+			}
+
+			// downgrade to read lock
+			locker.Lock();
+		}
 	}
 
 	if (page->cache != NULL)
@@ -2141,11 +2169,9 @@ vm_page_allocate_page(int pageState)
 	page->state = PAGE_STATE_BUSY;
 	page->usage_count = 2;
 
-	freeClearQueuelocker.Unlock();
+	locker.Unlock();
 
-	sActivePageQueue.Lock();
+	sActivePageQueue.AppendUnlocked(page);
-	sActivePageQueue.Append(page);
-	sActivePageQueue.Unlock();
 
 	// clear the page, if we had to take it from the free queue and a clear
 	// page was requested
@@ -2158,7 +2184,7 @@ vm_page_allocate_page(int pageState)
 
 static vm_page*
 allocate_page_run(page_num_t start, page_num_t length, int pageState,
-	VMPageQueuePairLocker& freeClearQueuelocker)
+	WriteLocker& freeClearQueueLocker)
 {
 	T(AllocatePageRun(length));
 
@@ -2177,19 +2203,17 @@ allocate_page_run(page_num_t start, page_num_t length, int pageState,
 		page.state = PAGE_STATE_BUSY;
 	}
 
-	freeClearQueuelocker.Unlock();
+	freeClearQueueLocker.Unlock();
 
 	// add the pages to the active queue
-	AutoLocker<VMPageQueue> activeQueueLocker(sActivePageQueue);
-
 	for (page_num_t i = 0; i < length; i++) {
 		vm_page& page = sPages[start + i];
-		sActivePageQueue.Append(&page);
+		sActivePageQueue.AppendUnlocked(&page);
+			// TODO: We could do this more efficiently, if we used a temporary
+			// on-stack queue and added all pages at once.
 		page.usage_count = 1;
 	}
 
-	activeQueueLocker.Unlock();
-
 	// clear pages, if requested
 	if (pageState == PAGE_STATE_CLEAR) {
 		for (page_num_t i = 0; i < length; i++) {
@@ -2215,12 +2239,12 @@ vm_page_allocate_page_run(int pageState, addr_t base, addr_t length)
 		// TODO: add more tries, ie. free some inactive, ...
 		// no free space
 
-	VMPageQueuePairLocker freeClearQueuelocker(sFreePageQueue, sClearPageQueue);
+	WriteLocker freeClearQueueLocker(sFreePageQueuesLock);
 
 	for (;;) {
 		bool foundRun = true;
 		if (start + length > sNumPages) {
-			freeClearQueuelocker.Unlock();
+			freeClearQueueLocker.Unlock();
 			vm_page_unreserve_pages(length);
 			return NULL;
 		}
@@ -2237,7 +2261,7 @@ vm_page_allocate_page_run(int pageState, addr_t base, addr_t length)
 
 		if (foundRun)
 			return allocate_page_run(start, length, pageState,
-				freeClearQueuelocker);
+				freeClearQueueLocker);
 
 		start += i;
 	}
@@ -2247,8 +2271,8 @@ vm_page_allocate_page_run(int pageState, addr_t base, addr_t length)
 vm_page *
 vm_page_allocate_page_run_no_base(int pageState, addr_t count)
 {
-	VMPageQueue *queue;
+	VMPageQueue* queue;
-	VMPageQueue *otherQueue;
+	VMPageQueue* otherQueue;
 	switch (pageState) {
 		case PAGE_STATE_FREE:
 			queue = &sFreePageQueue;
@@ -2267,7 +2291,7 @@ vm_page_allocate_page_run_no_base(int pageState, addr_t count)
 		// TODO: add more tries, ie. free some inactive, ...
 		// no free space
 
-	VMPageQueuePairLocker freeClearQueuelocker(sFreePageQueue, sClearPageQueue);
+	WriteLocker freeClearQueueLocker(sFreePageQueuesLock);
 
 	for (;;) {
 		VMPageQueue::Iterator it = queue->GetIterator();
@@ -2287,12 +2311,12 @@ vm_page_allocate_page_run_no_base(int pageState, addr_t count)
 
 			if (foundRun) {
 				return allocate_page_run(page - sPages, count, pageState,
-					freeClearQueuelocker);
+					freeClearQueueLocker);
 			}
 		}
 
 		if (queue == otherQueue) {
-			freeClearQueuelocker.Unlock();
+			freeClearQueueLocker.Unlock();
 			vm_page_unreserve_pages(count);
 		}
 
@@ -2334,7 +2358,7 @@ vm_page_free(VMCache *cache, vm_page *page)
 	if (page->state == PAGE_STATE_MODIFIED && cache->temporary)
 		atomic_add(&sModifiedTemporaryPages, -1);
 
-	set_page_state(page, PAGE_STATE_FREE, false);
+	free_page(page, false);
 }
 
 
@@ -2343,7 +2367,10 @@ vm_page_set_state(vm_page *page, int pageState)
 {
 	ASSERT(page->state != PAGE_STATE_FREE && page->state != PAGE_STATE_CLEAR);
 
-	set_page_state(page, pageState, false);
+	if (pageState == PAGE_STATE_FREE || pageState == PAGE_STATE_CLEAR)
+		free_page(page, pageState == PAGE_STATE_CLEAR);
+	else
+		set_page_state(page, pageState);
 }
 
 
@@ -2384,16 +2411,7 @@ vm_page_requeue(struct vm_page *page, bool tail)
 			break;
 	}
 
-	queue->Lock();
+	queue->RequeueUnlocked(page, tail);
-
-	queue->Remove(page);
-
-	if (tail)
-		queue->Append(page);
-	else
-		queue->Prepend(page);
-
-	queue->Unlock();
 }