From 6d1c68155a33b0cbe1df50986fa8345dcaaa6a32 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Thu, 19 Jul 2007 22:52:23 +0000 Subject: [PATCH] axeld + bonefish: * More conditional debug code (wrt page transitions between caches). * Replaced debugger command cache_chain by a nicer cache_tree. * While handling a soft fault: When we temporarily unlock a cache, it can theoretically become busy. One such occurrence is now handled properly, two more panic() ATM, though should be fixed. * When merging caches, we do now always replace a dummy page in the upper cache, not only when the concurrent page fault is a read fault. This prevents a page from the lower (to be discarded) cache from still remaining mapped (causing a panic). * When merging caches and replacing a dummy page, we were trying to remove the dummy page from the wrong cache (causing a panic). The Haiku kernel seems now to run shockingly stable. ATM, we have more than two hours uptime of a system booted and running over network. We didn't manage to get it down by fully building Pe, downloading, unzipping, and playing with various stuff. Someone should finally fix all those app server drawing bugs, though (hint, hint! ;-)). git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21672 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/private/kernel/vm_types.h | 8 +++ src/system/kernel/vm/vm.cpp | 81 ++++++++++++++++++++++++++++--- src/system/kernel/vm/vm_cache.cpp | 25 ++++++++-- src/system/kernel/vm/vm_page.c | 10 +++- 4 files changed, 110 insertions(+), 14 deletions(-) diff --git a/headers/private/kernel/vm_types.h b/headers/private/kernel/vm_types.h index 82b9a68e45..f1185b254a 100644 --- a/headers/private/kernel/vm_types.h +++ b/headers/private/kernel/vm_types.h @@ -21,6 +21,9 @@ // be in. //#define DEBUG_PAGE_QUEUE 1 +// Enables extra debug fields in the vm_page used to track page transitions +// between caches. +//#define DEBUG_PAGE_CACHE_TRANSITIONS 1 #ifdef __cplusplus struct vm_page_mapping; @@ -92,6 +95,11 @@ typedef struct vm_page { void* queue; #endif + #ifdef DEBUG_PAGE_CACHE_TRANSITIONS + uint32 debug_flags; + struct vm_page *collided_page; + #endif + uint8 type : 2; uint8 state : 3; uint8 busy_reading : 1; diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index b0ad5fcb3c..9c8cef4a6b 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -2195,8 +2195,29 @@ page_state_to_string(int state) } +static void +dump_cache_tree_recursively(vm_cache* cache, int level, + vm_cache* highlightCache) +{ + // print this cache + for (int i = 0; i < level; i++) + kprintf(" "); + if (cache == highlightCache) + kprintf("%p <--\n", cache); + else + kprintf("%p\n", cache); + + // recursively print its consumers + vm_cache* consumer = NULL; + while ((consumer = (vm_cache *)list_get_next_item(&cache->consumers, + consumer)) != NULL) { + dump_cache_tree_recursively(consumer, level + 1, highlightCache); + } +} + + static int -dump_cache_chain(int argc, char **argv) +dump_cache_tree(int argc, char **argv) { if (argc < 2 || strlen(argv[1]) < 2 || argv[1][0] != '0' @@ -2210,10 +2231,13 @@ dump_cache_chain(int argc, char **argv) return 0; vm_cache *cache = (vm_cache *)address; - while (cache != NULL) { - dprintf("%p\n", cache); - cache = cache->source; - } + vm_cache *root = cache; + + // find the root cache (the transitive source) + while (root->source != NULL) + root = root->source; + + dump_cache_tree_recursively(root, 0, cache); return 0; } @@ -2914,7 +2938,7 @@ vm_init(kernel_args *args) add_debugger_command("areas", &dump_area_list, "Dump a list of all areas"); add_debugger_command("area", &dump_area, "Dump info about a particular area"); add_debugger_command("cache", &dump_cache, "Dump vm_cache"); - add_debugger_command("cache_chain", &dump_cache_chain, "Dump vm_cache chain"); + add_debugger_command("cache_tree", &dump_cache_tree, "Dump vm_cache tree"); add_debugger_command("avail", &dump_available_memory, "Dump available memory"); add_debugger_command("dl", &display_mem, "dump memory long words (64-bit)"); add_debugger_command("dw", &display_mem, "dump memory words (32-bit)"); @@ -3197,8 +3221,22 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, // page must be busy // ToDo: don't wait forever! mutex_unlock(&cache->lock); - snooze(20000); + thread_yield(); mutex_lock(&cache->lock); + + if (cache->busy) { + // The cache became busy, which means, it is about to be + // removed by vm_cache_remove_consumer(). We start again with + // the top cache. + mutex_unlock(&cache->lock); + vm_cache_release_ref(cache); + + cache = topCache; + lastCache = cache; + + vm_cache_acquire_ref(cache); + mutex_lock(&cache->lock); + } } if (page != NULL && page != &dummyPage) @@ -3267,6 +3305,9 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, // upwards, too, we try this cache again mutex_unlock(&cache->lock); mutex_lock(&cache->lock); + if (cache->busy) + panic("fault_find_page(): cache became busy: %p\n", cache); + // TODO: Don't panic()! lastCache = NULL; continue; } else if (status < B_OK) @@ -3288,6 +3329,9 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, // top most cache may have direct write access. vm_cache_acquire_ref(cache); mutex_lock(&cache->lock); + if (cache->busy) + panic("fault_find_page(): 2. cache became busy: %p\n", cache); + // TOOD: Don't panic()! } // release the reference of the last vm_cache we still have from the loop above @@ -3324,12 +3368,29 @@ fault_get_page(vm_translation_map *map, vm_cache *topCache, // Insert the new page into our cache, and replace it with the dummy page if necessary // if we inserted a dummy page into this cache, we have to remove it now - if (dummyPage.state == PAGE_STATE_BUSY && dummyPage.cache == cache) + if (dummyPage.state == PAGE_STATE_BUSY && dummyPage.cache == cache) { +#ifdef DEBUG_PAGE_CACHE_TRANSITIONS + page->debug_flags = dummyPage.debug_flags | 0x8; + if (dummyPage.collided_page != NULL) { + dummyPage.collided_page->collided_page = page; + page->collided_page = dummyPage.collided_page; + } +#endif // DEBUG_PAGE_CACHE_TRANSITIONS + fault_remove_dummy_page(dummyPage, true); + } vm_cache_insert_page(cache, page, cacheOffset); if (dummyPage.state == PAGE_STATE_BUSY) { +#ifdef DEBUG_PAGE_CACHE_TRANSITIONS + page->debug_flags = dummyPage.debug_flags | 0x10; + if (dummyPage.collided_page != NULL) { + dummyPage.collided_page->collided_page = page; + page->collided_page = dummyPage.collided_page; + } +#endif // DEBUG_PAGE_CACHE_TRANSITIONS + // we had inserted the dummy cache in another cache, so let's remove it from there fault_remove_dummy_page(dummyPage, false); } @@ -3502,6 +3563,10 @@ vm_soft_fault(addr_t originalAddress, bool isWrite, bool isUser) dummyPage.type = PAGE_TYPE_DUMMY; dummyPage.busy_writing = isWrite; dummyPage.wired_count = 0; +#ifdef DEBUG_PAGE_CACHE_TRANSITIONS + dummyPage.debug_flags = 0; + dummyPage.collided_page = NULL; +#endif // DEBUG_PAGE_CACHE_TRANSITIONS vm_cache *copiedPageSource = NULL; vm_cache *pageSource; diff --git a/src/system/kernel/vm/vm_cache.cpp b/src/system/kernel/vm/vm_cache.cpp index 43ebad898e..1847444808 100644 --- a/src/system/kernel/vm/vm_cache.cpp +++ b/src/system/kernel/vm/vm_cache.cpp @@ -314,8 +314,10 @@ vm_cache_remove_page(vm_cache *cache, vm_page *page) TRACE(("vm_cache_remove_page: cache %p, page %p\n", cache, page)); ASSERT_LOCKED_MUTEX(&cache->lock); - if (page->cache != cache) - panic("remove page from %p: page cache is set to %p\n", cache, page->cache); + if (page->cache != cache) { + panic("remove page %p from cache %p: page cache is set to %p\n", page, + cache, page->cache); + } state = disable_interrupts(); acquire_spinlock(&sPageCacheTableLock); @@ -478,6 +480,8 @@ vm_cache_remove_consumer(vm_cache *cache, vm_cache *consumer) || list_is_empty(&cache->consumers) || cache->consumers.link.next != cache->consumers.link.prev || consumer != list_get_first_item(&cache->consumers)) { + dprintf("vm_cache_remove_consumer(): cache %p was modified; " + "not merging it\n"); merge = false; cache->busy = false; mutex_unlock(&consumer->lock); @@ -512,18 +516,29 @@ if (consumer->virtual_base == 0x11000) vm_cache_insert_page(consumer, page, (off_t)page->cache_offset << PAGE_SHIFT); } else if (consumerPage->state == PAGE_STATE_BUSY - && consumerPage->type == PAGE_TYPE_DUMMY - && !consumerPage->busy_writing) { + && consumerPage->type == PAGE_TYPE_DUMMY) { // the page is currently busy taking a read fault - IOW, // vm_soft_fault() has mapped our page so we can just // move it up //dprintf("%ld: merged busy page %p, cache %p, offset %ld\n", find_thread(NULL), page, cacheRef->cache, page->cache_offset); - vm_cache_remove_page(cache, consumerPage); + vm_cache_remove_page(consumer, consumerPage); consumerPage->state = PAGE_STATE_INACTIVE; vm_cache_remove_page(cache, page); vm_cache_insert_page(consumer, page, (off_t)page->cache_offset << PAGE_SHIFT); +#ifdef DEBUG_PAGE_CACHE_TRANSITIONS + } else { + page->debug_flags = 0; + if (consumerPage->state == PAGE_STATE_BUSY) + page->debug_flags |= 0x1; + if (consumerPage->type == PAGE_TYPE_DUMMY) + page->debug_flags |= 0x2; + if (!consumerPage->busy_writing) + page->debug_flags |= 0x4; + page->collided_page = consumerPage; + consumerPage->collided_page = page; +#endif // DEBUG_PAGE_CACHE_TRANSITIONS } #if 0 else if (consumer->virtual_base == 0x11000) diff --git a/src/system/kernel/vm/vm_page.c b/src/system/kernel/vm/vm_page.c index af33579937..9b92062328 100644 --- a/src/system/kernel/vm/vm_page.c +++ b/src/system/kernel/vm/vm_page.c @@ -269,8 +269,12 @@ dump_page(int argc, char **argv) kprintf("wired_count: %u\n", page->wired_count); kprintf("usage_count: %u\n", page->usage_count); #ifdef DEBUG_PAGE_QUEUE - kprintf("queue: %p\n", page->queue); + kprintf("queue: %p\n", page->queue); #endif + #ifdef DEBUG_PAGE_CACHE_TRANSITIONS + kprintf("debug_flags: 0x%lx\n", page->debug_flags); + kprintf("collided page: %p\n", page->collided_page); + #endif // DEBUG_PAGE_CACHE_TRANSITIONS kprintf("area mappings:\n"); mapping = page->mappings; @@ -831,6 +835,10 @@ vm_page_init(kernel_args *args) #ifdef DEBUG_PAGE_QUEUE sPages[i].queue = NULL; #endif + #ifdef DEBUG_PAGE_CACHE_TRANSITIONS + sPages[i].debug_flags = 0; + sPages[i].collided_page = NULL; + #endif // DEBUG_PAGE_CACHE_TRANSITIONS enqueue_page(&page_free_queue, &sPages[i]); }