From 7940fee00cf0ce43c3c40584a3b3e4fb8d77cfd8 Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Wed, 16 Mar 2022 13:02:26 +0000 Subject: [PATCH] Refactor MetaEntry remote_and_sizeclass Introduce a class that we can use to more completely separate the frontend encoding details from the backend. --- src/backend/backend.h | 22 ++++++---- src/backend/backend_concept.h | 4 +- src/backend/chunkallocator.h | 9 ++-- src/backend/metatypes.h | 41 +++++++------------ src/mem/corealloc.h | 39 ++++++++++-------- src/mem/localalloc.h | 29 ++++++------- src/mem/metaslab.h | 77 +++++++++++++++++++++-------------- src/mem/remotecache.h | 12 +++--- 8 files changed, 124 insertions(+), 109 deletions(-) diff --git a/src/backend/backend.h b/src/backend/backend.h index e81db08..703d0d2 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -59,10 +59,14 @@ namespace snmalloc * Set template parameter to true if it not an error * to access a location that is not backed by a chunk. */ - template - SNMALLOC_FAST_PATH static const MetaEntry& get_metaentry(address_t p) + template + SNMALLOC_FAST_PATH static const Ret& get_metaentry(address_t p) { - return concretePagemap.template get(p); + static_assert( + std::is_base_of_v && sizeof(MetaEntry) == sizeof(Ret), + "Backend Pagemap get_metaentry return must look like MetaEntry"); + return static_cast( + concretePagemap.template get(p)); } /** @@ -250,15 +254,15 @@ namespace snmalloc * (remote, sizeclass, metaslab) * where metaslab, is the second element of the pair return. */ - static std::pair, Metaslab*> alloc_chunk( - LocalState& local_state, - size_t size, - RemoteAllocator* remote, - sizeclass_t sizeclass) + static std::pair, Metaslab*> + alloc_chunk(LocalState& local_state, size_t size, uintptr_t ras) { SNMALLOC_ASSERT(bits::is_pow2(size)); SNMALLOC_ASSERT(size >= MIN_CHUNK_SIZE); + SNMALLOC_ASSERT((ras & MetaEntry::REMOTE_BACKEND_MARKER) == 0); + ras &= ~MetaEntry::REMOTE_BACKEND_MARKER; + auto meta_cap = local_state.get_meta_range()->alloc_range(PAGEMAP_METADATA_STRUCT_SIZE); @@ -289,7 +293,7 @@ namespace snmalloc meta->meta_common.chunk = p; - MetaEntry t(meta, remote, sizeclass); + MetaEntry t(&meta->meta_common, ras); Pagemap::set_metaentry(address_cast(p), size, t); p = Aal::capptr_bound(p, size); diff --git a/src/backend/backend_concept.h b/src/backend/backend_concept.h index 895e19a..c0d8f8e 100644 --- a/src/backend/backend_concept.h +++ b/src/backend/backend_concept.h @@ -24,10 +24,10 @@ namespace snmalloc { { Meta::set_metaentry(addr, sz, t) } -> ConceptSame; - { Meta::template get_metaentry(addr) } + { Meta::template get_metaentry(addr) } -> ConceptSame; - { Meta::template get_metaentry(addr) } + { Meta::template get_metaentry(addr) } -> ConceptSame; }; diff --git a/src/backend/chunkallocator.h b/src/backend/chunkallocator.h index 335f4f5..80d1be5 100644 --- a/src/backend/chunkallocator.h +++ b/src/backend/chunkallocator.h @@ -183,10 +183,9 @@ namespace snmalloc static std::pair, Metaslab*> alloc_chunk( typename SharedStateHandle::LocalState& local_state, ChunkAllocatorLocalState& chunk_alloc_local_state, - sizeclass_t sizeclass, chunksizeclass_t slab_sizeclass, size_t slab_size, - RemoteAllocator* remote) + uintptr_t ras) { using PAL = typename SharedStateHandle::Pal; ChunkAllocatorState& state = @@ -234,7 +233,7 @@ namespace snmalloc << " memory in stacks " << state.memory_in_stacks << std::endl; #endif - MetaEntry entry{meta, remote, sizeclass}; + MetaEntry entry{&meta->meta_common, ras}; SharedStateHandle::Pagemap::set_metaentry( address_cast(slab), slab_size, entry); return {slab, meta}; @@ -242,8 +241,8 @@ namespace snmalloc // Allocate a fresh slab as there are no available ones. // First create meta-data - auto [slab, meta] = SharedStateHandle::alloc_chunk( - &local_state, slab_size, remote, sizeclass); + auto [slab, meta] = + SharedStateHandle::alloc_chunk(&local_state, slab_size, ras); #ifdef SNMALLOC_TRACING std::cout << "Create slab:" << slab.unsafe_ptr() << " slab_sizeclass " << slab_sizeclass << " size " << slab_size << std::endl; diff --git a/src/backend/metatypes.h b/src/backend/metatypes.h index c65efaa..6646203 100644 --- a/src/backend/metatypes.h +++ b/src/backend/metatypes.h @@ -79,10 +79,6 @@ namespace snmalloc #endif // clang-format on - struct RemoteAllocator; - class Metaslab; - class sizeclass_t; - /** * Entry stored in the pagemap. See docs/AddressSpace.md for the full * MetaEntry lifecycle. @@ -140,41 +136,23 @@ namespace snmalloc * `get_remote_and_sizeclass`. */ SNMALLOC_FAST_PATH - MetaEntry(Metaslab* meta, uintptr_t remote_and_sizeclass) - : meta(unsafe_to_uintptr(meta)), + MetaEntry(MetaCommon* meta, uintptr_t remote_and_sizeclass) + : meta(unsafe_to_uintptr(meta)), remote_and_sizeclass(remote_and_sizeclass) {} - /* See mem/metaslab.h */ - SNMALLOC_FAST_PATH - MetaEntry(Metaslab* meta, RemoteAllocator* remote, sizeclass_t sizeclass); - - /** - * Return the Metaslab metadata associated with this chunk, guarded by an - * assert that this chunk is being used as a slab (i.e., has an associated - * owning allocator). - */ - [[nodiscard]] SNMALLOC_FAST_PATH Metaslab* get_metaslab() const - { - SNMALLOC_ASSERT(get_remote() != nullptr); - return unsafe_from_uintptr(meta & ~META_BOUNDARY_BIT); - } - /** * Return the remote and sizeclass in an implementation-defined encoding. * This is not guaranteed to be stable across snmalloc releases and so the * only safe use for this is to pass it to the two-argument constructor of * this class. */ - [[nodiscard]] SNMALLOC_FAST_PATH uintptr_t get_remote_and_sizeclass() const + [[nodiscard]] SNMALLOC_FAST_PATH const uintptr_t& + get_remote_and_sizeclass() const { return remote_and_sizeclass; } - /* See mem/metaslab.h */ - [[nodiscard]] SNMALLOC_FAST_PATH RemoteAllocator* get_remote() const; - [[nodiscard]] SNMALLOC_FAST_PATH sizeclass_t get_sizeclass() const; - MetaEntry(const MetaEntry&) = delete; MetaEntry& operator=(const MetaEntry& other) @@ -186,6 +164,16 @@ namespace snmalloc return *this; } + /** + * Return the Metaslab metadata associated with this chunk, guarded by an + * assert that this chunk is being used as a slab (i.e., has an associated + * owning allocator). + */ + [[nodiscard]] SNMALLOC_FAST_PATH MetaCommon* get_meta() const + { + return reinterpret_cast(meta & ~META_BOUNDARY_BIT); + } + void set_boundary() { meta |= META_BOUNDARY_BIT; @@ -201,5 +189,4 @@ namespace snmalloc return meta &= ~META_BOUNDARY_BIT; } }; - } // namespace snmalloc diff --git a/src/mem/corealloc.h b/src/mem/corealloc.h index 3b8e36a..b22fbb0 100644 --- a/src/mem/corealloc.h +++ b/src/mem/corealloc.h @@ -403,7 +403,8 @@ namespace snmalloc * by this thread, or handling the final deallocation onto a slab, * so it can be reused by other threads. */ - SNMALLOC_SLOW_PATH void dealloc_local_object_slow(const MetaEntry& entry) + SNMALLOC_SLOW_PATH void + dealloc_local_object_slow(const MetaslabMetaEntry& entry) { // TODO: Handle message queue on this path? @@ -486,19 +487,20 @@ namespace snmalloc [local_state](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { return capptr_domesticate(local_state, p); }; - auto cb = [this, &need_post](freelist::HeadPtr msg) - SNMALLOC_FAST_PATH_LAMBDA { + auto cb = [this, + &need_post](freelist::HeadPtr msg) SNMALLOC_FAST_PATH_LAMBDA { #ifdef SNMALLOC_TRACING - std::cout << "Handling remote" << std::endl; + std::cout << "Handling remote" << std::endl; #endif - auto& entry = SharedStateHandle::Pagemap::get_metaentry( - snmalloc::address_cast(msg)); + auto& entry = + SharedStateHandle::Pagemap::template get_metaentry( + snmalloc::address_cast(msg)); - handle_dealloc_remote(entry, msg.as_void(), need_post); + handle_dealloc_remote(entry, msg.as_void(), need_post); - return true; - }; + return true; + }; if constexpr (SharedStateHandle::Options.QueueHeadsAreTame) { @@ -532,7 +534,7 @@ namespace snmalloc * need_post will be set to true, if capacity is exceeded. */ void handle_dealloc_remote( - const MetaEntry& entry, + const MetaslabMetaEntry& entry, CapPtr p, bool& need_post) { @@ -672,8 +674,10 @@ namespace snmalloc SNMALLOC_FAST_PATH void dealloc_local_object(CapPtr p) { - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(snmalloc::address_cast(p)); + // MetaEntry-s seen here are expected to have meaningful Remote pointers + auto& entry = + SharedStateHandle::Pagemap::template get_metaentry( + snmalloc::address_cast(p)); if (SNMALLOC_LIKELY(dealloc_local_object_fast(entry, p, entropy))) return; @@ -681,7 +685,7 @@ namespace snmalloc } SNMALLOC_FAST_PATH static bool dealloc_local_object_fast( - const MetaEntry& entry, + const MetaslabMetaEntry& entry, CapPtr p, LocalEntropy& entropy) { @@ -786,8 +790,8 @@ namespace snmalloc auto [slab, meta] = SharedStateHandle::alloc_chunk( get_backend_local_state(), slab_size, - public_state(), - sizeclass_t::from_small_class(sizeclass)); + MetaslabMetaEntry::encode( + public_state(), sizeclass_t::from_small_class(sizeclass))); if (slab == nullptr) { @@ -840,8 +844,9 @@ namespace snmalloc { bool need_post = true; // Always going to post, so ignore. auto n_tame = p_tame->atomic_read_next(key_global, domesticate); - auto& entry = SharedStateHandle::Pagemap::get_metaentry( - snmalloc::address_cast(p_tame)); + const MetaslabMetaEntry& entry = + SharedStateHandle::Pagemap::template get_metaentry< + MetaslabMetaEntry>(snmalloc::address_cast(p_tame)); handle_dealloc_remote(entry, p_tame.as_void(), need_post); p_tame = n_tame; } diff --git a/src/mem/localalloc.h b/src/mem/localalloc.h index d9bf184..db32d61 100644 --- a/src/mem/localalloc.h +++ b/src/mem/localalloc.h @@ -183,8 +183,8 @@ namespace snmalloc auto [chunk, meta] = SharedStateHandle::alloc_chunk( core_alloc->get_backend_local_state(), large_size_to_chunk_size(size), - core_alloc->public_state(), - size_to_sizeclass_full(size)); + MetaslabMetaEntry::encode( + core_alloc->public_state(), size_to_sizeclass_full(size))); // set up meta data so sizeclass is correct, and hence alloc size, and // external pointer. #ifdef SNMALLOC_TRACING @@ -266,8 +266,9 @@ namespace snmalloc std::cout << "Remote dealloc post" << p.unsafe_ptr() << " size " << alloc_size(p.unsafe_ptr()) << std::endl; #endif - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(p)); + const MetaslabMetaEntry& entry = + SharedStateHandle::Pagemap::template get_metaentry( + address_cast(p)); local_cache.remote_dealloc_cache.template dealloc( entry.get_remote()->trunc_id(), p, key_global); post_remote_cache(); @@ -624,8 +625,9 @@ namespace snmalloc capptr::Alloc p_tame = capptr_domesticate( core_alloc->backend_state_ptr(), p_wild); - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(p_tame)); + const MetaslabMetaEntry& entry = + SharedStateHandle::Pagemap::template get_metaentry( + address_cast(p_tame)); if (SNMALLOC_LIKELY(local_cache.remote_allocator == entry.get_remote())) { # if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT) @@ -714,8 +716,9 @@ namespace snmalloc // To handle this case we require the uninitialised pagemap contain an // entry for the first chunk of memory, that states it represents a // large object, so we can pull the check for null off the fast path. - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(p_raw)); + const MetaslabMetaEntry& entry = + SharedStateHandle::Pagemap::template get_metaentry( + address_cast(p_raw)); return sizeclass_full_to_size(entry.get_sizeclass()); #endif @@ -759,9 +762,8 @@ namespace snmalloc size_t remaining_bytes(const void* p) { #ifndef SNMALLOC_PASS_THROUGH - const MetaEntry& entry = - SharedStateHandle::Pagemap::template get_metaentry( - address_cast(p)); + const MetaslabMetaEntry& entry = SharedStateHandle::Pagemap:: + template get_metaentry(address_cast(p)); auto sizeclass = entry.get_sizeclass(); return snmalloc::remaining_bytes(sizeclass, address_cast(p)); @@ -788,9 +790,8 @@ namespace snmalloc size_t index_in_object(const void* p) { #ifndef SNMALLOC_PASS_THROUGH - const MetaEntry& entry = - SharedStateHandle::Pagemap::template get_metaentry( - address_cast(p)); + const MetaslabMetaEntry& entry = SharedStateHandle::Pagemap:: + template get_metaentry(address_cast(p)); auto sizeclass = entry.get_sizeclass(); return snmalloc::index_in_object(sizeclass, address_cast(p)); diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h index fdf2577..2695114 100644 --- a/src/mem/metaslab.h +++ b/src/mem/metaslab.h @@ -226,39 +226,56 @@ namespace snmalloc }; /* - * MetaEntry methods that deal with RemoteAllocator* and sizeclass_t are here, - * so that the backend does not need to know the details and can, instead, - * just provide the storage space. + * A convenience wrapper aroun MetaEntry with a meaningful RemoteAllocator + * pointer. This encodes a RemoteAllocator* and a sizeclass_t into a the + * uintptr_t remote_and_sizeclass field. + * + * There's a little bit of an asymmetry here. Since the backend actually sets + * the entry (when associating a metadata structure), MetaslabMetaEntry-s are + * not constructed directly; please use ::encode(). On the other hand, the + * backend's Pagemap::get_metaentry() method is templated on its return type, + * so it is relatively straightforward to view a pagemap entry as a + * MetaslabMetaEntry and then use the accessors here for decoding. */ - - SNMALLOC_FAST_PATH_INLINE - MetaEntry::MetaEntry( - Metaslab* meta, - RemoteAllocator* remote, - sizeclass_t sizeclass = sizeclass_t()) - : meta(reinterpret_cast(meta)) + struct MetaslabMetaEntry : public MetaEntry { - /* remote might be nullptr; cast to uintptr_t before offsetting */ - remote_and_sizeclass = - pointer_offset(reinterpret_cast(remote), sizeclass.raw()); - } + /// Perform the encoding. + static SNMALLOC_FAST_PATH uintptr_t + encode(RemoteAllocator* remote, sizeclass_t sizeclass) + { + /* remote might be nullptr; cast to uintptr_t before offsetting */ + return pointer_offset( + reinterpret_cast(remote), sizeclass.raw()); + } - [[nodiscard]] SNMALLOC_FAST_PATH_INLINE RemoteAllocator* - MetaEntry::get_remote() const - { - return reinterpret_cast( - pointer_align_down( - remote_and_sizeclass)); - } + [[nodiscard]] SNMALLOC_FAST_PATH RemoteAllocator* get_remote() const + { + return reinterpret_cast( + pointer_align_down( + get_remote_and_sizeclass())); + } - [[nodiscard]] SNMALLOC_FAST_PATH_INLINE sizeclass_t - MetaEntry::get_sizeclass() const - { - // TODO: perhaps remove static_cast with resolution of - // https://github.com/CTSRD-CHERI/llvm-project/issues/588 - return sizeclass_t::from_raw( - static_cast(remote_and_sizeclass) & - (REMOTE_WITH_BACKEND_MARKER_ALIGN - 1)); - } + [[nodiscard]] SNMALLOC_FAST_PATH sizeclass_t get_sizeclass() const + { + // TODO: perhaps remove static_cast with resolution of + // https://github.com/CTSRD-CHERI/llvm-project/issues/588 + return sizeclass_t::from_raw( + static_cast(get_remote_and_sizeclass()) & + (REMOTE_WITH_BACKEND_MARKER_ALIGN - 1)); + } + + /** + * Return the Metaslab metadata associated with this chunk, guarded by an + * assert that this chunk is being used as a slab (i.e., has an associated + * owning allocator). + */ + [[nodiscard]] SNMALLOC_FAST_PATH Metaslab* get_metaslab() const + { + SNMALLOC_ASSERT(get_remote() != nullptr); + return reinterpret_cast(get_meta()); + } + }; + + static_assert(sizeof(MetaslabMetaEntry) == sizeof(MetaEntry)); } // namespace snmalloc diff --git a/src/mem/remotecache.h b/src/mem/remotecache.h index c80944d..e97df1a 100644 --- a/src/mem/remotecache.h +++ b/src/mem/remotecache.h @@ -52,7 +52,7 @@ namespace snmalloc * * This does not require initialisation to be safely called. */ - SNMALLOC_FAST_PATH bool reserve_space(const MetaEntry& entry) + SNMALLOC_FAST_PATH bool reserve_space(const MetaslabMetaEntry& entry) { auto size = static_cast(sizeclass_full_to_size(entry.get_sizeclass())); @@ -101,8 +101,9 @@ namespace snmalloc if (!list[i].empty()) { auto [first, last] = list[i].extract_segment(key); - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(first)); + const MetaslabMetaEntry& entry = + SharedStateHandle::Pagemap::template get_metaentry< + MetaslabMetaEntry>(address_cast(first)); auto remote = entry.get_remote(); // If the allocator is not correctly aligned, then the bit that is // set implies this is used by the backend, and we should not be @@ -141,8 +142,9 @@ namespace snmalloc // Use the next N bits to spread out remote deallocs in our own // slot. auto r = resend.take(key, domesticate); - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(r)); + const MetaslabMetaEntry& entry = + SharedStateHandle::Pagemap::template get_metaentry< + MetaslabMetaEntry>(address_cast(r)); auto i = entry.get_remote()->trunc_id(); size_t slot = get_slot(i, post_round); list[slot].add(r, key);