From 62ad70b17edfb05b22abe906ff199127f7444e65 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 18 Jan 2019 14:30:23 +0000 Subject: [PATCH 1/3] Fixes to remote deallocation The encoding in the top bits for the size class did not respect kernel pointers. Using an intptr_t means, we can use a signed shift to maintain the kernel pointers. --- CMakeLists.txt | 2 +- src/mem/remoteallocator.h | 33 +++++++++++++++++---------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b2c6ed2d7..e3842eeab 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -37,7 +37,7 @@ if(MSVC) set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG") else() find_package(Threads REQUIRED) - add_compile_options(-mcx16 -march=native -Wall -Wextra -Werror -g) + add_compile_options(-mcx16 -march=native -Wall -Wextra -Werror -Wsign-conversion -g) endif() set(CMAKE_CXX_STANDARD 17) diff --git a/src/mem/remoteallocator.h b/src/mem/remoteallocator.h index 7e9b30ac1..cb295bbb8 100644 --- a/src/mem/remoteallocator.h +++ b/src/mem/remoteallocator.h @@ -2,6 +2,7 @@ #include "../ds/mpscq.h" #include "../mem/allocconfig.h" +#include "../mem/sizeclass.h" #include @@ -10,13 +11,11 @@ namespace snmalloc struct Remote { static const size_t PTR_BITS = sizeof(void*) * 8; - static const size_t SIZECLASS_BITS = sizeof(uint8_t) * 8; + static const size_t SIZECLASS_BITS = + bits::next_pow2_bits_const(NUM_SIZECLASSES); static const bool USE_TOP_BITS = - SIZECLASS_BITS + bits::ADDRESS_BITS <= PTR_BITS; - static const uintptr_t SIZECLASS_SHIFT = PTR_BITS - SIZECLASS_BITS; - static const uintptr_t SIZECLASS_MASK = ((1ULL << SIZECLASS_BITS) - 1) - << SIZECLASS_SHIFT; - static const uintptr_t TARGET_MASK = ~SIZECLASS_MASK; + SIZECLASS_BITS + bits::ADDRESS_BITS < PTR_BITS; + static const uintptr_t SIZECLASS_MASK = ((1ULL << SIZECLASS_BITS) - 1); using alloc_id_t = size_t; union @@ -25,7 +24,11 @@ namespace snmalloc Remote* non_atomic_next; }; - uintptr_t value; + // Uses an intptr_t so that when we use the TOP_BITS to encode the + // sizeclass, then we can use signed shift to correctly handle kernel versus + // user mode. + intptr_t value; + // This will not exist for the minimum object size. This is only used if // USE_TOP_BITS is false, and the bottom bit of value is set. uint8_t possible_sizeclass; @@ -34,21 +37,19 @@ namespace snmalloc { if constexpr (USE_TOP_BITS) { - assert(id == (id & TARGET_MASK)); - value = (id & TARGET_MASK) | - ((static_cast(sizeclass) << SIZECLASS_SHIFT) & - SIZECLASS_MASK); + value = (intptr_t)( + (id << SIZECLASS_BITS) | ((static_cast(sizeclass)))); } else { assert((id & 1) == 0); if (sizeclass == 0) { - value = id | 1; + value = (intptr_t)(id | 1); } else { - value = id; + value = (intptr_t)id; possible_sizeclass = sizeclass; } } @@ -58,11 +59,11 @@ namespace snmalloc { if constexpr (USE_TOP_BITS) { - return value & TARGET_MASK; + return (alloc_id_t)(value >> SIZECLASS_BITS); } else { - return value & ~1; + return (alloc_id_t)(value & ~1); } } @@ -70,7 +71,7 @@ namespace snmalloc { if constexpr (USE_TOP_BITS) { - return (value & SIZECLASS_MASK) >> SIZECLASS_SHIFT; + return (static_cast((uintptr_t)value & SIZECLASS_MASK)); } else { From a62e77f930c063a407cfff202bdbdd91e601bbb2 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 18 Jan 2019 15:22:08 +0000 Subject: [PATCH 2/3] Align pagemap entries to OS_PAGE_SIZE Guarantee the page map is page aligned. Fix public API. --- src/mem/largealloc.h | 25 ++++++++++++++++++++----- src/mem/pagemap.h | 17 +++++++++-------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index 96198618a..d9a80cf6d 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -51,6 +51,13 @@ namespace snmalloc return std::make_pair(r, size); } + void new_block() + { + auto r_size = reserve_block(); + bump = (size_t)r_size.first; + remaining = r_size.second; + } + public: /** * Stack of large allocations that have been returned for reuse. @@ -61,7 +68,7 @@ namespace snmalloc * Primitive allocator for structure that are required before * the allocator can be running. ***/ - void* alloc_chunk(size_t size) + void* alloc_chunk(size_t size, size_t alignment = 64) { // Cache line align size = bits::align_up(size, 64); @@ -70,12 +77,20 @@ namespace snmalloc { FlagLock f(lock); - if (remaining < size) + auto aligned_bump = bits::align_up(bump, alignment); + if ((aligned_bump - bump) < size) + { + new_block(); + } + else { - auto r_size = reserve_block(); + remaining -= aligned_bump - bump; + bump = aligned_bump; + } - bump = (size_t)r_size.first; - remaining = r_size.second; + if (remaining < size) + { + new_block(); } p = (void*)bump; diff --git a/src/mem/pagemap.h b/src/mem/pagemap.h index b47bd93ba..1f4595bde 100644 --- a/src/mem/pagemap.h +++ b/src/mem/pagemap.h @@ -86,7 +86,8 @@ namespace snmalloc value, (PagemapEntry*)LOCKED_ENTRY, std::memory_order_relaxed)) { auto& v = default_memory_provider; - value = (PagemapEntry*)v.alloc_chunk(PAGEMAP_NODE_SIZE); + value = + (PagemapEntry*)v.alloc_chunk(PAGEMAP_NODE_SIZE, OS_PAGE_SIZE); e->store(value, std::memory_order_release); } else @@ -160,6 +161,13 @@ namespace snmalloc return &(leaf_ix.first->values[leaf_ix.second]); } + std::atomic* get_ptr(void* p) + { + bool success; + return get_addr(p, success); + } + + public: /** * Returns the index of a pagemap entry within a given page. This is used * in code that propagates changes to the pagemap elsewhere. @@ -182,13 +190,6 @@ namespace snmalloc reinterpret_cast(get_addr(p, success))); } - std::atomic* get_ptr(void* p) - { - bool success; - return get_addr(p, success); - } - - public: T get(void* p) { bool success; From c1e23c497ffb9b825e5269259705cb0eba8b8bd6 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 18 Jan 2019 17:00:10 +0000 Subject: [PATCH 3/3] Made alignment a template parameter. --- src/mem/largealloc.h | 3 ++- src/mem/pagemap.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index d9a80cf6d..4745471ef 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -68,7 +68,8 @@ namespace snmalloc * Primitive allocator for structure that are required before * the allocator can be running. ***/ - void* alloc_chunk(size_t size, size_t alignment = 64) + template + void* alloc_chunk(size_t size) { // Cache line align size = bits::align_up(size, 64); diff --git a/src/mem/pagemap.h b/src/mem/pagemap.h index 1f4595bde..ad0d91a46 100644 --- a/src/mem/pagemap.h +++ b/src/mem/pagemap.h @@ -87,7 +87,7 @@ namespace snmalloc { auto& v = default_memory_provider; value = - (PagemapEntry*)v.alloc_chunk(PAGEMAP_NODE_SIZE, OS_PAGE_SIZE); + (PagemapEntry*)v.alloc_chunk(PAGEMAP_NODE_SIZE); e->store(value, std::memory_order_release); } else