[prev in list] [next in list] [prev in thread] [next in thread]
List: haiku-commits
Subject: [haiku-commits] haiku: hrev57058 - src/system/kernel/vm headers/private/kernel/util src/system/kerne
From: waddlesplash <waddlesplash () gmail ! com>
Date: 2023-05-31 18:48:43
Message-ID: 20230531184843.6F5B03F7AC () turing ! freelists ! org
[Download RAW message or body]
hrev57058 adds 1 changeset to branch 'master'
old head: fbcc7b2711dcb3ed611911826e5317da3dd510d5
new head: cb1df90e30197426c6c64685c5ceebf15c6742f5
overview: https://git.haiku-os.org/haiku/log/?qt=range&q=cb1df90e3019+%5Efbcc7b2711dc
----------------------------------------------------------------------------
cb1df90e3019: Revert "kernel/vm: handle page protections in cut_area"
This reverts commit de07bc3fa58600e923d75cb732c60dacf160c098.
That's what I get for fixing bugs on test branches.
[ Augustin Cavalier <waddlesplash@gmail.com> ]
----------------------------------------------------------------------------
Revision: hrev57058
Commit: cb1df90e30197426c6c64685c5ceebf15c6742f5
URL: https://git.haiku-os.org/haiku/commit/?id=cb1df90e3019
Author: Augustin Cavalier <waddlesplash@gmail.com>
Date: Wed May 31 18:48:23 2023 UTC
----------------------------------------------------------------------------
4 files changed, 45 insertions(+), 187 deletions(-)
headers/private/kernel/util/Bitmap.h | 57 -------------
src/system/kernel/util/Bitmap.cpp | 44 +++++++++-
src/system/kernel/vm/vm.cpp | 110 +------------------------
src/tests/system/kernel/mmap_cut_tests.cpp | 21 +----
----------------------------------------------------------------------------
diff --git a/headers/private/kernel/util/Bitmap.h b/headers/private/kernel/util/Bitmap.h
index a103586c8a..e6f6d69627 100644
--- a/headers/private/kernel/util/Bitmap.h
+++ b/headers/private/kernel/util/Bitmap.h
@@ -16,8 +16,6 @@
# include <Debug.h>
#endif
-#include <string.h>
-
#include <SupportDefs.h>
namespace BKernel {
@@ -38,8 +36,6 @@ public:
ssize_t GetHighestSet() const;
- template<typename T>
- static void Shift(T* bits, size_t bitCount, ssize_t shift);
private:
size_t fElementsCount;
size_t fSize;
@@ -81,59 +77,6 @@ Bitmap::Clear(size_t index)
fBits[kArrayElement] &= ~addr_t(kBitMask);
}
-
-template<typename T>
-void
-Bitmap::Shift(T* bits, size_t bitCount, ssize_t shift)
-{
- if (shift == 0)
- return;
-
- const size_t bitsPerElement = sizeof(T) * 8;
- const size_t elementsCount = (bitCount + bitsPerElement - 1) / bitsPerElement;
- const size_t absoluteShift = (shift > 0) ? shift : -shift;
- const size_t nElements = absoluteShift / bitsPerElement;
- const size_t nBits = absoluteShift % bitsPerElement;
- if (nElements != 0) {
- if (shift > 0) {
- // "Left" shift.
- memmove(&bits[nElements], bits, sizeof(T) * (elementsCount - nElements));
- memset(bits, 0, sizeof(T) * nElements);
- } else if (shift < 0) {
- // "Right" shift.
- memmove(bits, &bits[nElements], sizeof(T) * (elementsCount - nElements));
- memset(&bits[elementsCount - nElements], 0, sizeof(T) * nElements);
- }
- }
-
- // If the shift was by a multiple of the element size, nothing more to do.
- if (nBits == 0)
- return;
-
- // One set of bits comes from the "current" element and are shifted in the
- // direction of the shift; the other set comes from the next-processed
- // element and are shifted in the opposite direction.
- if (shift > 0) {
- // "Left" shift.
- for (ssize_t i = elementsCount - 1; i >= 0; i--) {
- T low = 0;
- if (i != 0)
- low = bits[i - 1] >> (bitsPerElement - nBits);
- const T high = bits[i] << nBits;
- bits[i] = low | high;
- }
- } else if (shift < 0) {
- // "Right" shift.
- for (size_t i = 0; i < elementsCount; i++) {
- const T low = bits[i] >> nBits;
- T high = 0;
- if (i != (elementsCount - 1))
- high = bits[i + 1] << (bitsPerElement - nBits);
- bits[i] = low | high;
- }
- }
-}
-
} // namespace BKernel
diff --git a/src/system/kernel/util/Bitmap.cpp b/src/system/kernel/util/Bitmap.cpp
index a62b4bc01a..af42ec4e2b 100644
--- a/src/system/kernel/util/Bitmap.cpp
+++ b/src/system/kernel/util/Bitmap.cpp
@@ -66,7 +66,49 @@ Bitmap::Resize(size_t bitCount)
void
Bitmap::Shift(ssize_t bitCount)
{
- return Bitmap::Shift<addr_t>(fBits, fSize, bitCount);
+ if (bitCount == 0)
+ return;
+
+ const size_t shift = (bitCount > 0) ? bitCount : -bitCount;
+ const size_t nElements = shift / kBitsPerElement, nBits = shift % kBitsPerElement;
+ if (nElements != 0) {
+ if (bitCount > 0) {
+ // "Left" shift.
+ memmove(&fBits[nElements], fBits, sizeof(addr_t) * (fElementsCount - nElements));
+ memset(fBits, 0, sizeof(addr_t) * nElements);
+ } else if (bitCount < 0) {
+ // "Right" shift.
+ memmove(fBits, &fBits[nElements], sizeof(addr_t) * (fElementsCount - nElements));
+ memset(&fBits[fElementsCount - nElements], 0, sizeof(addr_t) * nElements);
+ }
+ }
+
+ // If the shift was by a multiple of the element size, nothing more to do.
+ if (nBits == 0)
+ return;
+
+ // One set of bits comes from the "current" element and are shifted in the
+ // direction of the shift; the other set comes from the next-processed
+ // element and are shifted in the opposite direction.
+ if (bitCount > 0) {
+ // "Left" shift.
+ for (ssize_t i = fElementsCount - 1; i >= 0; i--) {
+ addr_t low = 0;
+ if (i != 0)
+ low = fBits[i - 1] >> (kBitsPerElement - nBits);
+ const addr_t high = fBits[i] << nBits;
+ fBits[i] = low | high;
+ }
+ } else if (bitCount < 0) {
+ // "Right" shift.
+ for (size_t i = 0; i < fElementsCount; i++) {
+ const addr_t low = fBits[i] >> nBits;
+ addr_t high = 0;
+ if (i != (fElementsCount - 1))
+ high = fBits[i + 1] << (kBitsPerElement - nBits);
+ fBits[i] = low | high;
+ }
+ }
}
diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp
index 4d476e6f04..292b3826ac 100644
--- a/src/system/kernel/vm/vm.cpp
+++ b/src/system/kernel/vm/vm.cpp
@@ -47,7 +47,6 @@
#include <team.h>
#include <tracing.h>
#include <util/AutoLock.h>
-#include <util/Bitmap.h>
#include <util/ThreadAutoLock.h>
#include <vm/vm_page.h>
#include <vm/vm_priv.h>
@@ -677,8 +676,6 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
bool onlyCacheUser = cache->areas == area && area->cache_next == NULL
&& cache->consumers.IsEmpty() && area->cache_type == CACHE_TYPE_RAM;
- const addr_t oldSize = area->Size();
-
// Cut the end only?
if (offset > 0 && size == area->Size() - offset) {
status_t error = addressSpace->ShrinkAreaTail(area, offset,
@@ -686,19 +683,6 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
if (error != B_OK)
return error;
- if (area->page_protections != NULL) {
- size_t bytes = (area->Size() / B_PAGE_SIZE + 1) / 2;
- uint8* newProtections
- = (uint8*)realloc(area->page_protections, bytes);
-
- if (newProtections == NULL) {
- addressSpace->ShrinkAreaTail(area, oldSize, allocationFlags);
- return B_NO_MEMORY;
- }
-
- area->page_protections = newProtections;
- }
-
// unmap pages
unmap_pages(area, address, size);
@@ -721,28 +705,6 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
if (error != B_OK)
return error;
- if (area->page_protections != NULL) {
- // Allocate all memory before shifting as the shift might lose some
- // bits.
- size_t bytes = (area->Size() / B_PAGE_SIZE + 1) / 2;
- uint8* newProtections
- = (uint8*)malloc_etc(bytes, allocationFlags);
-
- if (newProtections == NULL) {
- addressSpace->ShrinkAreaHead(area, oldSize, allocationFlags);
- return B_NO_MEMORY;
- }
-
- size_t oldBytes = (oldSize / B_PAGE_SIZE + 1) / 2;
- ssize_t pagesShifted = (oldSize - area->Size()) / B_PAGE_SIZE;
- Bitmap::Shift<uint8>(area->page_protections, oldBytes * 8,
- -(pagesShifted * 4));
-
- memcpy(newProtections, area->page_protections, bytes);
- free_etc(area->page_protections, allocationFlags);
- area->page_protections = newProtections;
- }
-
// unmap pages
unmap_pages(area, address, size);
@@ -769,32 +731,12 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
unmap_pages(area, address, area->Size() - firstNewSize);
// resize the area
+ addr_t oldSize = area->Size();
status_t error = addressSpace->ShrinkAreaTail(area, firstNewSize,
allocationFlags);
if (error != B_OK)
return error;
- uint8* areaNewProtections = NULL;
- uint8* secondAreaNewProtections = NULL;
-
- // Try to allocate the new memory before making some hard to reverse
- // changes.
- if (area->page_protections != NULL) {
- size_t areaBytes = (area->Size() / B_PAGE_SIZE + 1) / 2;
- size_t secondAreaBytes = (secondSize / B_PAGE_SIZE + 1) / 2;
-
- areaNewProtections = (uint8*)malloc_etc(areaBytes, allocationFlags);
- secondAreaNewProtections = (uint8*)malloc_etc(secondAreaBytes,
- allocationFlags);
-
- if (areaNewProtections == NULL || secondAreaNewProtections == NULL) {
- addressSpace->ShrinkAreaTail(area, oldSize, allocationFlags);
- free_etc(areaNewProtections, allocationFlags);
- free_etc(secondAreaNewProtections, allocationFlags);
- return B_NO_MEMORY;
- }
- }
-
virtual_address_restrictions addressRestrictions = {};
addressRestrictions.address = (void*)secondBase;
addressRestrictions.address_specification = B_EXACT_ADDRESS;
@@ -808,8 +750,6 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
dynamic_cast<VMAnonymousNoSwapCache*>(cache) == NULL, priority);
if (error != B_OK) {
addressSpace->ShrinkAreaTail(area, oldSize, allocationFlags);
- free_etc(areaNewProtections, allocationFlags);
- free_etc(secondAreaNewProtections, allocationFlags);
return error;
}
@@ -858,8 +798,6 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
cache->ReleaseRefAndUnlock();
secondCache->ReleaseRefAndUnlock();
addressSpace->ShrinkAreaTail(area, oldSize, allocationFlags);
- free_etc(areaNewProtections, allocationFlags);
- free_etc(secondAreaNewProtections, allocationFlags);
return error;
}
@@ -874,58 +812,12 @@ cut_area(VMAddressSpace* addressSpace, VMArea* area, addr_t address,
&addressRestrictions, kernel, &secondArea, NULL);
if (error != B_OK) {
addressSpace->ShrinkAreaTail(area, oldSize, allocationFlags);
- free_etc(areaNewProtections, allocationFlags);
- free_etc(secondAreaNewProtections, allocationFlags);
return error;
}
// We need a cache reference for the new area.
cache->AcquireRefLocked();
}
- if (area->page_protections != NULL) {
- // Copy the protection bits of the first area.
- size_t areaBytes = (area->Size() / B_PAGE_SIZE + 1) / 2;
- memcpy(areaNewProtections, area->page_protections, areaBytes);
- uint8* areaOldProtections = area->page_protections;
- area->page_protections = areaNewProtections;
-
- // Shift the protection bits of the second area to the start of
- // the old array.
- size_t oldBytes = (oldSize / B_PAGE_SIZE + 1) / 2;
- addr_t secondAreaOffset = secondBase - area->Base();
- ssize_t secondAreaPagesShifted = secondAreaOffset / B_PAGE_SIZE;
- Bitmap::Shift<uint8>(areaOldProtections, oldBytes * 8,
- -(secondAreaPagesShifted * 4));
-
- // Copy the protection bits of the second area.
- size_t secondAreaBytes = (secondSize / B_PAGE_SIZE + 1) / 2;
- memcpy(secondAreaNewProtections, areaOldProtections, secondAreaBytes);
- secondArea->page_protections = secondAreaNewProtections;
-
- // We don't need this anymore.
- free_etc(areaOldProtections, allocationFlags);
-
- // Set the correct page protections for the second area.
- VMTranslationMap* map = addressSpace->TranslationMap();
- map->Lock();
- page_num_t firstPageOffset
- = secondArea->cache_offset / B_PAGE_SIZE;
- page_num_t lastPageOffset
- = firstPageOffset + secondArea->Size() / B_PAGE_SIZE;
- for (VMCachePagesTree::Iterator it
- = secondArea->cache->pages.GetIterator();
- vm_page* page = it.Next();) {
- if (page->cache_offset >= firstPageOffset
- && page->cache_offset <= lastPageOffset) {
- addr_t address = virtual_page_address(secondArea, page);
- uint32 pageProtection
- = get_area_page_protection(secondArea, address);
- map->ProtectPage(secondArea, address, pageProtection);
- }
- }
- map->Unlock();
- }
-
if (_secondArea != NULL)
*_secondArea = secondArea;
diff --git a/src/tests/system/kernel/mmap_cut_tests.cpp b/src/tests/system/kernel/mmap_cut_tests.cpp
index 66a4490287..ce4da2fd4a 100644
--- a/src/tests/system/kernel/mmap_cut_tests.cpp
+++ b/src/tests/system/kernel/mmap_cut_tests.cpp
@@ -19,7 +19,7 @@ main()
// should fail (negative offset)
void* ptr0 = mmap(NULL, B_PAGE_SIZE, PROT_READ, MAP_PRIVATE, fd, -4096);
- if (ptr0 != MAP_FAILED) {
+ if (ptr0 != NULL) {
printf("map-negative-offset unexpectedly succeeded!\n");
return -1;
}
@@ -39,24 +39,5 @@ main()
return status;
}
- uint8* ptr3 = (uint8*)mmap(NULL, B_PAGE_SIZE * 4, PROT_NONE,
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-
- // make the tail accessible
- mprotect(ptr3 + B_PAGE_SIZE * 3, B_PAGE_SIZE, PROT_READ | PROT_WRITE);
-
- // store any value
- ptr3[B_PAGE_SIZE * 3] = 'a';
-
- // cut the area in the middle, before the accessible tail
- mmap(ptr3 + B_PAGE_SIZE, B_PAGE_SIZE, PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
-
- // validate that this does not crash
- if (ptr3[B_PAGE_SIZE * 3] != 'a') {
- printf("map-protect-cut test failed!\n");
- return -1;
- }
-
return 0;
}
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic