diff --git a/headers/private/kernel/vm_cache.h b/headers/private/kernel/vm_cache.h index aa80aff033..e039a004e7 100644 --- a/headers/private/kernel/vm_cache.h +++ b/headers/private/kernel/vm_cache.h @@ -23,6 +23,7 @@ status_t vm_cache_init(struct kernel_args *args); vm_cache *vm_cache_create(vm_store *store); void vm_cache_acquire_ref(vm_cache *cache); void vm_cache_release_ref(vm_cache *cache); +vm_cache *vm_cache_acquire_page_cache_ref(vm_page *page); vm_page *vm_cache_lookup_page(vm_cache *cache, off_t page); void vm_cache_insert_page(vm_cache *cache, vm_page *page, off_t offset); void vm_cache_remove_page(vm_cache *cache, vm_page *page); diff --git a/src/system/kernel/vm/vm_cache.cpp b/src/system/kernel/vm/vm_cache.cpp index 5762ebbd58..a9e67adfef 100644 --- a/src/system/kernel/vm/vm_cache.cpp +++ b/src/system/kernel/vm/vm_cache.cpp @@ -11,6 +11,9 @@ #include #include +#include +#include + #include #include #include @@ -18,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -80,6 +82,28 @@ page_hash_func(void *_p, const void *_key, uint32 range) } +/*! Acquires a pseudo 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. To get a real reference, the caller + must subsequently call vm_cache_acquire_ref() and decrement the cache's ref + count manually afterwards. + Returns \c true, if the pseudo reference could be acquired. +*/ +static inline bool +acquire_unreferenced_cache_pseudo_ref(vm_cache* cache) +{ + while (true) { + int32 count = cache->ref_count; + if (count == 0) + return false; + + if (atomic_test_and_set(&cache->ref_count, count + 1, count) == count) + return true; + } +} + + status_t vm_cache_init(kernel_args *args) { @@ -275,6 +299,29 @@ vm_cache_release_ref(vm_cache *cache) } +vm_cache* +vm_cache_acquire_page_cache_ref(vm_page* page) +{ + InterruptsSpinLocker locker(sPageCacheTableLock); + + vm_cache* cache = page->cache; + if (cache == NULL) + return NULL; + + // get a pseudo reference + if (!acquire_unreferenced_cache_pseudo_ref(cache)) + return NULL; + + locker.Unlock(); + + // turn it into a real reference + vm_cache_acquire_ref(cache); + atomic_add(&cache->ref_count, -1); + + return cache; +} + + vm_page * vm_cache_lookup_page(vm_cache *cache, off_t offset) { @@ -326,12 +373,13 @@ vm_cache_insert_page(vm_cache *cache, vm_page *page, off_t offset) cache->page_list = page; cache->page_count++; - page->cache = cache; page->usage_count = 2; state = disable_interrupts(); acquire_spinlock(&sPageCacheTableLock); + page->cache = cache; + #if KDEBUG struct page_lookup_key key; key.offset = (uint32)(offset >> PAGE_SHIFT); @@ -373,6 +421,7 @@ vm_cache_remove_page(vm_cache *cache, vm_page *page) acquire_spinlock(&sPageCacheTableLock); hash_remove(sPageCacheTable, page); + page->cache = NULL; release_spinlock(&sPageCacheTableLock); restore_interrupts(state); @@ -388,7 +437,6 @@ vm_cache_remove_page(vm_cache *cache, vm_page *page) page->cache_next->cache_prev = page->cache_prev; } cache->page_count--; - page->cache = NULL; } @@ -499,27 +547,23 @@ vm_cache_remove_consumer(vm_cache *cache, vm_cache *consumer) && cache->consumers.link.next == cache->consumers.link.prev) { // The cache is not really needed anymore - it can be merged with its only // consumer left. - bool merge = false; consumer = (vm_cache *)list_get_first_item(&cache->consumers); - // Our cache doesn't have a ref to its consumer (only the other way around), - // so we cannot just acquire it here; it might be deleted right now - while (true) { - int32 count = consumer->ref_count; - if (count == 0) - break; - - if (atomic_test_and_set(&consumer->ref_count, count + 1, count) == count) { - // We managed to grab a reference to the consumerRef. - // Since this doesn't guarantee that we get the cache we wanted - // to, we need to check if this cache is really the last - // consumer of the cache we want to merge it with. - merge = true; - break; - } + bool merge = acquire_unreferenced_cache_pseudo_ref(consumer); + if (merge) { + // We managed to increment the reference count, but that's not a + // full reference. We get a real one now and decrement the ref count + // again. + vm_cache_acquire_ref(consumer); + atomic_add(&consumer->ref_count, -1); } + // In case we managed to grab a reference to the consumerRef, + // this doesn't guarantee that we get the cache we wanted + // to, so we need to check if this cache is really the last + // consumer of the cache we want to merge it with. + ConditionVariable busyCondition; if (merge) { diff --git a/src/system/kernel/vm/vm_daemons.cpp b/src/system/kernel/vm/vm_daemons.cpp index 1be6b768b1..6d485528e3 100644 --- a/src/system/kernel/vm/vm_daemons.cpp +++ b/src/system/kernel/vm/vm_daemons.cpp @@ -70,21 +70,8 @@ PageCacheLocker::Lock(vm_page* page) if (_IgnorePage(page)) return false; - vm_cache* cache = page->cache; - - // Grab a reference to this cache - the page does not own a reference - // to its cache, so we can't just acquire it the easy way - while (true) { - int32 count = cache->ref_count; - if (count == 0) { - cache = NULL; - break; - } - - if (atomic_test_and_set(&cache->ref_count, count + 1, count) == count) - break; - } - + // Grab a reference to this cache. + vm_cache* cache = vm_cache_acquire_page_cache_ref(page); if (cache == NULL) return false;