From 9d8c209014a71a82de460aafd94bed1f6aa90f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Thu, 4 Oct 2007 16:36:35 +0000 Subject: [PATCH] * vm_remove_all_page_mappings() now returns an accumulation of the flags of the unmapped page. * This is needed by everyone who calls this to make sure modifications to a page aren't ignored. Namely, the page scanner and the page thief were affected. * Cleaned up locking the page's cache a bit in page_thief(); there is now a helper class that takes care of everything. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22438 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/private/kernel/vm.h | 2 +- src/system/kernel/vm/vm.cpp | 16 +++++++- src/system/kernel/vm/vm_daemons.cpp | 10 ++++- src/system/kernel/vm/vm_page.cpp | 59 ++++++++++++++++++++++------- 4 files changed, 71 insertions(+), 16 deletions(-) diff --git a/headers/private/kernel/vm.h b/headers/private/kernel/vm.h index f729a5a3c2..05c256e863 100644 --- a/headers/private/kernel/vm.h +++ b/headers/private/kernel/vm.h @@ -130,7 +130,7 @@ status_t vm_get_page_mapping(team_id team, addr_t vaddr, addr_t *paddr); bool vm_test_map_modification(struct vm_page *page); int32 vm_test_map_activation(struct vm_page *page, bool *_modified); void vm_clear_map_flags(struct vm_page *page, uint32 flags); -void vm_remove_all_page_mappings(struct vm_page *page); +void vm_remove_all_page_mappings(struct vm_page *page, uint32 *_flags); status_t vm_unmap_pages(struct vm_area *area, addr_t base, size_t length); status_t vm_map_page(struct vm_area *area, struct vm_page *page, addr_t address, uint32 protection); diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index 4bdd502a2e..dc79ffc0b3 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -2433,9 +2433,14 @@ vm_clear_map_flags(vm_page *page, uint32 flags) } +/*! Removes all mappings from a page. + After you've called this function, the page is unmapped from memory. + The accumulated page flags of all mappings can be found in \a _flags. +*/ void -vm_remove_all_page_mappings(vm_page *page) +vm_remove_all_page_mappings(vm_page *page, uint32 *_flags) { + uint32 accumulatedFlags = 0; MutexLocker locker(sMappingLock); vm_page_mappings queue; @@ -2446,13 +2451,19 @@ vm_remove_all_page_mappings(vm_page *page) while ((mapping = iterator.Next()) != NULL) { vm_area *area = mapping->area; vm_translation_map *map = &area->address_space->translation_map; + addr_t physicalAddress; + uint32 flags; map->ops->lock(map); addr_t base = area->base + (page->cache_offset << PAGE_SHIFT); map->ops->unmap(map, base, base + (B_PAGE_SIZE - 1)); + map->ops->flush(map); + map->ops->query(map, base, &physicalAddress, &flags); map->ops->unlock(map); area->mappings.Remove(mapping); + + accumulatedFlags |= flags; } locker.Unlock(); @@ -2462,6 +2473,9 @@ vm_remove_all_page_mappings(vm_page *page) while ((mapping = queue.RemoveHead()) != NULL) { free(mapping); } + + if (_flags != NULL) + *_flags = accumulatedFlags; } diff --git a/src/system/kernel/vm/vm_daemons.cpp b/src/system/kernel/vm/vm_daemons.cpp index a452d4b7dd..133e74f084 100644 --- a/src/system/kernel/vm/vm_daemons.cpp +++ b/src/system/kernel/vm/vm_daemons.cpp @@ -139,7 +139,15 @@ check_page_activation(int32 index) page->usage_count--; if (page->usage_count < 0) { - vm_remove_all_page_mappings(page); + uint32 flags; + vm_remove_all_page_mappings(page, &flags); + + // 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) + return false; + if (page->state == PAGE_STATE_MODIFIED) vm_page_schedule_write_page(page); else diff --git a/src/system/kernel/vm/vm_page.cpp b/src/system/kernel/vm/vm_page.cpp index 6b6c901762..864da7b40d 100644 --- a/src/system/kernel/vm/vm_page.cpp +++ b/src/system/kernel/vm/vm_page.cpp @@ -842,34 +842,67 @@ page_thief(void* /*unused*/) // try to lock the page's cache - vm_cache* cache = vm_cache_acquire_page_cache_ref(page); - if (cache == NULL) + class PageCacheTryLocker { + public: + PageCacheTryLocker(vm_page *page) + : fIsLocked(false) + { + fCache = vm_cache_acquire_page_cache_ref(page); + if (fCache != NULL + && mutex_trylock(&fCache->lock) == B_OK) { + if (fCache == page->cache) + fIsLocked = true; + else + mutex_unlock(&fCache->lock); + } + } + + ~PageCacheTryLocker() + { + if (fIsLocked) + mutex_unlock(&fCache->lock); + + vm_cache_release_ref(fCache); + } + + bool IsLocked() { return fIsLocked; } + + private: + vm_cache *fCache; + bool fIsLocked; + } cacheLocker(page); + + if (!cacheLocker.IsLocked()) continue; - if (mutex_trylock(&cache->lock) != B_OK) { - vm_cache_release_ref(cache); - continue; - } if (page->state != PAGE_STATE_INACTIVE) { // TODO: if this is an active page (as in stealActive), we need // to unmap it and check if it's modified in an atomic operation. // For now, we'll just ignore it, even though this might let // vm_page_allocate_page() wait forever... - mutex_unlock(&cache->lock); - vm_cache_release_ref(cache); + continue; + } + + // 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); + continue; + } else if ((flags & PAGE_ACCESSED) != 0) { + // page is in active use, don't steal it + vm_page_set_state(page, PAGE_STATE_ACTIVE); continue; } // we can now steal this page //dprintf(" steal page %p from cache %p\n", page, cache); - vm_remove_all_page_mappings(page); - vm_cache_remove_page(cache, page); + + vm_cache_remove_page(page->cache, page); vm_page_set_state(page, PAGE_STATE_FREE); steal--; - - mutex_unlock(&cache->lock); - vm_cache_release_ref(cache); } }