kernel/vm: More fixes to commitment handling in cut_area.

* Fix a silly bug in compute_area_page_commitment that was leading to
   the cache's pages not being taken into account at all.

 * Don't let VMCache::Resize() and Rebase() alter the commitments,
   but rather let us do that. Add assertions that they did not fail.

 * Move area->cache_offset increment up so that compute_area_page_commitment
   can use it as it needs to.

Fixes assertion failures from boehm-gc tests following the previous commits.
This commit is contained in:
Augustin Cavalier
2024-12-13 16:57:11 -05:00
parent 97df206a85
commit 9774380755
+20 -13
View File
@@ -602,7 +602,7 @@ compute_area_page_commitment(VMArea* area)
size_t pages = 0; size_t pages = 0;
for (size_t i = 0; i < bytes; i++) { for (size_t i = 0; i < bytes; i++) {
const uint8 protection = area->page_protections[i]; const uint8 protection = area->page_protections[i];
const off_t pageOffset = bytes * 2 * B_PAGE_SIZE; const off_t pageOffset = area->cache_offset + (i * 2 * B_PAGE_SIZE);
if (area->cache->LookupPage(pageOffset) != NULL) if (area->cache->LookupPage(pageOffset) != NULL)
pages++; pages++;
else else
@@ -767,6 +767,12 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
allocationFlags = 0; allocationFlags = 0;
} }
int resizePriority = priority;
if (area->page_protections != NULL) {
// We'll adjust commitments directly, rather than letting VMCache do it.
resizePriority = -1;
}
VMCache* cache = vm_area_get_locked_cache(area); VMCache* cache = vm_area_get_locked_cache(area);
VMCacheChainLocker cacheChainLocker(cache); VMCacheChainLocker cacheChainLocker(cache);
cacheChainLocker.LockAllSourceCaches(); cacheChainLocker.LockAllSourceCaches();
@@ -779,7 +785,7 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
const addr_t oldSize = area->Size(); const addr_t oldSize = area->Size();
// Cut the end only? // Cut the end only?
if (offset > 0 && size == area->Size() - offset) { if (offset > 0 && size == (area->Size() - offset)) {
status_t error = addressSpace->ShrinkAreaTail(area, offset, status_t error = addressSpace->ShrinkAreaTail(area, offset,
allocationFlags); allocationFlags);
if (error != B_OK) if (error != B_OK)
@@ -804,13 +810,13 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
// Since VMCache::Resize() can temporarily drop the lock, we must // Since VMCache::Resize() can temporarily drop the lock, we must
// unlock all lower caches to prevent locking order inversion. // unlock all lower caches to prevent locking order inversion.
cacheChainLocker.Unlock(cache); cacheChainLocker.Unlock(cache);
cache->Resize(cache->virtual_base + offset, priority); status_t status = cache->Resize(cache->virtual_base + offset, resizePriority);
ASSERT_ALWAYS(status == B_OK);
} }
if (area->page_protections != NULL) { if (area->page_protections != NULL) {
// Resize() adjusts the commitment, so we must do this after that.
const size_t newCommitmentPages = compute_area_page_commitment(area); const size_t newCommitmentPages = compute_area_page_commitment(area);
cache->Commit(newCommitmentPages * B_PAGE_SIZE, VM_PRIORITY_USER); cache->Commit(newCommitmentPages * B_PAGE_SIZE, priority);
} }
if (onlyCacheUser) if (onlyCacheUser)
@@ -856,19 +862,19 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
// Since VMCache::Rebase() can temporarily drop the lock, we must // Since VMCache::Rebase() can temporarily drop the lock, we must
// unlock all lower caches to prevent locking order inversion. // unlock all lower caches to prevent locking order inversion.
cacheChainLocker.Unlock(cache); cacheChainLocker.Unlock(cache);
cache->Rebase(cache->virtual_base + size, priority); status_t status = cache->Rebase(cache->virtual_base + size, resizePriority);
ASSERT_ALWAYS(status == B_OK);
} }
area->cache_offset += size;
if (area->page_protections != NULL) { if (area->page_protections != NULL) {
// Rebase() adjusts the commitment, so we must do this after that.
const size_t newCommitmentPages = compute_area_page_commitment(area); const size_t newCommitmentPages = compute_area_page_commitment(area);
cache->Commit(newCommitmentPages * B_PAGE_SIZE, VM_PRIORITY_USER); cache->Commit(newCommitmentPages * B_PAGE_SIZE, priority);
} }
if (onlyCacheUser) if (onlyCacheUser)
cache->ReleaseRefAndUnlock(); cache->ReleaseRefAndUnlock();
area->cache_offset += size;
return B_OK; return B_OK;
} }
@@ -942,7 +948,8 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
// unlock all lower caches to prevent locking order inversion. // unlock all lower caches to prevent locking order inversion.
cacheChainLocker.Unlock(cache); cacheChainLocker.Unlock(cache);
areaCacheLocker.SetTo(cache, true); areaCacheLocker.SetTo(cache, true);
cache->Resize(cache->virtual_base + firstNewSize, priority); error = cache->Resize(cache->virtual_base + firstNewSize, resizePriority);
ASSERT_ALWAYS(error == B_OK);
// Don't unlock the cache yet because we might have to resize it back. // Don't unlock the cache yet because we might have to resize it back.
// (Or we might have to modify its commitment, if we have page_protections.) // (Or we might have to modify its commitment, if we have page_protections.)
@@ -955,7 +962,7 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
if (error != B_OK) { if (error != B_OK) {
// Restore the original cache. // Restore the original cache.
cache->Resize(cache->virtual_base + oldSize, priority); cache->Resize(cache->virtual_base + oldSize, resizePriority);
// Move the pages back. // Move the pages back.
status_t readoptStatus = cache->Adopt(secondCache, status_t readoptStatus = cache->Adopt(secondCache,
@@ -1021,10 +1028,10 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
// Shrink commitments. // Shrink commitments.
const size_t areaCommitPages = compute_area_page_commitment(area); const size_t areaCommitPages = compute_area_page_commitment(area);
area->cache->Commit(areaCommitPages * B_PAGE_SIZE, VM_PRIORITY_USER); area->cache->Commit(areaCommitPages * B_PAGE_SIZE, priority);
const size_t secondCommitPages = compute_area_page_commitment(secondArea); const size_t secondCommitPages = compute_area_page_commitment(secondArea);
secondArea->cache->Commit(secondCommitPages * B_PAGE_SIZE, VM_PRIORITY_USER); secondArea->cache->Commit(secondCommitPages * B_PAGE_SIZE, priority);
// Set the correct page protections for the second area. // Set the correct page protections for the second area.
VMTranslationMap* map = addressSpace->TranslationMap(); VMTranslationMap* map = addressSpace->TranslationMap();