From e1b630c55d9d0c25aef812efc9c2410f5ba53456 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sat, 28 Jun 2008 23:42:41 +0000 Subject: [PATCH] * 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 | 43 ++++- src/system/kernel/cache/file_cache.cpp | 14 +- src/system/kernel/fs/vfs.cpp | 9 +- src/system/kernel/vm/vm.cpp | 58 +++---- src/system/kernel/vm/vm_cache.cpp | 214 +++++++++---------------- src/system/kernel/vm/vm_page.cpp | 21 ++- 6 files changed, 152 insertions(+), 207 deletions(-) diff --git a/headers/private/kernel/vm_types.h b/headers/private/kernel/vm_types.h index b9305391a0..7a3ed561ba 100644 --- a/headers/private/kernel/vm_types.h +++ b/headers/private/kernel/vm_types.h @@ -30,6 +30,9 @@ //#define DEBUG_CACHE_LIST 1 #ifdef __cplusplus + +#include + struct vm_page_mapping; typedef DoublyLinkedListLink vm_page_mapping_link; @@ -69,20 +72,20 @@ class DoublyLinkedAreaLink { typedef class DoublyLinkedQueue vm_page_mappings; typedef class DoublyLinkedQueue 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 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* 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 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; diff --git a/src/system/kernel/cache/file_cache.cpp b/src/system/kernel/cache/file_cache.cpp index 9d52ed1ba5..df7b5e9f34 100644 --- a/src/system/kernel/cache/file_cache.cpp +++ b/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); diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp index e1e1422704..3949245bd3 100644 --- a/src/system/kernel/fs/vfs.cpp +++ b/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); diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index d1641e9f95..81f939117e 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/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; } diff --git a/src/system/kernel/vm/vm_cache.cpp b/src/system/kernel/vm/vm_cache.cpp index e2205c8d69..d9f328532d 100644 --- a/src/system/kernel/vm/vm_cache.cpp +++ b/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. } } diff --git a/src/system/kernel/vm/vm_page.cpp b/src/system/kernel/vm/vm_page.cpp index 5e06c8fed0..60487e52a7 100644 --- a/src/system/kernel/vm/vm_page.cpp +++ b/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++; }