⛏️ index : haiku.git

21e1997980e9daee379491ca85fce3d50a7af21d [patch]

2d02237ea1e09505c3af36cfbb89face5da7f1b0

ce88914d782a4f025ac8567f9d6e5935d09b6ed8

21e1997980e9daee379491ca85fce3d50a7af21d.tar.gz

kernel/vm: Previously-shared caches may have a different base or size.

Than the one used by the area, anyway, so we need to call Rebase
as well as Resize in all cases where the cache is getting reused.

This case happens (rarely) with the bdwgc's gctest, with mprotect VBD
enabled (which still doesn't work reliably, it sometimes crashes userland
or trips asserts in the kernel, but it seems to trip less after this
change.)

Also, let the CacheChainLocker take care of releasing and unlocking
the cache even in the onlyCacheUser case.

Diff

 src/system/kernel/vm/vm.cpp | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp
index 5fed564..352faff 100644
--- a/src/system/kernel/vm/vm.cpp
+++ a/src/system/kernel/vm/vm.cpp
@@ -831,8 +831,14 @@
			// Since VMCache::Resize() can temporarily drop the lock, we must
			// unlock all lower caches to prevent locking order inversion.
			cacheChainLocker.Unlock(cache);
			status_t status = cache->Resize(cache->virtual_base + offset, resizePriority);
			cacheChainLocker.SetTo(cache);

			// If the cache had other users before, it may have the wrong base.
			status_t status = cache->Rebase(area->cache_offset, resizePriority);
			ASSERT_ALWAYS(status == B_OK);

			status = cache->Resize(area->cache_offset + area->Size(), resizePriority);
			ASSERT_ALWAYS(status == B_OK);
		}

		if (resizePriority == -1) {
@@ -840,8 +846,6 @@
			cache->Commit(newCommitmentPages * B_PAGE_SIZE, priority);
		}

		if (onlyCacheUser)
			cache->ReleaseRefAndUnlock();
		return B_OK;
	}

@@ -883,7 +887,13 @@
			// Since VMCache::Rebase() can temporarily drop the lock, we must
			// unlock all lower caches to prevent locking order inversion.
			cacheChainLocker.Unlock(cache);
			status_t status = cache->Rebase(cache->virtual_base + size, resizePriority);
			cacheChainLocker.SetTo(cache);

			status_t status = cache->Rebase(area->cache_offset + size, resizePriority);
			ASSERT_ALWAYS(status == B_OK);

			// If the cache had other users before, it may be larger than wanted.
			status = cache->Resize(cache->virtual_base + area->Size(), resizePriority);
			ASSERT_ALWAYS(status == B_OK);
		}

@@ -893,9 +903,6 @@
			cache->Commit(newCommitmentPages * B_PAGE_SIZE, priority);
		}

		if (onlyCacheUser)
			cache->ReleaseRefAndUnlock();

		return B_OK;
	}

@@ -1017,6 +1024,10 @@
			free_etc(secondAreaNewProtections, allocationFlags);
			return error;
		}

		// If the cache had other users before, it may have the wrong base.
		error = cache->Rebase(area->cache_offset, resizePriority);
		ASSERT_ALWAYS(error == B_OK);

		error = cache->Resize(cache->virtual_base + firstNewSize, resizePriority);
		ASSERT_ALWAYS(error == B_OK);