From 772e46f8784ceb3539a8d8173c95ea7c61c467e3 Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Wed, 16 Mar 2022 09:57:14 +0000 Subject: [PATCH] Additional commentary and more verbose names --- src/backend/backend.h | 3 ++- src/backend/largebuddyrange.h | 32 +++++++++++++++++++++++++++++-- src/backend/metatypes.h | 36 +++++++++++++++++------------------ 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/backend/backend.h b/src/backend/backend.h index 528abb5..e81db08 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -217,7 +217,8 @@ namespace snmalloc * * The template argument is the type of the metadata being allocated. This * allows the backend to allocate different types of metadata in different - * places or with different policies. + * places or with different policies. The default implementation, here, + * does not avail itself of this degree of freedom. */ template static capptr::Chunk diff --git a/src/backend/largebuddyrange.h b/src/backend/largebuddyrange.h index defcc56..5b2954d 100644 --- a/src/backend/largebuddyrange.h +++ b/src/backend/largebuddyrange.h @@ -18,17 +18,45 @@ namespace snmalloc class BuddyChunkRep { public: + /* + * The values we store in our rbtree are the addresses of (combined spans + * of) chunks of the address space; as such, bits in (MIN_CHUNK_SIZE - 1) + * are unused and so the RED_BIT is packed therein. However, in practice, + * these are not "just any" uintptr_t-s, but specifically the uintptr_t-s + * inside the Pagemap's MetaEntry structures. As such, there are some + * additional bit-swizzling concerns; see set() and get() below. + */ using Holder = uintptr_t; using Contents = uintptr_t; - static constexpr address_t RED_BIT = 2; + static constexpr address_t RED_BIT = 1 << 1; + + static_assert(RED_BIT < MIN_CHUNK_SIZE); + static_assert(RED_BIT != MetaEntry::META_BOUNDARY_BIT); + static_assert(RED_BIT != MetaEntry::REMOTE_BACKEND_MARKER); static constexpr Contents null = 0; static void set(Holder* ptr, Contents r) { SNMALLOC_ASSERT((r & (MIN_CHUNK_SIZE - 1)) == 0); - // Preserve lower bits. + /* + * Preserve lower bits, claim as backend, and update contents of holder. + * + * This is excessive at present but no harder than being more precise + * while also being future-proof. All that is strictly required would be + * to preserve META_BOUNDARY_BIT and RED_BIT in ->meta and to assert + * REMOTE_BACKEND_MARKER in ->remote_and_sizeclass (if it isn't already + * asserted). However, we don't know which Holder* we have been given, + * nor do we know whether this Holder* is completely new (and so we are + * the first reasonable opportunity to assert REMOTE_BACKEND_MARKER) or + * recycled from the frontend, and so we preserve and assert more than + * strictly necessary. + * + * The use of `address_cast` below is a CHERI-ism; otherwise both `r` and + * `*ptr & ...` are plausibly provenance-carrying values and the compiler + * balks at the ambiguity. + */ *ptr = r | address_cast(*ptr & (MIN_CHUNK_SIZE - 1)) | MetaEntry::REMOTE_BACKEND_MARKER; } diff --git a/src/backend/metatypes.h b/src/backend/metatypes.h index b3c2473..c65efaa 100644 --- a/src/backend/metatypes.h +++ b/src/backend/metatypes.h @@ -84,7 +84,8 @@ namespace snmalloc class sizeclass_t; /** - * Entry stored in the pagemap. + * Entry stored in the pagemap. See docs/AddressSpace.md for the full + * MetaEntry lifecycle. */ class MetaEntry { @@ -92,9 +93,11 @@ namespace snmalloc friend class BuddyChunkRep; /** - * The pointer to the metaslab, the bottom bit is used to indicate if this - * is the first chunk in a PAL allocation, that cannot be combined with - * the preceeding chunk. + * In common cases, the pointer to the metaslab. See docs/AddressSpace.md + * for additional details. + * + * The bottom bit is used to indicate if this is the first chunk in a PAL + * allocation, that cannot be combined with the preceeding chunk. */ uintptr_t meta{0}; @@ -107,18 +110,12 @@ namespace snmalloc * representable. It is also true on Windows as you cannot Commit across * multiple continuous VirtualAllocs. */ - static constexpr address_t BOUNDARY_BIT = 1; + static constexpr address_t META_BOUNDARY_BIT = 1 << 0; /** - * A bit-packed pointer to the owning allocator (if any), and the sizeclass - * of this chunk. The sizeclass here is itself a union between two cases: - * - * * log_2(size), at least MIN_CHUNK_BITS, for large allocations. - * - * * a value in [0, NUM_SMALL_SIZECLASSES] for small allocations. These - * may be directly passed to the sizeclass (not slab_sizeclass) functions - * of sizeclasstable.h - * + * In common cases, a bit-packed pointer to the owning allocator (if any), + * and the sizeclass of this chunk. See mem/metaslab.h:MetaEntryRemote for + * details of this case and docs/AddressSpace.md for further details. */ uintptr_t remote_and_sizeclass{0}; @@ -160,7 +157,7 @@ namespace snmalloc [[nodiscard]] SNMALLOC_FAST_PATH Metaslab* get_metaslab() const { SNMALLOC_ASSERT(get_remote() != nullptr); - return unsafe_from_uintptr(meta & ~BOUNDARY_BIT); + return unsafe_from_uintptr(meta & ~META_BOUNDARY_BIT); } /** @@ -183,24 +180,25 @@ namespace snmalloc MetaEntry& operator=(const MetaEntry& other) { // Don't overwrite the boundary bit with the other's - meta = (other.meta & ~BOUNDARY_BIT) | address_cast(meta & BOUNDARY_BIT); + meta = (other.meta & ~META_BOUNDARY_BIT) | + address_cast(meta & META_BOUNDARY_BIT); remote_and_sizeclass = other.remote_and_sizeclass; return *this; } void set_boundary() { - meta |= BOUNDARY_BIT; + meta |= META_BOUNDARY_BIT; } [[nodiscard]] bool is_boundary() const { - return meta & BOUNDARY_BIT; + return meta & META_BOUNDARY_BIT; } bool clear_boundary_bit() { - return meta &= ~BOUNDARY_BIT; + return meta &= ~META_BOUNDARY_BIT; } };