From 7e53a2e82a2e6b0aa64e8d7defbc2790a44b50a4 Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Tue, 21 Sep 2021 19:17:51 +0100 Subject: [PATCH] CapPtr: shift free lists to Alloc bounds This is incomplete, yet still more reflective of what's going on: we take the exported pointers back from userspace and thread them directly into the free lists. So: move capptr_to_user_address_control to list construction time rather than list consumption time. --- src/mem/corealloc.h | 41 ++++++++++--------- src/mem/freelist.h | 4 +- src/mem/localalloc.h | 83 ++++++++++++++++++++------------------- src/mem/localcache.h | 11 +++--- src/mem/metaslab.h | 8 ++-- src/mem/remoteallocator.h | 32 +++++++-------- src/mem/remotecache.h | 11 +++--- 7 files changed, 99 insertions(+), 91 deletions(-) diff --git a/src/mem/corealloc.h b/src/mem/corealloc.h index ecc01b0..06858f8 100644 --- a/src/mem/corealloc.h +++ b/src/mem/corealloc.h @@ -162,8 +162,8 @@ namespace snmalloc // Manufacture an allocation to prime the queue // Using an actual allocation removes a conditional from a critical path. auto dummy = - capptr::AllocFull(small_alloc_one(MIN_ALLOC_SIZE)) - .template as_static>(); + capptr::Alloc(small_alloc_one(MIN_ALLOC_SIZE)) + .template as_static>(); if (dummy == nullptr) { error("Critical error: Out-of-memory during initialisation."); @@ -185,8 +185,9 @@ namespace snmalloc size, [&]( sizeclass_t sizeclass, - FreeListIter* - fl) { return small_alloc(sizeclass, *fl); }); + FreeListIter* fl) { + return small_alloc(sizeclass, *fl); + }); } static SNMALLOC_FAST_PATH void alloc_new_list( @@ -248,7 +249,8 @@ namespace snmalloc do { b.add( - FreeObject::make(curr_ptr.as_void()), + FreeObject::make( + capptr_to_user_address_control(curr_ptr.as_void())), key, entropy); curr_ptr = curr_ptr->next; @@ -258,9 +260,10 @@ namespace snmalloc do { b.add( - Aal::capptr_bound< - FreeObject::T, - capptr::bounds::AllocFull>(p, rsize), + FreeObject::make( + capptr_to_user_address_control( + Aal::capptr_bound( + p.as_void(), rsize))), key); p = pointer_offset(p, rsize); } while (p < slab_end); @@ -272,7 +275,7 @@ namespace snmalloc ChunkRecord* clear_slab(Metaslab* meta, sizeclass_t sizeclass) { auto& key = entropy.get_free_list_key(); - FreeListIter fl; + FreeListIter fl; auto more = meta->free_queue.close(fl, key); UNUSED(more); void* p = finish_alloc_no_zero(fl.take(key), sizeclass); @@ -435,7 +438,7 @@ namespace snmalloc */ void handle_dealloc_remote( const MetaEntry& entry, - CapPtr p, + CapPtr p, bool& need_post) { // TODO this needs to not double count stats @@ -444,7 +447,7 @@ namespace snmalloc if (likely(entry.get_remote() == public_state())) { - if (likely(dealloc_local_object_fast(entry, p.unsafe_ptr(), entropy))) + if (likely(dealloc_local_object_fast(entry, p.as_void(), entropy))) return; dealloc_local_object_slow(entry); @@ -569,7 +572,8 @@ namespace snmalloc return handle_message_queue_inner(action, args...); } - SNMALLOC_FAST_PATH void dealloc_local_object(void* p) + SNMALLOC_FAST_PATH void + dealloc_local_object(CapPtr p) { auto entry = SharedStateHandle::Pagemap::get_metaentry( backend_state_ptr(), snmalloc::address_cast(p)); @@ -580,7 +584,9 @@ namespace snmalloc } SNMALLOC_FAST_PATH static bool dealloc_local_object_fast( - const MetaEntry& entry, void* p, LocalEntropy& entropy) + const MetaEntry& entry, + CapPtr p, + LocalEntropy& entropy) { auto meta = entry.get_metaslab(); @@ -590,8 +596,7 @@ namespace snmalloc Metaslab::is_start_of_object(entry.get_sizeclass(), address_cast(p)), "Not deallocating start of an object"); - auto cp = FreeObject::QueuePtr( - reinterpret_cast*>(p)); + auto cp = p.as_static>(); auto& key = entropy.get_free_list_key(); @@ -604,7 +609,7 @@ namespace snmalloc template SNMALLOC_SLOW_PATH void* small_alloc( sizeclass_t sizeclass, - FreeListIter& + FreeListIter& fast_free_list) { size_t rsize = sizeclass_to_size(sizeclass); @@ -664,7 +669,7 @@ namespace snmalloc template SNMALLOC_SLOW_PATH void* small_alloc_slow( sizeclass_t sizeclass, - FreeListIter& + FreeListIter& fast_free_list, size_t rsize) { @@ -717,7 +722,7 @@ namespace snmalloc { SNMALLOC_ASSERT(attached_cache != nullptr); // TODO: Placeholder - auto domesticate = [](FreeObject::QueuePtr p) + auto domesticate = [](FreeObject::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { return p; }; if (destroy_queue) diff --git a/src/mem/freelist.h b/src/mem/freelist.h index 9dd8d9b..fadf81e 100644 --- a/src/mem/freelist.h +++ b/src/mem/freelist.h @@ -249,8 +249,8 @@ namespace snmalloc /** * Decode next. While traversing a queue, BView and BQueue here will often - * be equal (i.e., CBAllocExportWild) rather than dichotomous. However, - * we do occasionally decode an actual head pointer, so be polymorphic here. + * be equal (i.e., AllocUserWild) rather than dichotomous. However, we do + * occasionally decode an actual head pointer, so be polymorphic here. * * TODO: We'd like, in some sense, to more tightly couple or integrate this * into to the domestication process. We could introduce an additional diff --git a/src/mem/localalloc.h b/src/mem/localalloc.h index ac183f0..74d33b9 100644 --- a/src/mem/localalloc.h +++ b/src/mem/localalloc.h @@ -214,30 +214,29 @@ namespace snmalloc auto slowpath = [&]( sizeclass_t sizeclass, - FreeListIter* - fl) SNMALLOC_FAST_PATH_LAMBDA { - if (likely(core_alloc != nullptr)) - { - return core_alloc->handle_message_queue( - []( - CoreAlloc* core_alloc, - sizeclass_t sizeclass, - FreeListIter< - capptr::bounds::AllocFull, - capptr::bounds::AllocFull>* fl) { - return core_alloc->template small_alloc( - sizeclass, *fl); + FreeListIter* fl) + SNMALLOC_FAST_PATH_LAMBDA { + if (likely(core_alloc != nullptr)) + { + return core_alloc->handle_message_queue( + []( + CoreAlloc* core_alloc, + sizeclass_t sizeclass, + FreeListIter* + fl) { + return core_alloc->template small_alloc( + sizeclass, *fl); + }, + core_alloc, + sizeclass, + fl); + } + return lazy_init( + [&](CoreAlloc*, sizeclass_t sizeclass) { + return small_alloc(sizeclass_to_size(sizeclass)); }, - core_alloc, - sizeclass, - fl); - } - return lazy_init( - [&](CoreAlloc*, sizeclass_t sizeclass) { - return small_alloc(sizeclass_to_size(sizeclass)); - }, - sizeclass); - }; + sizeclass); + }; return local_cache.template alloc( size, slowpath); @@ -260,20 +259,18 @@ namespace snmalloc * In the second case we need to recheck if this is a remote deallocation, * as we might acquire the originating allocator. */ - SNMALLOC_SLOW_PATH void dealloc_remote_slow(void* p) + SNMALLOC_SLOW_PATH void dealloc_remote_slow(capptr::Alloc p) { if (core_alloc != nullptr) { #ifdef SNMALLOC_TRACING - std::cout << "Remote dealloc post" << p << " size " << alloc_size(p) - << std::endl; + std::cout << "Remote dealloc post" << p.unsafe_ptr() << " size " + << alloc_size(p.unsafe_ptr()) << std::endl; #endif MetaEntry entry = SharedStateHandle::Pagemap::get_metaentry( core_alloc->backend_state_ptr(), address_cast(p)); local_cache.remote_dealloc_cache.template dealloc( - entry.get_remote()->trunc_id(), - capptr::AllocFull(p), - key_global); + entry.get_remote()->trunc_id(), p, key_global); post_remote_cache(); return; } @@ -281,8 +278,8 @@ namespace snmalloc // Recheck what kind of dealloc we should do incase, the allocator we get // from lazy_init is the originating allocator. lazy_init( - [&](CoreAlloc*, void* p) { - dealloc(p); // TODO don't double count statistics + [&](CoreAlloc*, CapPtr p) { + dealloc(p.unsafe_ptr()); // TODO don't double count statistics return nullptr; }, p); @@ -450,10 +447,10 @@ namespace snmalloc return alloc(size); } - SNMALLOC_FAST_PATH void dealloc(void* p) + SNMALLOC_FAST_PATH void dealloc(void* p_raw) { #ifdef SNMALLOC_PASS_THROUGH - external_alloc::free(p); + external_alloc::free(p_raw); #else // TODO: // Care is needed so that dealloc(nullptr) works before init @@ -461,6 +458,7 @@ namespace snmalloc // before init, that maps null to a remote_deallocator that will never be // in thread local state. + auto p = capptr_from_client(p_raw); const MetaEntry& entry = SharedStateHandle::Pagemap::get_metaentry( core_alloc->backend_state_ptr(), address_cast(p)); if (likely(local_cache.remote_allocator == entry.get_remote())) @@ -478,12 +476,10 @@ namespace snmalloc if (local_cache.remote_dealloc_cache.reserve_space(entry)) { local_cache.remote_dealloc_cache.template dealloc( - entry.get_remote()->trunc_id(), - capptr::AllocFull(p), - key_global); + entry.get_remote()->trunc_id(), p, key_global); # ifdef SNMALLOC_TRACING - std::cout << "Remote dealloc fast" << p << " size " << alloc_size(p) - << std::endl; + std::cout << "Remote dealloc fast" << p_raw << " size " + << alloc_size(p_raw) << std::endl; # endif return; } @@ -493,7 +489,7 @@ namespace snmalloc } // Large deallocation or null. - if (likely(p != nullptr)) + if (likely(p.unsafe_ptr() != nullptr)) { // Check this is managed by this pagemap. check_client(entry.get_sizeclass() != 0, "Not allocated by snmalloc."); @@ -511,7 +507,14 @@ namespace snmalloc # endif ChunkRecord* slab_record = reinterpret_cast(entry.get_metaslab()); - slab_record->chunk = capptr::Chunk(p); + /* + * StrictProvenance TODO: this is a subversive amplification. p is + * AllocWild-bounded, but we're coercing it to Chunk-bounded. We + * should, instead, not be storing ->chunk here, but should be keeping + * a CapPtr to this region internally even while it's + * allocated. + */ + slab_record->chunk = capptr::Chunk(p.unsafe_ptr()); check_init( []( CoreAlloc* core_alloc, diff --git a/src/mem/localcache.h b/src/mem/localcache.h index 8184141..d72648d 100644 --- a/src/mem/localcache.h +++ b/src/mem/localcache.h @@ -13,20 +13,20 @@ namespace snmalloc using Stats = AllocStats; inline static SNMALLOC_FAST_PATH void* finish_alloc_no_zero( - FreeObject::HeadPtr p, + FreeObject::HeadPtr p, sizeclass_t sizeclass) { SNMALLOC_ASSERT(Metaslab::is_start_of_object(sizeclass, address_cast(p))); UNUSED(sizeclass); - auto r = capptr_reveal(capptr_to_user_address_control(p.as_void())); + auto r = capptr_reveal(p.as_void()); return r; } template inline static SNMALLOC_FAST_PATH void* finish_alloc( - FreeObject::HeadPtr p, + FreeObject::HeadPtr p, sizeclass_t sizeclass) { auto r = finish_alloc_no_zero(p, sizeclass); @@ -48,7 +48,7 @@ namespace snmalloc // Free list per small size class. These are used for // allocation on the fast path. This part of the code is inspired by // mimalloc. - FreeListIter + FreeListIter small_fast_free_lists[NUM_SIZECLASSES]; // This is the entropy for a particular thread. @@ -90,7 +90,8 @@ namespace snmalloc while (!small_fast_free_lists[i].empty()) { auto p = small_fast_free_lists[i].take(key); - dealloc(finish_alloc_no_zero(p, i)); + SNMALLOC_ASSERT(Metaslab::is_start_of_object(i, address_cast(p))); + dealloc(p.as_void()); } } diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h index 9a7165b..dca9dca 100644 --- a/src/mem/metaslab.h +++ b/src/mem/metaslab.h @@ -25,10 +25,10 @@ namespace snmalloc * Data-structure for building the free list for this slab. */ #ifdef SNMALLOC_CHECK_CLIENT - FreeListBuilder + FreeListBuilder free_queue; #else - FreeListBuilder + FreeListBuilder free_queue; #endif @@ -155,10 +155,10 @@ namespace snmalloc * available objects for this metaslab. */ static SNMALLOC_FAST_PATH - std::pair, bool> + std::pair, bool> alloc_free_list( Metaslab* meta, - FreeListIter& + FreeListIter& fast_free_list, LocalEntropy& entropy, sizeclass_t sizeclass) diff --git a/src/mem/remoteallocator.h b/src/mem/remoteallocator.h index 2be554f..4b5ff1b 100644 --- a/src/mem/remoteallocator.h +++ b/src/mem/remoteallocator.h @@ -36,11 +36,11 @@ namespace snmalloc // Store the message queue on a separate cacheline. It is mutable data that // is read by other threads. alignas(CACHELINE_SIZE) - FreeObject::AtomicQueuePtr back{nullptr}; + FreeObject::AtomicQueuePtr back{nullptr}; // Store the two ends on different cache lines as access by different // threads. - alignas(CACHELINE_SIZE) - FreeObject::QueuePtr front{nullptr}; + alignas(CACHELINE_SIZE) FreeObject::QueuePtr front{ + nullptr}; constexpr RemoteAllocator() = default; @@ -50,7 +50,7 @@ namespace snmalloc SNMALLOC_ASSERT(front != nullptr); } - void init(FreeObject::QueuePtr stub) + void init(FreeObject::QueuePtr stub) { FreeObject::atomic_store_null(stub, key_global); front = stub; @@ -58,9 +58,9 @@ namespace snmalloc invariant(); } - FreeObject::QueuePtr destroy() + FreeObject::QueuePtr destroy() { - FreeObject::QueuePtr fnt = front; + FreeObject::QueuePtr fnt = front; back.store(nullptr, std::memory_order_relaxed); front = nullptr; return fnt; @@ -68,7 +68,7 @@ namespace snmalloc inline bool is_empty() { - FreeObject::QueuePtr bk = + FreeObject::QueuePtr bk = back.load(std::memory_order_relaxed); return bk == front; @@ -79,22 +79,22 @@ namespace snmalloc * last should be linked together through their next pointers. */ void enqueue( - FreeObject::QueuePtr first, - FreeObject::QueuePtr last, + FreeObject::QueuePtr first, + FreeObject::QueuePtr last, const FreeListKey& key) { invariant(); FreeObject::atomic_store_null(last, key); // exchange needs to be a release, so nullptr in next is visible. - FreeObject::QueuePtr prev = + FreeObject::QueuePtr prev = back.exchange(last, std::memory_order_release); // XXX prev is not known to be domesticated FreeObject::atomic_store_next(prev, first, key); } - FreeObject::QueuePtr peek() + FreeObject::QueuePtr peek() { return front; } @@ -103,16 +103,16 @@ namespace snmalloc * Returns the front message, or null if not possible to return a message. */ std::pair< - FreeObject::HeadPtr, + FreeObject::HeadPtr, bool> dequeue(const FreeListKey& key) { - auto domesticate = [](FreeObject::QueuePtr p) + auto domesticate = [](FreeObject::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { return p; }; invariant(); - FreeObject::HeadPtr - first = domesticate(front); - FreeObject::QueuePtr next = + FreeObject::HeadPtr first = + domesticate(front); + FreeObject::QueuePtr next = first->atomic_read_next(key, domesticate); if (next != nullptr) diff --git a/src/mem/remotecache.h b/src/mem/remotecache.h index 7bcc57a..4f38ab9 100644 --- a/src/mem/remotecache.h +++ b/src/mem/remotecache.h @@ -19,8 +19,8 @@ namespace snmalloc std::array< FreeListBuilder< false, - capptr::bounds::AllocFull, - capptr::bounds::AllocFull, + capptr::bounds::Alloc, + capptr::bounds::Alloc, false>, REMOTE_SLOTS> list; @@ -73,12 +73,12 @@ namespace snmalloc template SNMALLOC_FAST_PATH void dealloc( RemoteAllocator::alloc_id_t target_id, - capptr::AllocFull p, + capptr::Alloc p, const FreeListKey& key) { SNMALLOC_ASSERT(initialised); auto r = - p.template as_reinterpret>(); + p.template as_reinterpret>(); list[get_slot(target_id, 0)].add(r, key); } @@ -118,8 +118,7 @@ namespace snmalloc // Entries could map back onto the "resend" list, // so take copy of the head, mark the last element, // and clear the original list. - FreeListIter - resend; + FreeListIter resend; list[my_slot].close(resend, key); post_round++;