* Replaced the global cache pages hash table by an IteratableSplayTree

per cache.
* Changed the strategy vm_cache_acquire_page_cache_ref() uses to ensure
  that the cache isn't deleted while trying to get a reference. Instead
  of the global cache pages hash table lock, it holds the global cache
  list lock now. We acquire + release this lock in delete_cache() after
  removing all pages and just before deleting the object.
* Some small optimizations using the property that the cache's pages are
  ordered, now (vm_cache_resize(), vm_page_write_modified_page_range(),
  vm_page_schedule_write_page_range()).
* Replaced some code counting a cache's pages by simply using
  vm_cache::page_count.


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

headers/private/kernel/vm_types.h

@@ -30,6 +30,9 @@
 //#define DEBUG_CACHE_LIST 1
 
 #ifdef __cplusplus
+
+#include <util/SplayTree.h>
+
 struct vm_page_mapping;
 typedef DoublyLinkedListLink<vm_page_mapping> vm_page_mapping_link;
 
@@ -69,20 +72,20 @@ class DoublyLinkedAreaLink {
 typedef class DoublyLinkedQueue<vm_page_mapping, DoublyLinkedPageLink> vm_page_mappings;
 typedef class DoublyLinkedQueue<vm_page_mapping, DoublyLinkedAreaLink> vm_area_mappings;
 
+typedef uint32 page_num_t;
+
 struct vm_page {
 	struct vm_page		*queue_prev;
 	struct vm_page		*queue_next;
 
-	struct vm_page		*hash_next;
-
 	addr_t				physical_page_number;
 
 	struct vm_cache		*cache;
-	uint32				cache_offset;
+	page_num_t			cache_offset;
 							// in page size units
 
-	struct vm_page		*cache_prev;
-	struct vm_page		*cache_next;
+	SplayTreeLink<vm_page>	cache_link;
+	vm_page					*cache_next;
 
 	vm_page_mappings	mappings;
 
@@ -134,6 +137,34 @@ struct vm_dummy_page : vm_page {
 	ConditionVariable	busy_condition;
 };
 
+struct VMCachePagesTreeDefinition {
+	typedef page_num_t KeyType;
+	typedef	vm_page NodeType;
+
+	static page_num_t GetKey(const NodeType* node)
+	{
+		return node->cache_offset;
+	}
+
+	static SplayTreeLink<NodeType>* GetLink(NodeType* node)
+	{
+		return &node->cache_link;
+	}
+
+	static int Compare(page_num_t key, const NodeType* node)
+	{
+		return key == node->cache_offset ? 0
+			: (key < node->cache_offset ? -1 : 1);
+	}
+
+	static NodeType** GetListLink(NodeType* node)
+	{
+		return &node->cache_next;
+	}
+};
+
+typedef IteratableSplayTree<VMCachePagesTreeDefinition> VMCachePagesTree;
+
 struct vm_cache {
 	mutex				lock;
 	struct vm_area		*areas;
@@ -141,7 +172,7 @@ struct vm_cache {
 	struct list_link	consumer_link;
 	struct list			consumers;
 		// list of caches that use this cache as a source
-	vm_page				*page_list;
+	VMCachePagesTree	pages;
 	struct vm_cache		*source;
 	struct vm_store		*store;
 	off_t				virtual_base;

src/system/kernel/cache/file_cache.cpp

@@ -127,8 +127,8 @@ reserve_pages(file_cache_ref *ref, size_t reservePages, bool isWrite)
 
 			if (isWrite) {
 				// just schedule some pages to be written back
-				for (vm_page *page = cache->page_list; page != NULL;
-						page = page->cache_next) {
+				for (VMCachePagesTree::Iterator it = cache->pages.GetIterator();
+						vm_page* page = it.Next();) {
 					if (page->state == PAGE_STATE_MODIFIED) {
 						// TODO: for now, we only schedule one
 						vm_page_schedule_write_page(page);
@@ -137,13 +137,11 @@ reserve_pages(file_cache_ref *ref, size_t reservePages, bool isWrite)
 				}
 			} else {
 				// free some pages from our cache
-				// TODO: start with oldest (requires the page list to be a real list)!
+				// TODO: start with oldest
 				uint32 left = reservePages;
-				vm_page *next;
-				for (vm_page *page = cache->page_list;
-						page != NULL && left > 0; page = next) {
-					next = page->cache_next;
-
+				vm_page *page;
+				for (VMCachePagesTree::Iterator it = cache->pages.GetIterator();
+						(page = it.Next()) != NULL && left > 0;) {
 					if (page->state != PAGE_STATE_MODIFIED
 						&& page->state != PAGE_STATE_BUSY) {
 						vm_cache_remove_page(cache, page);

src/system/kernel/fs/vfs.cpp

@@ -2793,16 +2793,9 @@ dump_vnode_caches(int argc, char **argv)
 		if (device != -1 && vnode->device != device)
 			continue;
 
-		// count pages in cache
-		size_t numPages = 0;
-		for (struct vm_page *page = vnode->cache->page_list;
-				page != NULL; page = page->cache_next) {
-			numPages++;
-		}
-
 		kprintf("%p%4ld%10Ld %p %8Ld%8ld\n", vnode, vnode->device, vnode->id,
 			vnode->cache, (vnode->cache->virtual_size + B_PAGE_SIZE - 1)
-				/ B_PAGE_SIZE, numPages);
+				/ B_PAGE_SIZE, vnode->cache->page_count);
 	}
 
 	hash_close(sVnodeTable, &iterator, false);

src/system/kernel/vm/vm.cpp

@@ -2226,11 +2226,11 @@ vm_clone_area(team_id team, const char *name, void **address,
 			vm_page_reserve_pages(reservePages);
 
 			// map in all pages from source
-			for (vm_page *page = cache->page_list; page != NULL;
-					page = page->cache_next) {
+			for (VMCachePagesTree::Iterator it = cache->pages.GetIterator();
+					vm_page* page  = it.Next();) {
 				vm_map_page(newArea, page, newArea->base
-					+ ((page->cache_offset << PAGE_SHIFT) - newArea->cache_offset),
-					protection);
+					+ ((page->cache_offset << PAGE_SHIFT)
+					- newArea->cache_offset), protection);
 			}
 
 			vm_page_unreserve_pages(reservePages);
@@ -2532,16 +2532,8 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection,
 				// we can change the cache's commitment to take only those pages
 				// into account that really are in this cache.
 
-				// count existing pages in this cache
-				struct vm_page *page = cache->page_list;
-				uint32 count = 0;
-
-				for (; page != NULL; page = page->cache_next) {
-					count++;
-				}
-
 				status = cache->store->ops->commit(cache->store,
-					cache->virtual_base + count * B_PAGE_SIZE);
+					cache->virtual_base + cache->page_count * B_PAGE_SIZE);
 
 				// ToDo: we may be able to join with our source cache, if count == 0
 			}
@@ -2573,13 +2565,12 @@ vm_set_area_protection(team_id team, area_id areaID, uint32 newProtection,
 					= &area->address_space->translation_map;
 				map->ops->lock(map);
 
-				vm_page* page = cache->page_list;
-				while (page) {
+				for (VMCachePagesTree::Iterator it = cache->pages.GetIterator();
+						vm_page* page = it.Next();) {
 					addr_t address = area->base
 						+ (page->cache_offset << PAGE_SHIFT);
 					map->ops->protect(map, address, address - 1 + B_PAGE_SIZE,
 						newProtection);
-					page = page->cache_next;
 				}
 
 				map->ops->unlock(map);
@@ -3324,25 +3315,22 @@ dump_cache(int argc, char **argv)
 	}
 
 	kprintf("  pages:\n");
-	int32 count = 0;
-	for (vm_page *page = cache->page_list; page != NULL; page = page->cache_next) {
-		count++;
-		if (!showPages)
-			continue;
-
-		if (page->type == PAGE_TYPE_PHYSICAL) {
-			kprintf("\t%p ppn 0x%lx offset 0x%lx type %u state %u (%s) wired_count %u\n",
-				page, page->physical_page_number, page->cache_offset, page->type, page->state,
-				page_state_to_string(page->state), page->wired_count);
-		} else if(page->type == PAGE_TYPE_DUMMY) {
-			kprintf("\t%p DUMMY PAGE state %u (%s)\n",
-				page, page->state, page_state_to_string(page->state));
-		} else
-			kprintf("\t%p UNKNOWN PAGE type %u\n", page, page->type);
-	}
-
-	if (!showPages)
-		kprintf("\t%ld in cache\n", count);
+	if (showPages) {
+		for (VMCachePagesTree::Iterator it = cache->pages.GetIterator();
+				vm_page *page = it.Next();) {
+			if (page->type == PAGE_TYPE_PHYSICAL) {
+				kprintf("\t%p ppn 0x%lx offset 0x%lx type %u state %u (%s) "
+					"wired_count %u\n", page, page->physical_page_number,
+					page->cache_offset, page->type, page->state,
+					page_state_to_string(page->state), page->wired_count);
+			} else if(page->type == PAGE_TYPE_DUMMY) {
+				kprintf("\t%p DUMMY PAGE state %u (%s)\n",
+					page, page->state, page_state_to_string(page->state));
+			} else
+				kprintf("\t%p UNKNOWN PAGE type %u\n", page, page->type);
+		}
+	} else
+		kprintf("\t%ld in cache\n", cache->page_count);
 
 	return 0;
 }

src/system/kernel/vm/vm_cache.cpp

@@ -36,13 +36,11 @@
 #endif
 
 
-static hash_table* sPageCacheTable;
-static spinlock sPageCacheTableLock;
-
 #if DEBUG_CACHE_LIST
 vm_cache* gDebugCacheList;
-static spinlock sDebugCacheListLock;
 #endif
+static mutex sCacheListLock = MUTEX_INITIALIZER("global vm_cache list");
+	// The lock is also needed when the debug feature is disabled.
 
 
 struct page_lookup_key {
@@ -352,8 +350,7 @@ page_hash_func(void* _p, const void* _key, uint32 range)
 
 /*!	Acquires a reference to a cache yet unreferenced by the caller. The
 	caller must make sure, that the cache is not deleted, e.g. by holding the
-	cache's source cache lock or by holding the page cache table lock while the
-	cache is still referred to by a page.
+	cache's source cache lock or by holding the global cache list lock.
 	Returns \c true, if the reference could be acquired.
 */
 static inline bool
@@ -380,56 +377,45 @@ delete_cache(vm_cache* cache)
 
 	T(Delete(cache));
 
-#if DEBUG_CACHE_LIST
-	int state = disable_interrupts();
-	acquire_spinlock(&sDebugCacheListLock);
+	// delete the cache's backing store
+	cache->store->ops->destroy(cache->store);
 
+	// free all of the pages in the cache
+	while (vm_page* page = cache->pages.Root()) {
+		if (!page->mappings.IsEmpty() || page->wired_count != 0) {
+			panic("remove page %p from cache %p: page still has mappings!\n",
+				page, cache);
+		}
+
+		// remove it
+		cache->pages.Remove(page);
+		page->cache = NULL;
+		// TODO: we also need to remove all of the page's mappings!
+
+		TRACE(("vm_cache_release_ref: freeing page 0x%lx\n",
+			oldPage->physical_page_number));
+		vm_page_free(cache, page);
+	}
+
+	// remove the ref to the source
+	if (cache->source)
+		vm_cache_remove_consumer(cache->source, cache);
+
+	// We lock and unlock the sCacheListLock, even if the DEBUG_CACHE_LIST is
+	// not enabled. This synchronization point is needed for
+	// vm_cache_acquire_page_cache_ref().
+	mutex_lock(&sCacheListLock);
+
+#if DEBUG_CACHE_LIST
 	if (cache->debug_previous)
 		cache->debug_previous->debug_next = cache->debug_next;
 	if (cache->debug_next)
 		cache->debug_next->debug_previous = cache->debug_previous;
 	if (cache == gDebugCacheList)
 		gDebugCacheList = cache->debug_next;
-
-	release_spinlock(&sDebugCacheListLock);
-	restore_interrupts(state);
 #endif
 
-	// delete the cache's backing store
-	cache->store->ops->destroy(cache->store);
-
-	// free all of the pages in the cache
-	vm_page* page = cache->page_list;
-	while (page) {
-		vm_page* oldPage = page;
-		int state;
-
-		page = page->cache_next;
-
-		if (!oldPage->mappings.IsEmpty() || oldPage->wired_count != 0) {
-			panic("remove page %p from cache %p: page still has mappings!\n",
-				oldPage, cache);
-		}
-
-		// remove it from the hash table
-		state = disable_interrupts();
-		acquire_spinlock(&sPageCacheTableLock);
-
-		hash_remove(sPageCacheTable, oldPage);
-		oldPage->cache = NULL;
-		// TODO: we also need to remove all of the page's mappings!
-
-		release_spinlock(&sPageCacheTableLock);
-		restore_interrupts(state);
-
-		TRACE(("vm_cache_release_ref: freeing page 0x%lx\n",
-			oldPage->physical_page_number));
-		vm_page_free(cache, oldPage);
-	}
-
-	// remove the ref to the source
-	if (cache->source)
-		vm_cache_remove_consumer(cache->source, cache);
+	mutex_unlock(&sCacheListLock);
 
 	mutex_destroy(&cache->lock);
 	free(cache);
@@ -505,13 +491,11 @@ merge_cache_with_only_consumer(vm_cache* cache)
 
 	T(Merge(cache, consumer));
 
-	vm_page* nextPage;
-	for (vm_page* page = cache->page_list; page != NULL;
-			page = nextPage) {
-		vm_page* consumerPage;
-		nextPage = page->cache_next;
-
-		consumerPage = vm_cache_lookup_page(consumer,
+	for (VMCachePagesTree::Iterator it = cache->pages.GetIterator();
+			vm_page* page = it.Next();) {
+		// Note: Removing the current node while iterating through a
+		// IteratableSplayTree is safe.
+		vm_page* consumerPage = vm_cache_lookup_page(consumer,
 			(off_t)page->cache_offset << PAGE_SHIFT);
 		if (consumerPage == NULL) {
 			// the page is not yet in the consumer cache - move it upwards
@@ -583,12 +567,6 @@ panic("cacheRef %p ref count too low!\n", cache);
 status_t
 vm_cache_init(kernel_args* args)
 {
-	// TODO: The table should grow/shrink dynamically.
-	sPageCacheTable = hash_init(vm_page_num_pages() / 2,
-		offsetof(vm_page, hash_next), &page_compare_func, &page_hash_func);
-	if (sPageCacheTable == NULL)
-		panic("vm_cache_init: no memory\n");
-
 	return B_OK;
 }
 
@@ -609,7 +587,7 @@ vm_cache_create(vm_store* store)
 
 	mutex_init(&cache->lock, "vm_cache");
 	list_init_etc(&cache->consumers, offsetof(vm_cache, consumer_link));
-	cache->page_list = NULL;
+	new(&cache->pages) VMCachePagesTree;
 	cache->areas = NULL;
 	cache->ref_count = 1;
 	cache->source = NULL;
@@ -622,8 +600,7 @@ vm_cache_create(vm_store* store)
 
 
 #if DEBUG_CACHE_LIST
-	int state = disable_interrupts();
-	acquire_spinlock(&sDebugCacheListLock);
+	mutex_lock(&sCacheListLock);
 
 	if (gDebugCacheList)
 		gDebugCacheList->debug_previous = cache;
@@ -631,8 +608,7 @@ vm_cache_create(vm_store* store)
 	cache->debug_next = gDebugCacheList;
 	gDebugCacheList = cache;
 
-	release_spinlock(&sDebugCacheListLock);
-	restore_interrupts(state);
+	mutex_unlock(&sCacheListLock);
 #endif
 
 	// connect the store to its cache
@@ -708,9 +684,10 @@ vm_cache_release_ref(vm_cache* cache)
 vm_cache*
 vm_cache_acquire_page_cache_ref(vm_page* page)
 {
-	InterruptsSpinLocker locker(sPageCacheTableLock);
+	MutexLocker locker(sCacheListLock);
 
 	vm_cache* cache = page->cache;
+		// Getting/setting the pointer must be atomic on all architectures.
 	if (cache == NULL)
 		return NULL;
 
@@ -727,17 +704,7 @@ vm_cache_lookup_page(vm_cache* cache, off_t offset)
 {
 	ASSERT_LOCKED_MUTEX(&cache->lock);
 
-	struct page_lookup_key key;
-	key.offset = (uint32)(offset >> PAGE_SHIFT);
-	key.cache = cache;
-
-	cpu_status state = disable_interrupts();
-	acquire_spinlock(&sPageCacheTableLock);
-
-	vm_page* page = (vm_page*)hash_lookup(sPageCacheTable, &key);
-
-	release_spinlock(&sPageCacheTableLock);
-	restore_interrupts(state);
+	vm_page* page = cache->pages.Lookup((page_num_t)(offset >> PAGE_SHIFT));
 
 	if (page != NULL && cache != page->cache)
 		panic("page %p not in cache %p\n", page, cache);
@@ -760,27 +727,13 @@ vm_cache_insert_page(vm_cache* cache, vm_page* page, off_t offset)
 
 	T2(InsertPage(cache, page, offset));
 
-	page->cache_offset = (uint32)(offset >> PAGE_SHIFT);
-
-	if (cache->page_list != NULL)
-		cache->page_list->cache_prev = page;
-
-	page->cache_next = cache->page_list;
-	page->cache_prev = NULL;
-	cache->page_list = page;
+	page->cache_offset = (page_num_t)(offset >> PAGE_SHIFT);
 	cache->page_count++;
-
 	page->usage_count = 2;
-
-	InterruptsSpinLocker locker(sPageCacheTableLock);
-
 	page->cache = cache;
 
 #if KDEBUG
-	struct page_lookup_key key;
-	key.offset = (uint32)(offset >> PAGE_SHIFT);
-	key.cache = cache;
-	vm_page* otherPage = (vm_page*)hash_lookup(sPageCacheTable, &key);
+	vm_page* otherPage = cache->pages.Lookup(page->cache_offset);
 	if (otherPage != NULL) {
 		panic("vm_cache_insert_page(): there's already page %p with cache "
 			"offset %lu in cache %p; inserting page %p", otherPage,
@@ -788,7 +741,7 @@ vm_cache_insert_page(vm_cache* cache, vm_page* page, off_t offset)
 	}
 #endif	// KDEBUG
 
-	hash_insert(sPageCacheTable, page);
+	cache->pages.Insert(page);
 }
 
 
@@ -809,25 +762,8 @@ vm_cache_remove_page(vm_cache* cache, vm_page* page)
 
 	T2(RemovePage(cache, page));
 
-	cpu_status state = disable_interrupts();
-	acquire_spinlock(&sPageCacheTableLock);
-
-	hash_remove(sPageCacheTable, page);
+	cache->pages.Remove(page);
 	page->cache = NULL;
-
-	release_spinlock(&sPageCacheTableLock);
-	restore_interrupts(state);
-
-	if (cache->page_list == page) {
-		if (page->cache_next != NULL)
-			page->cache_next->cache_prev = NULL;
-		cache->page_list = page->cache_next;
-	} else {
-		if (page->cache_prev != NULL)
-			page->cache_prev->cache_next = page->cache_next;
-		if (page->cache_next != NULL)
-			page->cache_next->cache_prev = page->cache_prev;
-	}
 	cache->page_count--;
 }
 
@@ -907,40 +843,34 @@ vm_cache_resize(vm_cache* cache, off_t newSize)
 	if (newPageCount < oldPageCount) {
 		// we need to remove all pages in the cache outside of the new virtual
 		// size
-		vm_page* page = cache->page_list;
-		vm_page* next;
+		for (VMCachePagesTree::Iterator it
+					= cache->pages.GetIterator(newPageCount, true, true);
+				vm_page* page = it.Next();) {
+			if (page->state == PAGE_STATE_BUSY) {
+				if (page->busy_writing) {
+					// We cannot wait for the page to become available
+					// as we might cause a deadlock this way
+					page->busy_writing = false;
+						// this will notify the writer to free the page
+				} else {
+					// wait for page to become unbusy
+					ConditionVariableEntry entry;
+					entry.Add(page);
+					mutex_unlock(&cache->lock);
+					entry.Wait();
+					mutex_lock(&cache->lock);
 
-		while (page != NULL) {
-			next = page->cache_next;
-
-			if (page->cache_offset >= newPageCount) {
-				if (page->state == PAGE_STATE_BUSY) {
-					if (page->busy_writing) {
-						// We cannot wait for the page to become available
-						// as we might cause a deadlock this way
-						page->busy_writing = false;
-							// this will notify the writer to free the page
-						page = next;
-					} else {
-						// wait for page to become unbusy
-						ConditionVariableEntry entry;
-						entry.Add(page);
-						mutex_unlock(&cache->lock);
-						entry.Wait();
-						mutex_lock(&cache->lock);
-
-						// restart from the start of the list
-						page = cache->page_list;
-					}
-					continue;
+					// restart from the start of the list
+					it = cache->pages.GetIterator(newPageCount, true, true);
 				}
-
-				// remove the page and put it into the free queue
-				vm_cache_remove_page(cache, page);
-				vm_page_free(cache, page);
+				continue;
 			}
 
-			page = next;
+			// remove the page and put it into the free queue
+			vm_cache_remove_page(cache, page);
+			vm_page_free(cache, page);
+				// Note: When iterating through a IteratableSplayTree
+				// removing the current node is safe.
 		}
 	}
 

src/system/kernel/vm/vm_page.cpp

@@ -497,11 +497,10 @@ dump_page(int argc, char **argv)
 
 	kprintf("PAGE: %p\n", page);
 	kprintf("queue_next,prev: %p, %p\n", page->queue_next, page->queue_prev);
-	kprintf("hash_next:       %p\n", page->hash_next);
 	kprintf("physical_number: %lx\n", page->physical_page_number);
 	kprintf("cache:           %p\n", page->cache);
 	kprintf("cache_offset:    %ld\n", page->cache_offset);
-	kprintf("cache_next,prev: %p, %p\n", page->cache_next, page->cache_prev);
+	kprintf("cache_next:      %p\n", page->cache_next);
 	kprintf("type:            %d\n", page->type);
 	kprintf("state:           %s\n", page_state_to_string(page->state));
 	kprintf("wired_count:     %d\n", page->wired_count);
@@ -1324,11 +1323,13 @@ vm_page_write_modified_page_range(struct vm_cache *cache, uint32 firstPage,
 {
 	// TODO: join adjacent pages into one vec list
 
-	for (vm_page *page = cache->page_list; page; page = page->cache_next) {
+	for (VMCachePagesTree::Iterator it
+				= cache->pages.GetIterator(firstPage, true, true);
+			vm_page *page = it.Next();) {
 		bool dequeuedPage = false;
 
-		if (page->cache_offset < firstPage || page->cache_offset >= endPage)
-			continue;
+		if (page->cache_offset >= endPage)
+			break;
 
 		if (page->state == PAGE_STATE_MODIFIED) {
 			InterruptsSpinLocker locker(&sPageLock);
@@ -1428,11 +1429,15 @@ vm_page_schedule_write_page_range(struct vm_cache *cache, uint32 firstPage,
 	uint32 endPage)
 {
 	uint32 modified = 0;
-	for (vm_page *page = cache->page_list; page; page = page->cache_next) {
+	for (VMCachePagesTree::Iterator it
+				= cache->pages.GetIterator(firstPage, true, true);
+			vm_page *page = it.Next();) {
 		bool dequeuedPage = false;
 
-		if (page->cache_offset >= firstPage && page->cache_offset < endPage
-			&& page->state == PAGE_STATE_MODIFIED) {
+		if (page->cache_offset >= endPage)
+			break;
+
+		if (page->state == PAGE_STATE_MODIFIED) {
 			vm_page_requeue(page, false);
 			modified++;
 		}