* Fixed several occasions of bugs with respect to the handling of

overcommitting stores:
  - has_precommitted was incorrectly set to true in the constructor
  - when a precommitted page was committed, vm_store::committed_size
    was still changed.
  - unreserving memory did not update vm_store::committed_size.
  - when precommitted pages were committed, their page count instead of their
    size was reserved.
* All this lead to bug #1970 which should be fixed now.
* Cleanup of vm_cache.cpp, no functional change.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24742 a95241bf-73f2-0310-859d-f6bbb57e9c96
This commit is contained in:
Axel Dörfler
2008-04-02 12:19:28 +00:00
parent ace2d5ee37
commit 62f892990b
2 changed files with 67 additions and 72 deletions
+15 -20
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2007, Axel Dörfler, [email protected].
* Copyright 2002-2008, Axel Dörfler, [email protected].
* Distributed under the terms of the MIT License.
*
* Copyright 2001-2002, Travis Geiselbrecht. All rights reserved.
@@ -382,8 +382,7 @@ vm_cache_insert_page(vm_cache *cache, vm_page *page, off_t offset)
}
/*!
Removes the vm_page from this cache. Of course, the page must
/*! Removes the vm_page from this cache. Of course, the page must
really be in this cache or evil things will happen.
The cache lock must be held.
*/
@@ -437,8 +436,7 @@ vm_cache_write_modified(vm_cache *cache, bool fsReenter)
}
/*!
Commits the memory to the store if the \a commitment is larger than
/*! Commits the memory to the store if the \a commitment is larger than
what's committed already.
Assumes you have the \a ref's lock held.
*/
@@ -452,7 +450,8 @@ vm_cache_set_minimal_commitment_locked(vm_cache *cache, off_t commitment)
vm_store* store = cache->store;
status_t status = B_OK;
// If we don't have enough committed space to cover through to the new end of region...
// If we don't have enough committed space to cover through to the new end
// of the area...
if (store->committed_size < commitment) {
// ToDo: should we check if the cache's virtual size is large
// enough for a commitment of that size?
@@ -465,8 +464,7 @@ vm_cache_set_minimal_commitment_locked(vm_cache *cache, off_t commitment)
}
/*!
This function updates the size field of the vm_cache structure.
/*! This function updates the size field of the vm_cache structure.
If needed, it will free up all pages that don't belong to the cache anymore.
The cache lock must be held when you call it.
Since removed pages don't belong to the cache any longer, they are not
@@ -493,7 +491,8 @@ 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, *next;
vm_page* page = cache->page_list;
vm_page* next;
while (page != NULL) {
next = page->cache_next;
@@ -534,8 +533,7 @@ vm_cache_resize(vm_cache *cache, off_t newSize)
}
/*!
Removes the \a consumer from the \a cache.
/*! Removes the \a consumer from the \a cache.
It will also release the reference to the cacheRef owned by the consumer.
Assumes you have the consumer's cache lock held.
*/
@@ -598,15 +596,14 @@ vm_cache_remove_consumer(vm_cache *cache, vm_cache *consumer)
}
if (merge) {
vm_page *page, *nextPage;
vm_cache *newSource;
consumer = (vm_cache*)list_remove_head_item(&cache->consumers);
TRACE(("merge vm cache %p (ref == %ld) with vm cache %p\n",
cache, cache->ref_count, consumer));
for (page = cache->page_list; page != NULL; page = nextPage) {
vm_page* nextPage;
for (vm_page* page = cache->page_list; page != NULL;
page = nextPage) {
vm_page* consumerPage;
nextPage = page->cache_next;
@@ -644,7 +641,7 @@ vm_cache_remove_consumer(vm_cache *cache, vm_cache *consumer)
}
}
newSource = cache->source;
vm_cache* newSource = cache->source;
// The remaining consumer has gotten a new source
mutex_lock(&newSource->lock);
@@ -677,8 +674,7 @@ panic("cacheRef %p ref count too low!\n", cache);
}
/*!
Marks the \a cache as source of the \a consumer cache,
/*! Marks the \a cache as source of the \a consumer cache,
and adds the \a consumer to its list.
This also grabs a reference to the source cache.
Assumes you have the cache and the consumer's lock held.
@@ -700,8 +696,7 @@ vm_cache_add_consumer_locked(vm_cache *cache, vm_cache *consumer)
}
/*!
Adds the \a area to the \a cache.
/*! Adds the \a area to the \a cache.
Assumes you have the locked the cache.
*/
status_t
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2007, Axel Dörfler, [email protected].
* Copyright 2002-2008, Axel Dörfler, [email protected].
* Distributed under the terms of the MIT License.
*
* Copyright 2001-2002, Travis Geiselbrecht. All rights reserved.
@@ -50,6 +50,9 @@ anonymous_commit(struct vm_store *_store, off_t size)
{
anonymous_store *store = (anonymous_store *)_store;
size -= store->vm.cache->virtual_base;
// anonymous stores don't need to span over their whole source
// if we can overcommit, we don't commit here, but in anonymous_fault()
if (store->can_overcommit) {
if (store->has_precommitted)
@@ -57,26 +60,23 @@ anonymous_commit(struct vm_store *_store, off_t size)
// pre-commit some pages to make a later failure less probable
store->has_precommitted = true;
if (size > store->vm.cache->virtual_base + store->precommitted_pages)
size = store->vm.cache->virtual_base + store->precommitted_pages;
uint32 precommitted = store->precommitted_pages * B_PAGE_SIZE;
if (size > precommitted)
size = precommitted;
}
size -= store->vm.cache->virtual_base;
// anonymous stores don't need to span over their whole source
// Check to see how much we could commit - we need real memory
if (size > store->vm.committed_size) {
// try to commit
if (vm_try_reserve_memory(size - store->vm.committed_size) != B_OK)
return B_NO_MEMORY;
store->vm.committed_size = size;
} else {
// we can release some
vm_unreserve_memory(store->vm.committed_size - size);
}
store->vm.committed_size = size;
return B_OK;
}
@@ -136,10 +136,10 @@ anonymous_fault(struct vm_store *_store, struct vm_address_space *aspace,
// try to commit additional memory
if (vm_try_reserve_memory(B_PAGE_SIZE) != B_OK)
return B_NO_MEMORY;
} else
store->precommitted_pages--;
store->vm.committed_size += B_PAGE_SIZE;
} else
store->precommitted_pages--;
}
// This will cause vm_soft_fault() to handle the fault
@@ -177,7 +177,7 @@ vm_store_create_anonymous_noswap(bool canOvercommit,
store->vm.cache = NULL;
store->vm.committed_size = 0;
store->can_overcommit = canOvercommit;
store->has_precommitted = numPrecommittedPages != 0;
store->has_precommitted = false;
store->precommitted_pages = min_c(numPrecommittedPages, 255);
store->guarded_size = numGuardPages * B_PAGE_SIZE;