diff --git a/build/config_headers/kernel_debug_config.h b/build/config_headers/kernel_debug_config.h index 186fefb7e2..5b767929fb 100644 --- a/build/config_headers/kernel_debug_config.h +++ b/build/config_headers/kernel_debug_config.h @@ -80,10 +80,14 @@ // VM -// Enables the vm_page::queue, i.e. it is tracked which queue the page should -// be in. +// Enables the vm_page::queue field, i.e. it is tracked which queue the page +// should be in. #define DEBUG_PAGE_QUEUE 0 +// Enables the vm_page::access_count field, which is used to detect invalid +// concurrent access to the page. +#define DEBUG_PAGE_ACCESS 1 + // Enables a global list of all vm_cache structures. #define DEBUG_CACHE_LIST KDEBUG_LEVEL_1 diff --git a/headers/private/kernel/vm/vm_page.h b/headers/private/kernel/vm/vm_page.h index 54e57fc111..8e19bf1c87 100644 --- a/headers/private/kernel/vm/vm_page.h +++ b/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 diff --git a/headers/private/kernel/vm/vm_types.h b/headers/private/kernel/vm/vm_types.h index c435f7cb39..0e453c9d83 100644 --- a/headers/private/kernel/vm/vm_types.h +++ b/headers/private/kernel/vm/vm_types.h @@ -1,5 +1,5 @@ /* - * Copyright 2009, Ingo Weinhold, ingo_weinhold@gmx.de. + * Copyright 2009-2010, Ingo Weinhold, ingo_weinhold@gmx.de. * Copyright 2002-2009, Axel Dörfler, axeld@pinc-software.de. * 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 + +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 diff --git a/src/add-ons/kernel/bus_managers/agp_gart/agp_gart.cpp b/src/add-ons/kernel/bus_managers/agp_gart/agp_gart.cpp index 10e5eebe0c..eda0564504 100644 --- a/src/add-ons/kernel/bus_managers/agp_gart/agp_gart.cpp +++ b/src/add-ons/kernel/bus_managers/agp_gart/agp_gart.cpp @@ -83,6 +83,9 @@ struct aperture_memory { vm_page **pages; vm_page *page; }; +#ifdef DEBUG_PAGE_ACCESS + thread_id allocating_thread; +#endif #else area_id area; #endif @@ -550,6 +553,11 @@ Aperture::AllocateMemory(aperture_memory *memory, uint32 flags) memory->pages[i] = vm_page_allocate_page(PAGE_STATE_CLEAR); vm_page_unreserve_pages(count); } + +#ifdef DEBUG_PAGE_ACCESS + memory->allocating_thread = find_thread(NULL); +#endif + #else void *address; memory->area = create_area("GART memory", &address, B_ANY_KERNEL_ADDRESS, @@ -682,12 +690,15 @@ Aperture::_Free(aperture_memory *memory) if ((memory->flags & B_APERTURE_NEED_PHYSICAL) != 0) { vm_page *page = memory->page; for (uint32 i = 0; i < count; i++, page++) { + DEBUG_PAGE_ACCESS_TRANSFER(page, memory->allocating_thread); vm_page_set_state(page, PAGE_STATE_FREE); } memory->page = NULL; } else { for (uint32 i = 0; i < count; i++) { + DEBUG_PAGE_ACCESS_TRANSFER(memory->pages[i], + memory->allocating_thread); vm_page_set_state(memory->pages[i], PAGE_STATE_FREE); } diff --git a/src/system/kernel/arch/m68k/arch_vm_translation_map_impl.cpp b/src/system/kernel/arch/m68k/arch_vm_translation_map_impl.cpp index 0960da7369..a465713d31 100644 --- a/src/system/kernel/arch/m68k/arch_vm_translation_map_impl.cpp +++ b/src/system/kernel/arch/m68k/arch_vm_translation_map_impl.cpp @@ -376,10 +376,13 @@ destroy_tmap(vm_translation_map *map) panic("destroy_tmap: didn't find pgtable page\n"); return; } + DEBUG_PAGE_ACCESS_START(page); vm_page_set_state(page, PAGE_STATE_FREE); } - if (((i+1)%NUM_DIRTBL_PER_PAGE) == 0) + if (((i + 1) % NUM_DIRTBL_PER_PAGE) == 0) { + DEBUG_PAGE_ACCESS_END(dirpage); vm_page_set_state(dirpage, PAGE_STATE_FREE); + } } free(map->arch_data->rtdir_virt); } @@ -545,6 +548,8 @@ map_tmap(vm_translation_map *map, addr_t va, addr_t pa, uint32 attributes) // mark the page WIRED vm_page_set_state(page, PAGE_STATE_WIRED); + DEBUG_PAGE_ACCESS_END(page); + pgdir = page->physical_page_number * B_PAGE_SIZE; TRACE(("map_tmap: asked for free page for pgdir. 0x%lx\n", pgdir)); @@ -591,6 +596,8 @@ map_tmap(vm_translation_map *map, addr_t va, addr_t pa, uint32 attributes) // mark the page WIRED vm_page_set_state(page, PAGE_STATE_WIRED); + DEBUG_PAGE_ACCESS_END(page); + pgtable = page->physical_page_number * B_PAGE_SIZE; TRACE(("map_tmap: asked for free page for pgtable. 0x%lx\n", pgtable)); diff --git a/src/system/kernel/arch/x86/arch_vm_translation_map.cpp b/src/system/kernel/arch/x86/arch_vm_translation_map.cpp index de4d00d5a6..ac6ba6c692 100644 --- a/src/system/kernel/arch/x86/arch_vm_translation_map.cpp +++ b/src/system/kernel/arch/x86/arch_vm_translation_map.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2008-2009, Ingo Weinhold, ingo_weinhold@gmx.de. + * Copyright 2008-2010, Ingo Weinhold, ingo_weinhold@gmx.de. * Copyright 2002-2007, Axel Dörfler, axeld@pinc-software.de. All rights reserved. * Distributed under the terms of the MIT License. * @@ -274,6 +274,7 @@ destroy_tmap(vm_translation_map *map) page = vm_lookup_page(pgtable_addr); if (!page) panic("destroy_tmap: didn't find pgtable page\n"); + DEBUG_PAGE_ACCESS_START(page); vm_page_set_state(page, PAGE_STATE_FREE); } } @@ -369,6 +370,8 @@ map_tmap(vm_translation_map *map, addr_t va, addr_t pa, uint32 attributes) // mark the page WIRED vm_page_set_state(page, PAGE_STATE_WIRED); + DEBUG_PAGE_ACCESS_END(page); + pgtable = page->physical_page_number * B_PAGE_SIZE; TRACE(("map_tmap: asked for free page for pgtable. 0x%lx\n", pgtable)); diff --git a/src/system/kernel/cache/file_cache.cpp b/src/system/kernel/cache/file_cache.cpp index 9bcde8bbbc..d6b027b338 100644 --- a/src/system/kernel/cache/file_cache.cpp +++ b/src/system/kernel/cache/file_cache.cpp @@ -92,6 +92,9 @@ private: off_t fOffset; uint32 fVecCount; size_t fSize; +#if DEBUG_PAGE_ACCESS + thread_id fAllocatingThread; +#endif }; typedef status_t (*cache_func)(file_cache_ref* ref, void* cookie, off_t offset, @@ -150,8 +153,6 @@ PrecacheIO::Prepare() uint32 i = 0; for (size_t pos = 0; pos < fSize; pos += B_PAGE_SIZE) { vm_page* page = vm_page_allocate_page(PAGE_STATE_FREE); - if (page == NULL) - break; fCache->InsertPage(page, fOffset + pos); @@ -160,15 +161,9 @@ PrecacheIO::Prepare() fPages[i++] = page; } - if (i != fPageCount) { - // allocating pages failed - while (i-- > 0) { - fCache->NotifyPageEvents(fPages[i], PAGE_EVENT_NOT_BUSY); - fCache->RemovePage(fPages[i]); - vm_page_set_state(fPages[i], PAGE_STATE_FREE); - } - return B_NO_MEMORY; - } +#if DEBUG_PAGE_ACCESS + fAllocatingThread = find_thread(NULL); +#endif return B_OK; } @@ -207,12 +202,17 @@ PrecacheIO::IOFinished(status_t status, bool partialTransfer, + bytesTouched, 0, B_PAGE_SIZE - bytesTouched); } + DEBUG_PAGE_ACCESS_TRANSFER(fPages[i], fAllocatingThread); + fPages[i]->state = PAGE_STATE_ACTIVE; fCache->NotifyPageEvents(fPages[i], PAGE_EVENT_NOT_BUSY); + + DEBUG_PAGE_ACCESS_END(fPages[i]); } // Free pages after failed I/O for (uint32 i = pagesTransferred; i < fPageCount; i++) { + DEBUG_PAGE_ACCESS_TRANSFER(fPages[i], fAllocatingThread); fCache->NotifyPageEvents(fPages[i], PAGE_EVENT_NOT_BUSY); fCache->RemovePage(fPages[i]); vm_page_set_state(fPages[i], PAGE_STATE_FREE); @@ -305,6 +305,7 @@ reserve_pages(file_cache_ref* ref, size_t reservePages, bool isWrite) (page = it.Next()) != NULL && left > 0;) { if (page->state != PAGE_STATE_MODIFIED && page->state != PAGE_STATE_BUSY) { + DEBUG_PAGE_ACCESS_START(page); cache->RemovePage(page); vm_page_set_state(page, PAGE_STATE_FREE); left--; @@ -382,8 +383,6 @@ read_into_cache(file_cache_ref* ref, void* cookie, off_t offset, for (size_t pos = 0; pos < numBytes; pos += B_PAGE_SIZE) { vm_page* page = pages[pageIndex++] = vm_page_allocate_page( PAGE_STATE_FREE); - if (page == NULL) - panic("no more pages!"); cache->InsertPage(page, offset + pos); @@ -436,6 +435,8 @@ read_into_cache(file_cache_ref* ref, void* cookie, off_t offset, // make the pages accessible in the cache for (int32 i = pageIndex; i-- > 0;) { + DEBUG_PAGE_ACCESS_END(pages[i]); + pages[i]->state = PAGE_STATE_ACTIVE; cache->NotifyPageEvents(pages[i], PAGE_EVENT_NOT_BUSY); @@ -610,6 +611,8 @@ write_to_cache(file_cache_ref* ref, void* cookie, off_t offset, pages[i]->state = PAGE_STATE_ACTIVE; else vm_page_set_state(pages[i], PAGE_STATE_MODIFIED); + + DEBUG_PAGE_ACCESS_END(pages[i]); } return status; @@ -798,6 +801,9 @@ cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer, // Since we don't actually map pages as part of an area, we have // to manually maintain their usage_count page->usage_count = 2; + // TODO: Just because this request comes from the FS API, it + // doesn't mean the page is not mapped. We might actually + // decrease the usage count of a hot page here. if (doWrite || useBuffer) { // Since the following user_mem{cpy,set}() might cause a page @@ -827,8 +833,11 @@ cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer, locker.Lock(); page->state = oldPageState; - if (doWrite && page->state != PAGE_STATE_MODIFIED) + if (doWrite && page->state != PAGE_STATE_MODIFIED) { + DEBUG_PAGE_ACCESS_START(page); vm_page_set_state(page, PAGE_STATE_MODIFIED); + DEBUG_PAGE_ACCESS_END(page); + } cache->NotifyPageEvents(page, PAGE_EVENT_NOT_BUSY); } @@ -1001,6 +1010,8 @@ cache_prefetch_vnode(struct vnode* vnode, off_t offset, size_t size) cache->ReleaseRefAndUnlock(); vm_page_unreserve_pages(reservePages); + // TODO: We should periodically unreserve as we go, so we don't + // unnecessarily put pressure on the free page pool. } diff --git a/src/system/kernel/vm/Jamfile b/src/system/kernel/vm/Jamfile index 78f5cf8980..25b8ed16c0 100644 --- a/src/system/kernel/vm/Jamfile +++ b/src/system/kernel/vm/Jamfile @@ -18,6 +18,7 @@ KernelMergeObject kernel_vm.o : VMKernelAddressSpace.cpp VMKernelArea.cpp VMNullCache.cpp + VMPageQueue.cpp VMUserAddressSpace.cpp VMUserArea.cpp diff --git a/src/system/kernel/vm/VMAnonymousCache.cpp b/src/system/kernel/vm/VMAnonymousCache.cpp index d56514b9b9..0495cf7737 100644 --- a/src/system/kernel/vm/VMAnonymousCache.cpp +++ b/src/system/kernel/vm/VMAnonymousCache.cpp @@ -1,6 +1,6 @@ /* * Copyright 2008, Zhao Shuai, upczhsh@163.com. - * Copyright 2008-2009, Ingo Weinhold, ingo_weinhold@gmx.de. + * Copyright 2008-2010, Ingo Weinhold, ingo_weinhold@gmx.de. * Copyright 2002-2009, Axel Dörfler, axeld@pinc-software.de. * Distributed under the terms of the MIT License. * @@ -952,6 +952,7 @@ VMAnonymousCache::_MergePagesSmallerConsumer(VMAnonymousCache* source) vm_page* sourcePage = source->LookupPage( (off_t)page->cache_offset << PAGE_SHIFT); if (sourcePage != NULL) { + DEBUG_PAGE_ACCESS_START(sourcePage); source->RemovePage(sourcePage); vm_page_free(source, sourcePage); } @@ -1000,6 +1001,7 @@ VMAnonymousCache::_MergeSwapPages(VMAnonymousCache* source) if (swapBlock->swap_slots[i] != SWAP_SLOT_NONE) { vm_page* page = source->LookupPage( (off_t)(swapBlockPageIndex + i) << PAGE_SHIFT); + DEBUG_PAGE_ACCESS_START(page); source->RemovePage(page); vm_page_free(source, page); } diff --git a/src/system/kernel/vm/VMCache.cpp b/src/system/kernel/vm/VMCache.cpp index 88cf072018..796e279484 100644 --- a/src/system/kernel/vm/VMCache.cpp +++ b/src/system/kernel/vm/VMCache.cpp @@ -612,6 +612,7 @@ VMCache::Delete() TRACE(("vm_cache_release_ref: freeing page 0x%lx\n", oldPage->physical_page_number)); + DEBUG_PAGE_ACCESS_START(page); vm_page_free(this, page); } @@ -1032,6 +1033,7 @@ VMCache::Resize(off_t newSize) } // remove the page and put it into the free queue + DEBUG_PAGE_ACCESS_START(page); vm_remove_all_page_mappings(page, NULL); ASSERT(page->wired_count == 0); // TODO: Find a real solution! Unmapping is probably fine, but @@ -1081,6 +1083,7 @@ VMCache::FlushAndRemoveAllPages() if (page->wired_count > 0 || !page->mappings.IsEmpty()) return B_BUSY; + DEBUG_PAGE_ACCESS_START(page); RemovePage(page); vm_page_free(this, page); // Note: When iterating through a IteratableSplayTree diff --git a/src/system/kernel/vm/VMPageQueue.cpp b/src/system/kernel/vm/VMPageQueue.cpp new file mode 100644 index 0000000000..290647524c --- /dev/null +++ b/src/system/kernel/vm/VMPageQueue.cpp @@ -0,0 +1,17 @@ +/* + * Copyright 2010, Ingo Weinhold, ingo_weinhold@gmx.de. + * Distributed under the terms of the MIT License. + */ + + +#include "VMPageQueue.h" + + +void +VMPageQueue::Init(const char* name, int lockingOrder) +{ + fName = name; + fLockingOrder = lockingOrder; + fCount = 0; + mutex_init(&fLock, fName); +} diff --git a/src/system/kernel/vm/VMPageQueue.h b/src/system/kernel/vm/VMPageQueue.h index c64d36db91..e5ae0b3050 100644 --- a/src/system/kernel/vm/VMPageQueue.h +++ b/src/system/kernel/vm/VMPageQueue.h @@ -1,5 +1,5 @@ /* - * Copyright 2009, Ingo Weinhold, ingo_weinhold@gmx.de. + * Copyright 2010, Ingo Weinhold, ingo_weinhold@gmx.de. * Copyright 2002-2008, Axel Dörfler, axeld@pinc-software.de. * Distributed under the terms of the MIT License. * @@ -10,6 +10,13 @@ #define VM_PAGE_QUEUE_H +#include + +#include +#include + + + struct VMPageQueue { public: typedef DoublyLinkedListfLockingOrder) { + Lock(); + other->Lock(); + } else { + other->Lock(); + Lock(); + } } @@ -147,18 +183,6 @@ VMPageQueue::RemoveHead() } -/*! Moves a page to the tail of this queue, but only does so if - the page is currently in another queue. -*/ -void -VMPageQueue::MoveFrom(VMPageQueue* from, vm_page* page) -{ - if (from != this) { - from->Remove(page); - Append(page); - } -} - vm_page* VMPageQueue::Head() const { @@ -194,4 +218,87 @@ 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 diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index af422d08ac..bad8ab99b9 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2009, Ingo Weinhold, ingo_weinhold@gmx.de. + * Copyright 2009-2010, Ingo Weinhold, ingo_weinhold@gmx.de. * Copyright 2002-2009, Axel Dörfler, axeld@pinc-software.de. * Distributed under the terms of the MIT License. * @@ -344,6 +344,9 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address, // unmap pages vm_unmap_pages(area, address, oldSize - newSize, false); + // TODO: preserveModified = false is wrong, since this could be a + // cloned area or a write-mmap()ed file, in which case we'd lose + // information. // If no one else uses the area's cache, we can resize it, too. if (cache->areas == area && area->cache_next == NULL @@ -366,6 +369,7 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address, // unmap pages vm_unmap_pages(area, oldBase, newBase - oldBase, false); + // TODO: See the vm_unmap_pages() above. // resize the area status_t error = addressSpace->ShrinkAreaHead(area, newSize); @@ -389,6 +393,7 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address, // unmap pages vm_unmap_pages(area, address, area->Size() - firstNewSize, false); + // TODO: See the vm_unmap_pages() above. // resize the area addr_t oldSize = area->Size(); @@ -874,6 +879,10 @@ vm_create_anonymous_area(team_id team, const char* name, void** address, vm_page* page = vm_page_allocate_page(newPageState); cache->InsertPage(page, offset); vm_map_page(area, page, address, protection); + // TODO: This sets the page state to "active", but it would + // make more sense to set it to "wired". + + DEBUG_PAGE_ACCESS_END(page); // Periodically unreserve pages we've already allocated, so that // we don't unnecessarily increase the pressure on the VM. @@ -917,9 +926,13 @@ vm_create_anonymous_area(team_id team, const char* name, void** address, physicalAddress); } + DEBUG_PAGE_ACCESS_START(page); + increment_page_wired_count(page); - vm_page_set_state(page, PAGE_STATE_WIRED); cache->InsertPage(page, offset); + vm_page_set_state(page, PAGE_STATE_WIRED); + + DEBUG_PAGE_ACCESS_END(page); } map->ops->unlock(map); @@ -950,8 +963,10 @@ vm_create_anonymous_area(team_id team, const char* name, void** address, panic("couldn't map physical page in page run\n"); increment_page_wired_count(page); - vm_page_set_state(page, PAGE_STATE_WIRED); cache->InsertPage(page, offset); + vm_page_set_state(page, PAGE_STATE_WIRED); + + DEBUG_PAGE_ACCESS_END(page); } map->ops->unlock(map); @@ -1246,9 +1261,11 @@ pre_map_area_pages(VMArea* area, VMCache* cache) if (page->state == PAGE_STATE_BUSY || page->usage_count <= 0) continue; + DEBUG_PAGE_ACCESS_START(page); vm_map_page(area, page, baseAddress + (page->cache_offset * B_PAGE_SIZE - cacheOffset), B_READ_AREA | B_KERNEL_READ_AREA); + DEBUG_PAGE_ACCESS_END(page); } } @@ -1543,10 +1560,17 @@ vm_clone_area(team_id team, const char* name, void** address, // map in all pages from source 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); + if (page->state != PAGE_STATE_BUSY) { + DEBUG_PAGE_ACCESS_START(page); + vm_map_page(newArea, page, + newArea->Base() + ((page->cache_offset << PAGE_SHIFT) + - newArea->cache_offset), + protection); + DEBUG_PAGE_ACCESS_END(page); + } } + // TODO: B_FULL_LOCK means that all pages are locked. We are not + // ensuring that! vm_page_unreserve_pages(reservePages); } @@ -1573,6 +1597,9 @@ delete_area(VMAddressSpace* addressSpace, VMArea* area) // Unmap the virtual address space the area occupied vm_unmap_pages(area, area->Base(), area->Size(), !area->cache->temporary); + // TODO: Even if the cache is temporary we might need to preserve the + // modified flag, since the area could be a clone and backed by swap. + // We would lose information in this case. if (!area->cache->temporary) area->cache->WriteModified(); @@ -1995,6 +2022,9 @@ vm_remove_all_page_mappings(vm_page* page, uint32* _flags) } +/*! If \a preserveModified is \c true, the caller must hold the lock of the + page's cache and the page must not be busy. +*/ bool vm_unmap_page(VMArea* area, addr_t virtualAddress, bool preserveModified) { @@ -2068,6 +2098,11 @@ vm_unmap_page(VMArea* area, addr_t virtualAddress, bool preserveModified) } +/*! If \a preserveModified is \c true, the caller must hold the lock of all + mapped pages' caches and none of the pages must be busy. + TODO: Particularly the latter is very inconvenient. See the TODOs below for + reasons for this requirement. +*/ status_t vm_unmap_pages(VMArea* area, addr_t base, size_t size, bool preserveModified) { @@ -2116,9 +2151,18 @@ vm_unmap_pages(VMArea* area, addr_t base, size_t size, bool preserveModified) physicalAddress); } + DEBUG_PAGE_ACCESS_START(page); + // TODO: No guarantee for that. See below. + if ((flags & PAGE_MODIFIED) != 0 - && page->state != PAGE_STATE_MODIFIED) + && page->state != PAGE_STATE_MODIFIED) { vm_page_set_state(page, PAGE_STATE_MODIFIED); + // TODO: We are only allowed to do this, if (a) we have also + // locked the cache and (b) the page is not busy! Not doing + // it is problematic, too, since we'd lose information. + } + + DEBUG_PAGE_ACCESS_END(page); } } map->ops->unlock(map); @@ -2170,6 +2214,8 @@ vm_map_page(VMArea* area, vm_page* page, addr_t address, uint32 protection) vm_translation_map* map = &area->address_space->TranslationMap(); vm_page_mapping* mapping = NULL; + DEBUG_PAGE_ACCESS_CHECK(page); + if (area->wiring == B_NO_LOCK) { mapping = (vm_page_mapping*)malloc_nogrow(sizeof(vm_page_mapping)); if (mapping == NULL) @@ -2883,8 +2929,10 @@ unmap_and_free_physical_pages(vm_translation_map* map, addr_t start, addr_t end) if (map->ops->query(map, current, &physicalAddress, &flags) == B_OK && (flags & PAGE_PRESENT) != 0) { vm_page* page = vm_lookup_page(physicalAddress / B_PAGE_SIZE); - if (page != NULL) + if (page != NULL) { + DEBUG_PAGE_ACCESS_START(page); vm_page_set_state(page, PAGE_STATE_FREE); + } } } @@ -3708,6 +3756,8 @@ fault_get_page(PageFaultContext& context) page->state = PAGE_STATE_ACTIVE; cache->NotifyPageEvents(page, PAGE_EVENT_NOT_BUSY); + DEBUG_PAGE_ACCESS_END(page); + // Since we needed to unlock everything temporarily, the area // situation might have changed. So we need to restart the whole // process. @@ -3749,7 +3799,8 @@ fault_get_page(PageFaultContext& context) // insert the new page into our cache context.topCache->InsertPage(page, context.cacheOffset); - } + } else + DEBUG_PAGE_ACCESS_START(page); context.page = page; return B_OK; @@ -3870,7 +3921,7 @@ vm_soft_fault(VMAddressSpace* addressSpace, addr_t originalAddress, addr_t physicalAddress; uint32 flags; - vm_page* mappedPage; + vm_page* mappedPage = NULL; if (context.map->ops->query(context.map, address, &physicalAddress, &flags) == B_OK && (flags & PAGE_PRESENT) != 0 @@ -3889,12 +3940,26 @@ vm_soft_fault(VMAddressSpace* addressSpace, addr_t originalAddress, context.map->ops->unlock(context.map); - if (unmapPage) + if (unmapPage) { + // Note: The mapped page is a page of a lower cache. We are + // guaranteed to have that cached locked, our new page is a copy of + // that page, and the page is not busy. The logic for that guarantee + // is as follows: Since the page is mapped, it must live in the top + // cache (ruled out above) or any of its lower caches, and there is + // (was before the new page was inserted) no other page in any + // cache between the top cache and the page's cache (otherwise that + // would be mapped instead). That in turn means that our algorithm + // must have found it and therefore it cannot be busy either. + DEBUG_PAGE_ACCESS_START(mappedPage); vm_unmap_page(area, address, true); + DEBUG_PAGE_ACCESS_END(mappedPage); + } if (mapPage) vm_map_page(area, context.page, address, newProtection); + DEBUG_PAGE_ACCESS_END(context.page); + break; } @@ -5241,6 +5306,8 @@ _user_set_memory_protection(void* _address, size_t size, int protection) if (unmapPage) vm_unmap_page(area, pageAddress, true); + // TODO: We need to lock the page's cache for that, since + // it potentially changes the page's state. } } diff --git a/src/system/kernel/vm/vm_daemons.cpp b/src/system/kernel/vm/vm_daemons.cpp index 0bb61bb6e1..0c8a01b9fd 100644 --- a/src/system/kernel/vm/vm_daemons.cpp +++ b/src/system/kernel/vm/vm_daemons.cpp @@ -271,6 +271,8 @@ check_page_activation(int32 index) if (!locker.IsLocked()) return false; + DEBUG_PAGE_ACCESS_START(page); + bool modified; int32 activation = vm_test_map_activation(page, &modified); if (modified && page->state != PAGE_STATE_MODIFIED) { @@ -293,6 +295,7 @@ check_page_activation(int32 index) track_page_usage(page); #endif + DEBUG_PAGE_ACCESS_END(page); return false; } @@ -310,8 +313,10 @@ check_page_activation(int32 index) // recheck eventual last minute changes if ((flags & PAGE_MODIFIED) != 0 && page->state != PAGE_STATE_MODIFIED) vm_page_set_state(page, PAGE_STATE_MODIFIED); - if ((flags & PAGE_ACCESSED) != 0 && ++page->usage_count >= 0) + if ((flags & PAGE_ACCESSED) != 0 && ++page->usage_count >= 0) { + DEBUG_PAGE_ACCESS_END(page); return false; + } if (page->state == PAGE_STATE_MODIFIED) vm_page_schedule_write_page(page); @@ -321,6 +326,7 @@ check_page_activation(int32 index) T(DeactivatePage(page)); } + DEBUG_PAGE_ACCESS_END(page); return true; } @@ -334,6 +340,8 @@ free_page_swap_space(int32 index) if (!locker.IsLocked()) return false; + DEBUG_PAGE_ACCESS_START(page); + if (page->cache->temporary && page->wired_count == 0 && page->cache->HasPage(page->cache_offset << PAGE_SHIFT) && page->usage_count > 0) { @@ -343,9 +351,11 @@ free_page_swap_space(int32 index) // stolen and we'd lose its data. vm_page_set_state(page, PAGE_STATE_MODIFIED); T(FreedPageSwap(page)); + DEBUG_PAGE_ACCESS_END(page); return true; } } + DEBUG_PAGE_ACCESS_END(page); return false; } #endif diff --git a/src/system/kernel/vm/vm_page.cpp b/src/system/kernel/vm/vm_page.cpp index 5abddea4f6..9d3f59de59 100644 --- a/src/system/kernel/vm/vm_page.cpp +++ b/src/system/kernel/vm/vm_page.cpp @@ -1,4 +1,5 @@ /* + * Copyright 2010, Ingo Weinhold, ingo_weinhold@gmx.de. * Copyright 2002-2009, Axel Dörfler, axeld@pinc-software.de. * Distributed under the terms of the MIT License. * @@ -71,10 +72,10 @@ static addr_t sPhysicalPageOffset; static size_t sNumPages; static vint32 sUnreservedFreePages; static vint32 sPageDeficit; -static size_t sModifiedTemporaryPages; +static vint32 sModifiedTemporaryPages; static ConditionVariable sFreePageCondition; -static mutex sPageLock = MUTEX_INITIALIZER("pages"); +static mutex sPageDeficitLock = MUTEX_INITIALIZER("page deficit"); static sem_id sWriterWaitSem; @@ -392,6 +393,9 @@ dump_page(int argc, char **argv) #if DEBUG_PAGE_QUEUE kprintf("queue: %p\n", page->queue); #endif + #if DEBUG_PAGE_ACCESS + kprintf("accessor: %" B_PRId32 "\n", page->accessing_thread); + #endif kprintf("area mappings:\n"); vm_page_mappings::Iterator iterator = page->mappings.GetIterator(); @@ -503,7 +507,7 @@ dump_page_stats(int argc, char **argv) kprintf("wired: %lu\nmodified: %lu\nfree: %lu\nclear: %lu\n", counter[PAGE_STATE_WIRED], counter[PAGE_STATE_MODIFIED], counter[PAGE_STATE_FREE], counter[PAGE_STATE_CLEAR]); - kprintf("unreserved free pages: %lu\n", sUnreservedFreePages); + kprintf("unreserved free pages: %" B_PRId32 "\n", sUnreservedFreePages); kprintf("page deficit: %lu\n", sPageDeficit); kprintf("mapped pages: %lu\n", gMappedPagesCount); @@ -522,22 +526,25 @@ dump_page_stats(int argc, char **argv) } -static status_t -set_page_state_nolock(vm_page *page, int pageState) +/*! 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) { - if (pageState == page->state) - return B_OK; + DEBUG_PAGE_ACCESS_CHECK(page); - VMPageQueue *fromQueue = NULL; - VMPageQueue *toQueue = NULL; + if (pageState == page->state) + return; + + VMPageQueue* fromQueue; int32 freeCountDiff = 0; switch (page->state) { case PAGE_STATE_BUSY: case PAGE_STATE_ACTIVE: - case PAGE_STATE_WIRED: - case PAGE_STATE_UNUSED: fromQueue = &sActivePageQueue; break; case PAGE_STATE_INACTIVE: @@ -554,10 +561,14 @@ set_page_state_nolock(vm_page *page, int pageState) fromQueue = &sClearPageQueue; freeCountDiff = -1; break; - default: - panic("vm_page_set_state: vm_page %p in invalid state %d\n", - page, page->state); + case PAGE_STATE_WIRED: + case PAGE_STATE_UNUSED: + fromQueue = NULL; break; + default: + panic("set_page_state: vm_page %p in invalid state %d\n", + page, page->state); + return; } if (page->state == PAGE_STATE_CLEAR || page->state == PAGE_STATE_FREE) { @@ -565,11 +576,11 @@ set_page_state_nolock(vm_page *page, int pageState) panic("free page %p has cache", page); } + VMPageQueue* toQueue; + switch (pageState) { case PAGE_STATE_BUSY: case PAGE_STATE_ACTIVE: - case PAGE_STATE_WIRED: - case PAGE_STATE_UNUSED: toQueue = &sActivePageQueue; break; case PAGE_STATE_INACTIVE: @@ -586,14 +597,22 @@ set_page_state_nolock(vm_page *page, int pageState) toQueue = &sClearPageQueue; freeCountDiff++; break; + case PAGE_STATE_WIRED: + case PAGE_STATE_UNUSED: + toQueue = NULL; + break; default: - panic("vm_page_set_state: invalid target state %d\n", pageState); + panic("set_page_state: invalid target state %d\n", pageState); + return; } if (pageState == PAGE_STATE_CLEAR || pageState == PAGE_STATE_FREE || pageState == PAGE_STATE_INACTIVE) { - if (sPageDeficit > 0) - sFreePageCondition.NotifyOne(); + if (sPageDeficit > 0) { + MutexLocker pageDeficitLocker(sPageDeficitLock); + if (sPageDeficit > 0) + sFreePageCondition.NotifyOne(); + } if (pageState != PAGE_STATE_INACTIVE) { if (page->cache != NULL) @@ -604,9 +623,9 @@ set_page_state_nolock(vm_page *page, int pageState) } if (page->cache != NULL && page->cache->temporary) { if (pageState == PAGE_STATE_MODIFIED) - sModifiedTemporaryPages++; + atomic_add(&sModifiedTemporaryPages, 1); else if (page->state == PAGE_STATE_MODIFIED) - sModifiedTemporaryPages--; + atomic_add(&sModifiedTemporaryPages, -1); } #ifdef PAGE_ALLOCATION_TRACING @@ -616,22 +635,52 @@ set_page_state_nolock(vm_page *page, int pageState) } #endif // PAGE_ALLOCATION_TRACING - page->state = pageState; - toQueue->MoveFrom(fromQueue, page); + // move the page + if (toQueue == fromQueue) { + // Note: Theoretically we are required to lock when changing the page + // state, even if we don't change the queue. We actually don't have to + // do this, though, since only for the active queue there are different + // page states and active pages have a cache that must be locked at + // this point. So we rely on the fact that everyone must lock the cache + // before trying to change/interpret the page state. + ASSERT(page->cache != NULL); + page->cache->AssertLocked(); + page->state = pageState; + } else { + VMPageQueuePairLocker locker; + if (!queuesLocked) + locker.SetTo(fromQueue, toQueue); + + if (fromQueue != NULL) + fromQueue->Remove(page); + + page->state = pageState; + + if (toQueue != NULL) { + if (pageState == PAGE_STATE_CLEAR || pageState == PAGE_STATE_FREE) { + 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); - - return B_OK; } /*! Moves a modified page into either the active or inactive page queue depending on its usage count and wiring. + The page queues must not be locked. */ static void move_page_to_active_or_inactive_queue(vm_page *page, bool dequeued) { + DEBUG_PAGE_ACCESS_CHECK(page); + // Note, this logic must be in sync with what the page daemon does int32 state; if (!page->mappings.IsEmpty() || page->usage_count >= 0 @@ -644,11 +693,13 @@ 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.Append(page); + queue.Unlock(); if (page->cache->temporary) - sModifiedTemporaryPages--; + atomic_add(&sModifiedTemporaryPages, -1); } else - set_page_state_nolock(page, state); + set_page_state(page, state, false); } @@ -688,7 +739,7 @@ page_scrubber(void *unused) continue; // get some pages from the free queue - MutexLocker locker(sPageLock); + AutoLocker freeQueueLocker(sFreePageQueue); vm_page *page[SCRUB_SIZE]; int32 scrubCount = 0; @@ -697,11 +748,13 @@ page_scrubber(void *unused) if (page[i] == NULL) break; + DEBUG_PAGE_ACCESS_START(page[i]); + page[i]->state = PAGE_STATE_BUSY; scrubCount++; } - locker.Unlock(); + freeQueueLocker.Unlock(); if (scrubCount == 0) { vm_page_unreserve_pages(SCRUB_SIZE); @@ -714,15 +767,16 @@ page_scrubber(void *unused) for (int32 i = 0; i < scrubCount; i++) clear_page(page[i]); - locker.Lock(); + AutoLocker clearQueueLocker(sClearPageQueue); // and put them into the clear queue for (int32 i = 0; i < scrubCount; i++) { page[i]->state = PAGE_STATE_CLEAR; sClearPageQueue.Append(page[i]); + DEBUG_PAGE_ACCESS_END(page[i]); } - locker.Unlock(); + clearQueueLocker.Unlock(); vm_page_unreserve_pages(SCRUB_SIZE); @@ -746,6 +800,8 @@ remove_page_marker(struct vm_page &marker) if (marker.state == PAGE_STATE_UNUSED) return; + DEBUG_PAGE_ACCESS_CHECK(&marker); + VMPageQueue *queue; switch (marker.state) { @@ -763,8 +819,9 @@ remove_page_marker(struct vm_page &marker) return; } - MutexLocker locker(sPageLock); + queue->Lock(); queue->Remove(&marker); + queue->Unlock(); marker.state = PAGE_STATE_UNUSED; } @@ -773,9 +830,11 @@ remove_page_marker(struct vm_page &marker) static vm_page * next_modified_page(struct vm_page &marker) { - MutexLocker locker(sPageLock); + AutoLocker locker(sModifiedPageQueue); vm_page *page; + DEBUG_PAGE_ACCESS_CHECK(&marker); + if (marker.state == PAGE_STATE_MODIFIED) { page = sModifiedPageQueue.Next(&marker); sModifiedPageQueue.Remove(&marker); @@ -884,9 +943,14 @@ PageWriteWrapper::~PageWriteWrapper() } +/*! The page's cache must be locked. + The modified queue must be locked. +*/ void PageWriteWrapper::SetTo(vm_page* page, bool dequeuedPage) { + DEBUG_PAGE_ACCESS_CHECK(page); + if (page->state == PAGE_STATE_BUSY) panic("setting page write wrapper to busy page"); @@ -904,6 +968,8 @@ PageWriteWrapper::SetTo(vm_page* page, bool dequeuedPage) } +/*! The page's cache must be locked. +*/ void PageWriteWrapper::ClearModifiedFlag() { @@ -921,6 +987,8 @@ PageWriteWrapper::ClearModifiedFlag() } +/*! The page's cache must be locked. +*/ void PageWriteWrapper::CheckRemoveFromShrunkenCache() { @@ -932,26 +1000,34 @@ PageWriteWrapper::CheckRemoveFromShrunkenCache() } +/*! The page's cache must be locked. + The page queues must not be locked. +*/ void PageWriteWrapper::Done(status_t result) { if (!fIsActive) panic("completing page write wrapper that is not active"); + DEBUG_PAGE_ACCESS_CHECK(fPage); + if (result == B_OK) { // put it into the active/inactive queue move_page_to_active_or_inactive_queue(fPage, fDequeuedPage); fPage->busy_writing = false; + DEBUG_PAGE_ACCESS_END(fPage); } else { // Writing the page failed -- move to the modified queue. If we dequeued // it from there, just enqueue it again, otherwise set the page state // explicitly, which will take care of moving between the queues. if (fDequeuedPage) { fPage->state = PAGE_STATE_MODIFIED; + sModifiedPageQueue.Lock(); sModifiedPageQueue.Append(fPage); + sModifiedPageQueue.Unlock(); } else { fPage->state = fOldPageState; - set_page_state_nolock(fPage, PAGE_STATE_MODIFIED); + set_page_state(fPage, PAGE_STATE_MODIFIED, false); } if (!fPage->busy_writing) { @@ -961,12 +1037,14 @@ PageWriteWrapper::Done(status_t result) // Adjust temporary modified pages count, if necessary. if (fDequeuedPage && fCache->temporary) - sModifiedTemporaryPages--; + atomic_add(&sModifiedTemporaryPages, -1); // free the page - set_page_state_nolock(fPage, PAGE_STATE_FREE); - } else + set_page_state(fPage, PAGE_STATE_FREE, false); + } else { fPage->busy_writing = false; + DEBUG_PAGE_ACCESS_END(fPage); + } } @@ -975,6 +1053,8 @@ PageWriteWrapper::Done(status_t result) } +/*! The page's cache must be locked. +*/ void PageWriteTransfer::SetTo(PageWriterRun* run, vm_page* page, int32 maxPages) { @@ -991,6 +1071,8 @@ PageWriteTransfer::SetTo(PageWriterRun* run, vm_page* page, int32 maxPages) } +/*! The page's cache must be locked. +*/ bool PageWriteTransfer::AddPage(vm_page* page) { @@ -1114,6 +1196,9 @@ PageWriterRun::PrepareNextRun() } +/*! The page's cache must be locked. + The modified queue must be locked. +*/ void PageWriterRun::AddPage(vm_page* page) { @@ -1155,11 +1240,9 @@ PageWriterRun::Go() fWrappers[checkIndex++].CheckRemoveFromShrunkenCache(); } - MutexLocker locker(sPageLock); for (uint32 j = 0; j < transfer.PageCount(); j++) fWrappers[wrapperIndex++].Done(transfer.Status()); - locker.Unlock(); transfer.Cache()->Unlock(); } @@ -1214,9 +1297,16 @@ page_writer(void* /*unused*/) marker.type = PAGE_TYPE_DUMMY; marker.cache = NULL; marker.state = PAGE_STATE_UNUSED; +#if DEBUG_PAGE_QUEUE + marker.queue = NULL; +#endif +#if DEBUG_PAGE_ACCESS + marker.accessing_thread = thread_get_current_thread_id(); +#endif while (true) { - if (sModifiedPageQueue.Count() - sModifiedTemporaryPages < 1024) { + if ((int32)sModifiedPageQueue.Count() - sModifiedTemporaryPages + < 1024) { int32 count = 0; get_sem_count(sWriterWaitSem, &count); if (count == 0) @@ -1226,10 +1316,13 @@ page_writer(void* /*unused*/) // all 3 seconds when no one triggers us } + int32 modifiedPages = sModifiedPageQueue.Count() + - sModifiedTemporaryPages; + if (modifiedPages <= 0) + continue; + // depending on how urgent it becomes to get pages to disk, we adjust // our I/O priority - page_num_t modifiedPages = sModifiedPageQueue.Count() - - sModifiedTemporaryPages; uint32 lowPagesState = low_resource_state(B_KERNEL_RESOURCE_PAGES); int32 ioPriority = B_IDLE_PRIORITY; if (lowPagesState >= B_LOW_RESOURCE_CRITICAL @@ -1265,6 +1358,8 @@ page_writer(void* /*unused*/) if (!cacheLocker.IsLocked()) continue; + DEBUG_PAGE_ACCESS_START(page); + VMCache *cache = page->cache; // Don't write back wired (locked) pages and don't write RAM pages @@ -1278,32 +1373,33 @@ page_writer(void* /*unused*/) (off_t)page->cache_offset << PAGE_SHIFT)) #endif )) { + DEBUG_PAGE_ACCESS_END(page); continue; } // we need our own reference to the store, as it might // currently be destructed if (cache->AcquireUnreferencedStoreRef() != B_OK) { + DEBUG_PAGE_ACCESS_END(page); cacheLocker.Unlock(); thread_yield(true); continue; } - MutexLocker locker(sPageLock); - // state might have changed while we were locking the cache if (page->state != PAGE_STATE_MODIFIED) { // release the cache reference - locker.Unlock(); + DEBUG_PAGE_ACCESS_END(page); cache->ReleaseStoreRef(); continue; } - sModifiedPageQueue.Remove(page); + sModifiedPageQueue.Lock(); + sModifiedPageQueue.Remove(page); run.AddPage(page); - locker.Unlock(); + sModifiedPageQueue.Unlock(); //dprintf("write page %p, cache %p (%ld)\n", page, page->cache, page->cache->ref_count); TPW(WritePage(page)); @@ -1346,25 +1442,23 @@ page_writer(void* /*unused*/) static vm_page * find_page_candidate(struct vm_page &marker) { - MutexLocker locker(sPageLock); - VMPageQueue *queue; + DEBUG_PAGE_ACCESS_CHECK(&marker); + + AutoLocker locker(sInactivePageQueue); vm_page *page; if (marker.state == PAGE_STATE_UNUSED) { // Get the first free pages of the (in)active queue - queue = &sInactivePageQueue; page = sInactivePageQueue.Head(); } else { // Get the next page of the current queue - if (marker.state == PAGE_STATE_INACTIVE) { - queue = &sInactivePageQueue; - } else { + if (marker.state != PAGE_STATE_INACTIVE) { panic("invalid marker %p state", &marker); - queue = NULL; + return NULL; } - page = queue->Next(&marker); - queue->Remove(&marker); + page = sInactivePageQueue.Next(&marker); + sInactivePageQueue.Remove(&marker); marker.state = PAGE_STATE_UNUSED; } @@ -1372,11 +1466,11 @@ find_page_candidate(struct vm_page &marker) if (!is_marker_page(page) && page->state == PAGE_STATE_INACTIVE) { // we found a candidate, insert marker marker.state = PAGE_STATE_INACTIVE; - queue->InsertAfter(page, &marker); + sInactivePageQueue.InsertAfter(page, &marker); return page; } - page = queue->Next(page); + page = sInactivePageQueue.Next(page); } return NULL; @@ -1390,23 +1484,27 @@ steal_page(vm_page *page) if (vm_cache_acquire_locked_page_cache(page, false) == NULL) return false; - AutoLocker cacheLocker(page->cache, true, false); + AutoLocker cacheLocker(page->cache, true); MethodDeleter _2(page->cache, &VMCache::ReleaseRefLocked); // check again if that page is still a candidate if (page->state != PAGE_STATE_INACTIVE) return false; + DEBUG_PAGE_ACCESS_START(page); + // recheck eventual last minute changes uint32 flags; vm_remove_all_page_mappings(page, &flags); if ((flags & PAGE_MODIFIED) != 0) { // page was modified, don't steal it - vm_page_set_state(page, PAGE_STATE_MODIFIED); + set_page_state(page, PAGE_STATE_MODIFIED, false); + DEBUG_PAGE_ACCESS_END(page); return false; } else if ((flags & PAGE_ACCESSED) != 0) { // page is in active use, don't steal it - vm_page_set_state(page, PAGE_STATE_ACTIVE); + set_page_state(page, PAGE_STATE_ACTIVE, false); + DEBUG_PAGE_ACCESS_END(page); return false; } @@ -1417,7 +1515,7 @@ steal_page(vm_page *page) page->cache->RemovePage(page); - MutexLocker _(sPageLock); + AutoLocker _(sInactivePageQueue); sInactivePageQueue.Remove(page); return true; } @@ -1431,6 +1529,12 @@ steal_pages(vm_page **pages, size_t count) marker.type = PAGE_TYPE_DUMMY; marker.cache = NULL; marker.state = PAGE_STATE_UNUSED; +#if DEBUG_PAGE_QUEUE + marker.queue = NULL; +#endif +#if DEBUG_PAGE_ACCESS + marker.accessing_thread = thread_get_current_thread_id(); +#endif bool tried = false; size_t stolen = 0; @@ -1441,9 +1545,10 @@ steal_pages(vm_page **pages, size_t count) break; if (steal_page(page)) { - MutexLocker locker(sPageLock); + AutoLocker locker(sFreePageQueue); sFreePageQueue.Append(page); page->state = PAGE_STATE_FREE; + DEBUG_PAGE_ACCESS_END(page); locker.Unlock(); atomic_add(&sUnreservedFreePages, 1); @@ -1458,8 +1563,6 @@ steal_pages(vm_page **pages, size_t count) remove_page_marker(marker); - MutexLocker locker(sPageLock); - if (count == 0 || sUnreservedFreePages >= 0) return stolen; @@ -1470,6 +1573,8 @@ steal_pages(vm_page **pages, size_t count) // we need to wait for pages to become inactive + MutexLocker pageDeficitLocker(sPageDeficitLock); + sPageDeficit++; if (sUnreservedFreePages >= 0) { // There are enough pages available now. No need to wait after all. @@ -1479,7 +1584,7 @@ steal_pages(vm_page **pages, size_t count) ConditionVariableEntry freeConditionEntry; freeConditionEntry.Add(&sFreePageQueue); - locker.Unlock(); + pageDeficitLocker.Unlock(); if (tried) { // We tried all potential pages, but one or more couldn't be stolen @@ -1495,7 +1600,7 @@ steal_pages(vm_page **pages, size_t count) freeConditionEntry.Wait(); } - locker.Lock(); + pageDeficitLocker.Lock(); sPageDeficit--; if (sUnreservedFreePages >= 0) @@ -1554,12 +1659,15 @@ vm_page_write_modified_page_range(struct VMCache* cache, uint32 firstPage, bool dequeuedPage = false; if (page != NULL) { + DEBUG_PAGE_ACCESS_START(page); + if (page->state == PAGE_STATE_MODIFIED) { - MutexLocker locker(&sPageLock); + AutoLocker locker(sModifiedPageQueue); sModifiedPageQueue.Remove(page); dequeuedPage = true; } else if (page->state == PAGE_STATE_BUSY || !vm_test_map_modification(page)) { + DEBUG_PAGE_ACCESS_END(page); page = NULL; } } @@ -1599,13 +1707,9 @@ vm_page_write_modified_page_range(struct VMCache* cache, uint32 firstPage, wrappers[i]->CheckRemoveFromShrunkenCache(); } - MutexLocker locker(&sPageLock); - for (int32 i = 0; i < usedWrappers; i++) wrappers[i]->Done(status); - locker.Unlock(); - usedWrappers = 0; if (page != NULL) { @@ -1616,7 +1720,7 @@ vm_page_write_modified_page_range(struct VMCache* cache, uint32 firstPage, } if (wrapperPool != stackWrappers) - delete [] wrapperPool; + delete[] wrapperPool; return B_OK; } @@ -1661,10 +1765,14 @@ vm_page_schedule_write_page_range(struct VMCache *cache, uint32 firstPage, if (page->cache_offset >= endPage) break; + DEBUG_PAGE_ACCESS_START(page); + if (page->state == PAGE_STATE_MODIFIED) { vm_page_requeue(page, false); modified++; } + + DEBUG_PAGE_ACCESS_END(page); } if (modified > 0) @@ -1703,6 +1811,13 @@ vm_page_init(kernel_args *args) { TRACE(("vm_page_init: entry\n")); + // init page queues + sModifiedPageQueue.Init("modified pages queue", 5); + sInactivePageQueue.Init("inactive pages queue", 4); + sActivePageQueue.Init("active pages queue", 3); + sFreePageQueue.Init("free pages queue", 2); + sClearPageQueue.Init("clear pages queue", 1); + // map in the new free page table sPages = (vm_page *)vm_allocate_early(args, sNumPages * sizeof(vm_page), ~0L, B_KERNEL_READ_AREA | B_KERNEL_WRITE_AREA); @@ -1723,6 +1838,9 @@ vm_page_init(kernel_args *args) #if DEBUG_PAGE_QUEUE sPages[i].queue = NULL; #endif + #if DEBUG_PAGE_ACCESS + sPages[i].accessing_thread = -1; + #endif sFreePageQueue.Append(&sPages[i]); } @@ -1810,14 +1928,19 @@ vm_mark_page_range_inuse(addr_t startPage, addr_t length) return B_BAD_VALUE; } - MutexLocker _(sPageLock); + VMPageQueuePairLocker locker(sFreePageQueue, sClearPageQueue); 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: - set_page_state_nolock(page, PAGE_STATE_UNUSED); +// 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); + DEBUG_PAGE_ACCESS_END(page); break; case PAGE_STATE_WIRED: break; @@ -1854,7 +1977,7 @@ vm_page_unreserve_pages(uint32 count) atomic_add(&sUnreservedFreePages, count); if (sPageDeficit > 0) { - MutexLocker locker(sPageLock); + MutexLocker pageDeficitLocker(sPageDeficitLock); if (sPageDeficit > 0) sFreePageCondition.NotifyAll(); } @@ -1879,13 +2002,9 @@ vm_page_reserve_pages(uint32 count) if (oldFreePages >= (int32)count) return; - MutexLocker locker(sPageLock); - if (oldFreePages > 0) count -= oldFreePages; - locker.Unlock(); - steal_pages(NULL, count + 1); // we get one more, just in case we can do something someone // else can't @@ -1936,15 +2055,15 @@ vm_page_allocate_page(int pageState) atomic_add(&sUnreservedFreePages, -1); - MutexLocker locker(sPageLock); - T(AllocatePage()); + VMPageQueuePairLocker freeClearQueuelocker(sFreePageQueue, sClearPageQueue); + vm_page *page = queue->RemoveHead(); if (page == NULL) { #if DEBUG_PAGE_QUEUE if (queue->Count() != 0) - panic("queue %p corrupted, count = %d\n", queue, queue->Count()); + panic("queue %p corrupted, count = %ld\n", queue, queue->Count()); #endif // if the primary queue was empty, grab the page from the @@ -1958,15 +2077,20 @@ vm_page_allocate_page(int pageState) if (page->cache != NULL) panic("supposed to be free page %p has cache\n", page); + DEBUG_PAGE_ACCESS_START(page); + int oldPageState = page->state; page->state = PAGE_STATE_BUSY; page->usage_count = 2; + freeClearQueuelocker.Unlock(); + + sActivePageQueue.Lock(); sActivePageQueue.Append(page); + sActivePageQueue.Unlock(); - locker.Unlock(); - - // if needed take the page from the free queue and zero it out + // clear the page, if we had to take it from the free queue and a clear + // page was requested if (pageState == PAGE_STATE_CLEAR && oldPageState != PAGE_STATE_CLEAR) clear_page(page); @@ -1974,10 +2098,58 @@ vm_page_allocate_page(int pageState) } +static vm_page* +allocate_page_run(page_num_t start, page_num_t length, int pageState, + VMPageQueuePairLocker& freeClearQueuelocker) +{ + T(AllocatePageRun(length)); + + // pull the pages out of the appropriate queues + for (page_num_t i = 0; i < length; i++) { + vm_page& page = sPages[start + i]; + DEBUG_PAGE_ACCESS_START(&page); + if (page.state == PAGE_STATE_CLEAR) { + page.is_cleared = true; + sClearPageQueue.Remove(&page); + } else { + page.is_cleared = false; + sFreePageQueue.Remove(&page); + } + + page.state = PAGE_STATE_BUSY; + } + + freeClearQueuelocker.Unlock(); + + // add the pages to the active queue + AutoLocker activeQueueLocker(sActivePageQueue); + + for (page_num_t i = 0; i < length; i++) { + vm_page& page = sPages[start + i]; + sActivePageQueue.Append(&page); + page.usage_count = 1; + } + + activeQueueLocker.Unlock(); + + // clear pages, if requested + if (pageState == PAGE_STATE_CLEAR) { + for (page_num_t i = 0; i < length; i++) { + if (!sPages[start + i].is_cleared) + clear_page(&sPages[start + i]); + } + } + + // Note: We don't unreserve the pages since we pulled them out of the + // free/clear queues without adjusting sUnreservedFreePages. + + return &sPages[start]; +} + + vm_page * vm_page_allocate_page_run(int pageState, addr_t base, addr_t length) { - vm_page *firstPage = NULL; uint32 start = base >> PAGE_SHIFT; if (!vm_page_try_reserve_pages(length)) @@ -1985,12 +2157,15 @@ vm_page_allocate_page_run(int pageState, addr_t base, addr_t length) // TODO: add more tries, ie. free some inactive, ... // no free space - MutexLocker locker(sPageLock); + VMPageQueuePairLocker freeClearQueuelocker(sFreePageQueue, sClearPageQueue); for (;;) { bool foundRun = true; - if (start + length > sNumPages) - break; + if (start + length > sNumPages) { + freeClearQueuelocker.Unlock(); + vm_page_unreserve_pages(length); + return NULL; + } uint32 i; for (i = 0; i < length; i++) { @@ -2002,35 +2177,12 @@ vm_page_allocate_page_run(int pageState, addr_t base, addr_t length) } } - if (foundRun) { - // pull the pages out of the appropriate queues - for (i = 0; i < length; i++) { - sPages[start + i].is_cleared - = sPages[start + i].state == PAGE_STATE_CLEAR; - set_page_state_nolock(&sPages[start + i], PAGE_STATE_BUSY); - sPages[start + i].usage_count = 2; - } + if (foundRun) + return allocate_page_run(start, length, pageState, + freeClearQueuelocker); - firstPage = &sPages[start]; - break; - } else - start += i; + start += i; } - - T(AllocatePageRun(length)); - - locker.Unlock(); - - vm_page_unreserve_pages(length); - - if (firstPage != NULL && pageState == PAGE_STATE_CLEAR) { - for (uint32 i = 0; i < length; i++) { - if (!sPages[start + i].is_cleared) - clear_page(&sPages[start + i]); - } - } - - return firstPage; } @@ -2057,10 +2209,9 @@ vm_page_allocate_page_run_no_base(int pageState, addr_t count) // TODO: add more tries, ie. free some inactive, ... // no free space - MutexLocker locker(sPageLock); + VMPageQueuePairLocker freeClearQueuelocker(sFreePageQueue, sClearPageQueue); - vm_page *firstPage = NULL; - for (uint32 twice = 0; twice < 2; twice++) { + for (;;) { VMPageQueue::Iterator it = queue->GetIterator(); while (vm_page *page = it.Next()) { vm_page *current = page; @@ -2077,42 +2228,18 @@ vm_page_allocate_page_run_no_base(int pageState, addr_t count) } if (foundRun) { - atomic_add(&sUnreservedFreePages, -count); - - // pull the pages out of the appropriate queues - current = page; - for (uint32 i = 0; i < count; i++, current++) { - current->is_cleared = current->state == PAGE_STATE_CLEAR; - set_page_state_nolock(current, PAGE_STATE_BUSY); - current->usage_count = 2; - } - - firstPage = page; - break; + return allocate_page_run(page - sPages, count, pageState, + freeClearQueuelocker); } } - if (firstPage != NULL) - break; + if (queue == otherQueue) { + freeClearQueuelocker.Unlock(); + vm_page_unreserve_pages(count); + } queue = otherQueue; } - - T(AllocatePageRun(count)); - - locker.Unlock(); - - vm_page_unreserve_pages(count); - - if (firstPage != NULL && pageState == PAGE_STATE_CLEAR) { - vm_page *current = firstPage; - for (uint32 i = 0; i < count; i++, current++) { - if (!current->is_cleared) - clear_page(current); - } - } - - return firstPage; } @@ -2144,40 +2271,40 @@ vm_lookup_page(addr_t pageNumber) void vm_page_free(VMCache *cache, vm_page *page) { - MutexLocker _(sPageLock); + ASSERT(page->state != PAGE_STATE_FREE && page->state != PAGE_STATE_CLEAR); - if (page->cache == NULL && page->state == PAGE_STATE_MODIFIED - && cache->temporary) { - sModifiedTemporaryPages--; - } + if (page->state == PAGE_STATE_MODIFIED && cache->temporary) + atomic_add(&sModifiedTemporaryPages, -1); - set_page_state_nolock(page, PAGE_STATE_FREE); + set_page_state(page, PAGE_STATE_FREE, false); } -status_t +void vm_page_set_state(vm_page *page, int pageState) { - MutexLocker _(sPageLock); + ASSERT(page->state != PAGE_STATE_FREE && page->state != PAGE_STATE_CLEAR); - return set_page_state_nolock(page, pageState); + set_page_state(page, pageState, false); } /*! Moves a page to either the tail of the head of its current queue, depending on \a tail. + The page must have a cache and the cache must be locked! */ void vm_page_requeue(struct vm_page *page, bool tail) { - MutexLocker _(sPageLock); + ASSERT(page->cache != NULL); + page->cache->AssertLocked(); + DEBUG_PAGE_ACCESS_CHECK(page); + VMPageQueue *queue = NULL; switch (page->state) { case PAGE_STATE_BUSY: case PAGE_STATE_ACTIVE: - case PAGE_STATE_WIRED: - case PAGE_STATE_UNUSED: queue = &sActivePageQueue; break; case PAGE_STATE_INACTIVE: @@ -2187,23 +2314,28 @@ vm_page_requeue(struct vm_page *page, bool tail) queue = &sModifiedPageQueue; break; case PAGE_STATE_FREE: - queue = &sFreePageQueue; - break; case PAGE_STATE_CLEAR: - queue = &sClearPageQueue; - break; + panic("vm_page_requeue() called for free/clear page %p", page); + return; + case PAGE_STATE_WIRED: + case PAGE_STATE_UNUSED: + return; default: panic("vm_page_touch: vm_page %p in invalid state %d\n", page, page->state); break; } + queue->Lock(); + queue->Remove(page); if (tail) queue->Append(page); else queue->Prepend(page); + + queue->Unlock(); }