* Added new debug feature (DEBUG_PAGE_ACCESS) to detect invalid concurrent

access to a vm_page. It is basically an atomically accessed thread ID field
  in the vm_page structure, which is explicitly set by macros marking the
  critical sections. As a first positive effect I had to review quite a bit of
  code and found several issues.
* Added several TODOs and comments. Some harmless ones, but also a few
  troublesome ones in vm.cpp regarding page unmapping.
* file_cache: PrecacheIO::Prepare()/read_into_cache: Removed superfluous
  vm_page_allocate_page() return value checks. It cannot fail anymore.
* Removed the heavily contended "pages" lock. We use different policies now:
  - sModifiedTemporaryPages is accessed atomically.
  - sPageDeficitLock and sFreePageCondition are protected by a new mutex.
  - The page queues have individual locks (mutexes).
  - Renamed set_page_state_nolock() to set_page_state(). Unless the caller says
    otherwise, it does now lock the affected pages queues itself. Also changed
    the return value to void -- we panic() anyway.
* set_page_state(): Add free/clear pages to the beginning of their respective
  queues as this is more cache-friendly.
* Pages with the states PAGE_STATE_WIRED or PAGE_STATE_UNUSED are no longer
  in any queue. They were in the "active" queue, but there's no good reason
  to have them there. In case we decide to let the page daemon work the queues
  (like FreeBSD) they would just be in the way.
* Pulled the common part of vm_page_allocate_page_run[_no_base]() into a helper
  function. Also fixed a bug I introduced previously: The functions must not
  vm_page_unreserve_pages() on success, since they remove the pages from the
  free/clear queue without decrementing sUnreservedFreePages.
* vm_page_set_state(): Changed return type to void. The function cannot really
  fail and no-one was checking it anyway.
* vm_page_free(), vm_page_set_state(): Added assertion: The page must not be
  free/clear before. This is implied by the policy that no-one is allowed to
  access free/clear pages without holding the respective queue's lock, which is
  not the case at this point. This found the bug fixed in r34912.
* vm_page_requeue(): Added general assertions. panic() when requeuing of
  free/clear pages is requested. Same reason as above.
* vm_clone_area(), B_FULL_LOCK case: Don't map busy pages. The implementation is
  still not correct, though.

My usual -j8 Haiku build test runs another 10% faster, now. The total kernel
time drops about 18%. As hoped the new locks have only a fraction of the old
"pages" lock contention. Other locks lead the "most wanted list" now.



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

headers/private/kernel/vm/vm_page.h

@@ -28,7 +28,7 @@ status_t vm_page_init_post_thread(struct kernel_args *args);
 status_t vm_mark_page_inuse(addr_t page);
 status_t vm_mark_page_range_inuse(addr_t startPage, addr_t length);
 void vm_page_free(struct VMCache *cache, struct vm_page *page);
-status_t vm_page_set_state(struct vm_page *page, int state);
+void vm_page_set_state(struct vm_page *page, int state);
 void vm_page_requeue(struct vm_page *page, bool tail);
 
 // get some data about the number of pages in the system

headers/private/kernel/vm/vm_types.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009, Ingo Weinhold, [email protected].
+ * Copyright 2009-2010, Ingo Weinhold, [email protected].
  * Copyright 2002-2009, Axel Dörfler, [email protected].
  * Distributed under the terms of the MIT License.
  *
@@ -90,6 +90,10 @@ struct vm_page {
 	void*					queue;
 #endif
 
+#if DEBUG_PAGE_ACCESS
+	vint32					accessing_thread;
+#endif
+
 	uint8					type : 2;
 	uint8					state : 3;
 
@@ -121,4 +125,70 @@ enum {
 };
 
 
+#if DEBUG_PAGE_ACCESS
+#	include <thread.h>
+
+static inline void
+vm_page_debug_access_start(vm_page* page)
+{
+	thread_id threadID = thread_get_current_thread_id();
+	thread_id previousThread = atomic_test_and_set(&page->accessing_thread,
+		threadID, -1);
+	if (previousThread != -1) {
+		panic("Invalid concurrent access to page %p (start), currently "
+			"accessed by: %" B_PRId32, page, previousThread);
+	}
+}
+
+
+static inline void
+vm_page_debug_access_end(vm_page* page)
+{
+	thread_id threadID = thread_get_current_thread_id();
+	thread_id previousThread = atomic_test_and_set(&page->accessing_thread, -1,
+		threadID);
+	if (previousThread != threadID) {
+		panic("Invalid concurrent access to page %p (end) by current thread, "
+			"current accessor is: %" B_PRId32, page, previousThread);
+	}
+}
+
+
+static inline void
+vm_page_debug_access_check(vm_page* page)
+{
+	thread_id thread = page->accessing_thread;
+	if (thread != thread_get_current_thread_id()) {
+		panic("Invalid concurrent access to page %p (check), currently "
+			"accessed by: %" B_PRId32, page, thread);
+	}
+}
+
+
+static inline void
+vm_page_debug_access_transfer(vm_page* page, thread_id expectedPreviousThread)
+{
+	thread_id threadID = thread_get_current_thread_id();
+	thread_id previousThread = atomic_test_and_set(&page->accessing_thread,
+		threadID, expectedPreviousThread);
+	if (previousThread != expectedPreviousThread) {
+		panic("Invalid access transfer for page %p, currently accessed by: "
+			"%" B_PRId32 ", expected: %" B_PRId32, page, previousThread,
+			expectedPreviousThread);
+	}
+}
+
+#	define DEBUG_PAGE_ACCESS_START(page)	vm_page_debug_access_start(page)
+#	define DEBUG_PAGE_ACCESS_END(page)		vm_page_debug_access_end(page)
+#	define DEBUG_PAGE_ACCESS_CHECK(page)	vm_page_debug_access_check(page)
+#	define DEBUG_PAGE_ACCESS_TRANSFER(page, thread)	\
+		vm_page_debug_access_transfer(page, thread)
+#else
+#	define DEBUG_PAGE_ACCESS_START(page)			do {} while (false)
+#	define DEBUG_PAGE_ACCESS_END(page)				do {} while (false)
+#	define DEBUG_PAGE_ACCESS_CHECK(page)			do {} while (false)
+#	define DEBUG_PAGE_ACCESS_TRANSFER(page, thread)	do {} while (false)
+#endif
+
+
 #endif	// _KERNEL_VM_VM_TYPES_H