From 70c3e00df77e6a930a16ac76694ce289e2e8a36b Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Fri, 20 Aug 2021 21:19:18 +0100 Subject: [PATCH] AddressSpace: use Backend to access Pagemap And do so by type, rather than by value. While here, introduce a C++20 concept for this Backend-offered proxy and adjust the template parameters appropriately. This will be useful for the process sandbox code, which needs to mediate stores to the pagemap, but can provide a read-only view. --- src/backend/address_space.h | 30 ++++++------ src/backend/address_space_core.h | 67 +++++++++++++++------------ src/backend/backend.h | 79 +++++++++++++++++--------------- src/backend/backend_concept.h | 57 +++++++++++++++++++++++ src/mem/corealloc.h | 9 ++-- src/mem/localalloc.h | 11 +++-- src/mem/remotecache.h | 5 +- src/mem/slaballocator.h | 3 +- 8 files changed, 168 insertions(+), 93 deletions(-) create mode 100644 src/backend/backend_concept.h diff --git a/src/backend/address_space.h b/src/backend/address_space.h index fbb8be9..cbdc0fd 100644 --- a/src/backend/address_space.h +++ b/src/backend/address_space.h @@ -42,8 +42,8 @@ namespace snmalloc * part of satisfying the request will be registered with the provided * arena_map for use in subsequent amplification. */ - template - CapPtr reserve(size_t size, Pagemap& pagemap) + template + CapPtr reserve(size_t size) { #ifdef SNMALLOC_TRACING std::cout << "ASM reserve request:" << size << std::endl; @@ -63,7 +63,7 @@ namespace snmalloc { auto base = CapPtr( PAL::template reserve_aligned(size)); - pagemap.register_range(address_cast(base), size); + Pagemap::register_range(address_cast(base), size); return base; } } @@ -71,7 +71,7 @@ namespace snmalloc CapPtr res; { FlagLock lock(spin_lock); - res = core.template reserve(size, pagemap); + res = core.template reserve(size); if (res == nullptr) { // Allocation failed ask OS for more memory @@ -133,12 +133,12 @@ namespace snmalloc return nullptr; } - pagemap.register_range(address_cast(block), block_size); + Pagemap::register_range(address_cast(block), block_size); - core.template add_range(block, block_size, pagemap); + core.template add_range(block, block_size); // still holding lock so guaranteed to succeed. - res = core.template reserve(size, pagemap); + res = core.template reserve(size); } } @@ -156,8 +156,8 @@ namespace snmalloc * This is useful for allowing the space required for alignment to be * used, by smaller objects. */ - template - CapPtr reserve_with_left_over(size_t size, Pagemap& pagemap) + template + CapPtr reserve_with_left_over(size_t size) { SNMALLOC_ASSERT(size >= sizeof(void*)); @@ -165,15 +165,15 @@ namespace snmalloc size_t rsize = bits::next_pow2(size); - auto res = reserve(rsize, pagemap); + auto res = reserve(rsize); if (res != nullptr) { if (rsize > size) { FlagLock lock(spin_lock); - core.template add_range( - pointer_offset(res, size), rsize - size, pagemap); + core.template add_range( + pointer_offset(res, size), rsize - size); } if constexpr (committed) @@ -193,11 +193,11 @@ namespace snmalloc * Add a range of memory to the address space. * Divides blocks into power of two sizes with natural alignment */ - template - void add_range(CapPtr base, size_t length, Pagemap& pagemap) + template + void add_range(CapPtr base, size_t length) { FlagLock lock(spin_lock); - core.add_range(base, length, pagemap); + core.add_range(base, length); } }; } // namespace snmalloc diff --git a/src/backend/address_space_core.h b/src/backend/address_space_core.h index d24b89f..68712b0 100644 --- a/src/backend/address_space_core.h +++ b/src/backend/address_space_core.h @@ -4,6 +4,7 @@ #include "../mem/allocconfig.h" #include "../mem/metaslab.h" #include "../pal/pal.h" +#include "backend_concept.h" #include #ifdef SNMALLOC_TRACING @@ -67,12 +68,11 @@ namespace snmalloc * to store the next pointer for the list of unused address space of a * particular size. */ - template + template void set_next( size_t align_bits, CapPtr base, - CapPtr next, - Pagemap& pagemap) + CapPtr next) { if (align_bits >= MIN_CHUNK_BITS) { @@ -84,7 +84,7 @@ namespace snmalloc // external_pointer, for example) will not attempt to follow this // "Metaslab" pointer. MetaEntry t(reinterpret_cast(next.unsafe_ptr()), nullptr, 0); - pagemap.set(address_cast(base), t); + Pagemap::set_meta_data(address_cast(base), 1, t); return; } @@ -100,13 +100,14 @@ namespace snmalloc * to store the next pointer for the list of unused address space of a * particular size. */ - template - CapPtr get_next( - size_t align_bits, CapPtr base, Pagemap& pagemap) + template + CapPtr + get_next(size_t align_bits, CapPtr base) { if (align_bits >= MIN_CHUNK_BITS) { - const MetaEntry& t = pagemap.template get(address_cast(base)); + const MetaEntry& t = + Pagemap::template get_meta_data(address_cast(base)); return CapPtr( reinterpret_cast(t.get_metaslab())); } @@ -117,14 +118,15 @@ namespace snmalloc /** * Adds a block to `ranges`. */ - template - void add_block( - size_t align_bits, CapPtr base, Pagemap& pagemap) + template< + SNMALLOC_CONCEPT(ConceptPAL) PAL, + SNMALLOC_CONCEPT(ConceptBackendMeta) Pagemap> + void add_block(size_t align_bits, CapPtr base) { check_block(base, align_bits); SNMALLOC_ASSERT(align_bits < 64); - set_next(align_bits, base, ranges[align_bits], pagemap); + set_next(align_bits, base, ranges[align_bits]); ranges[align_bits] = base.as_static(); } @@ -132,8 +134,10 @@ namespace snmalloc * Find a block of the correct size. May split larger blocks * to satisfy this request. */ - template - CapPtr remove_block(size_t align_bits, Pagemap& pagemap) + template< + SNMALLOC_CONCEPT(ConceptPAL) PAL, + SNMALLOC_CONCEPT(ConceptBackendMeta) Pagemap> + CapPtr remove_block(size_t align_bits) { CapPtr first = ranges[align_bits]; if (first == nullptr) @@ -146,7 +150,7 @@ namespace snmalloc // Look for larger block and split up recursively CapPtr bigger = - remove_block(align_bits + 1, pagemap); + remove_block(align_bits + 1); if (bigger != nullptr) { // This block is going to be broken up into sub CHUNK_SIZE blocks @@ -160,10 +164,9 @@ namespace snmalloc size_t left_over_size = bits::one_at_bit(align_bits); auto left_over = pointer_offset(bigger, left_over_size); - add_block( + add_block( align_bits, - Aal::capptr_bound(left_over, left_over_size), - pagemap); + Aal::capptr_bound(left_over, left_over_size)); check_block(left_over.as_static(), align_bits); } check_block(bigger.as_static(), align_bits + 1); @@ -171,7 +174,7 @@ namespace snmalloc } check_block(first, align_bits); - ranges[align_bits] = get_next(align_bits, first, pagemap); + ranges[align_bits] = get_next(align_bits, first); return first.as_void(); } @@ -180,8 +183,10 @@ namespace snmalloc * Add a range of memory to the address space. * Divides blocks into power of two sizes with natural alignment */ - template - void add_range(CapPtr base, size_t length, Pagemap& pagemap) + template< + SNMALLOC_CONCEPT(ConceptPAL) PAL, + SNMALLOC_CONCEPT(ConceptBackendMeta) Pagemap> + void add_range(CapPtr base, size_t length) { // For start and end that are not chunk sized, we need to // commit the pages to track the allocations. @@ -206,7 +211,7 @@ namespace snmalloc auto b = base.as_static(); check_block(b, align_bits); - add_block(align_bits, b, pagemap); + add_block(align_bits, b); base = pointer_offset(base, align); length -= align; @@ -238,8 +243,10 @@ namespace snmalloc * part of satisfying the request will be registered with the provided * arena_map for use in subsequent amplification. */ - template - CapPtr reserve(size_t size, Pagemap& pagemap) + template< + SNMALLOC_CONCEPT(ConceptPAL) PAL, + SNMALLOC_CONCEPT(ConceptBackendMeta) Pagemap> + CapPtr reserve(size_t size) { #ifdef SNMALLOC_TRACING std::cout << "ASM Core reserve request:" << size << std::endl; @@ -248,7 +255,7 @@ namespace snmalloc SNMALLOC_ASSERT(bits::is_pow2(size)); SNMALLOC_ASSERT(size >= sizeof(void*)); - return remove_block(bits::next_pow2_bits(size), pagemap); + return remove_block(bits::next_pow2_bits(size)); } /** @@ -258,8 +265,10 @@ namespace snmalloc * This is useful for allowing the space required for alignment to be * used, by smaller objects. */ - template - CapPtr reserve_with_left_over(size_t size, Pagemap& pagemap) + template< + SNMALLOC_CONCEPT(ConceptPAL) PAL, + SNMALLOC_CONCEPT(ConceptBackendMeta) Pagemap> + CapPtr reserve_with_left_over(size_t size) { SNMALLOC_ASSERT(size >= sizeof(void*)); @@ -267,13 +276,13 @@ namespace snmalloc size_t rsize = bits::next_pow2(size); - auto res = reserve(rsize, pagemap); + auto res = reserve(rsize); if (res != nullptr) { if (rsize > size) { - add_range(pointer_offset(res, size), rsize - size, pagemap); + add_range(pointer_offset(res, size), rsize - size); } } return res; diff --git a/src/backend/backend.h b/src/backend/backend.h index b9b62b9..1837910 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -62,6 +62,38 @@ namespace snmalloc static inline FlatPagemap pagemap; + struct Pagemap + { + /** + * Get the metadata associated with a chunk. + * + * 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_meta_data(address_t p) + { + return pagemap.template get(p); + } + + /** + * Set the metadata associated with a chunk. + */ + SNMALLOC_FAST_PATH + static void set_meta_data(address_t p, size_t size, MetaEntry t) + { + for (address_t a = p; a < p + size; a += MIN_CHUNK_SIZE) + { + pagemap.set(a, t); + } + } + + static void register_range(address_t p, size_t sz) + { + pagemap.register_range(p, sz); + } + }; + public: template static std::enable_if_t init() @@ -77,8 +109,8 @@ namespace snmalloc static_assert(fixed_range_ == fixed_range, "Don't set SFINAE parameter!"); auto [heap_base, heap_length] = pagemap.init(base, length); - address_space.add_range( - CapPtr(heap_base), heap_length, pagemap); + address_space.template add_range( + CapPtr(heap_base), heap_length); } private: @@ -136,14 +168,14 @@ namespace snmalloc auto& local = local_state->local_address_space; #endif - p = local.template reserve_with_left_over(size, pagemap); + p = local.template reserve_with_left_over(size); if (p != nullptr) { return p; } auto refill_size = LOCAL_CACHE_BLOCK; - auto refill = global.template reserve(refill_size, pagemap); + auto refill = global.template reserve(refill_size); if (refill == nullptr) return nullptr; @@ -155,10 +187,10 @@ namespace snmalloc } #endif PAL::template notify_using(refill.unsafe_ptr(), refill_size); - local.template add_range(refill, refill_size, pagemap); + local.template add_range(refill, refill_size); // This should succeed - return local.template reserve_with_left_over(size, pagemap); + return local.template reserve_with_left_over(size); } #ifdef SNMALLOC_CHECK_CLIENT @@ -169,7 +201,7 @@ namespace snmalloc size_t rsize = bits::max(OS_PAGE_SIZE, bits::next_pow2(size)); size_t size_request = rsize * 64; - p = global.template reserve(size_request, pagemap); + p = global.template reserve(size_request); if (p == nullptr) return nullptr; @@ -185,7 +217,7 @@ namespace snmalloc SNMALLOC_ASSERT(!is_meta); #endif - p = global.template reserve_with_left_over(size, pagemap); + p = global.template reserve_with_left_over(size); return p; } @@ -249,37 +281,8 @@ namespace snmalloc } MetaEntry t(meta, remote, sizeclass); - - for (address_t a = address_cast(p); - a < address_cast(pointer_offset(p, size)); - a += MIN_CHUNK_SIZE) - { - pagemap.set(a, t); - } + Pagemap::set_meta_data(address_cast(p), size, t); return {p, meta}; } - - /** - * Get the metadata associated with a chunk. - * - * Set template parameter to true if it not an error - * to access a location that is not backed by a chunk. - */ - template - static const MetaEntry& get_meta_data(address_t p) - { - return pagemap.template get(p); - } - - /** - * Set the metadata associated with a chunk. - */ - static void set_meta_data(address_t p, size_t size, MetaEntry t) - { - for (address_t a = p; a < p + size; a += MIN_CHUNK_SIZE) - { - pagemap.set(a, t); - } - } }; } // namespace snmalloc diff --git a/src/backend/backend_concept.h b/src/backend/backend_concept.h new file mode 100644 index 0000000..cca098b --- /dev/null +++ b/src/backend/backend_concept.h @@ -0,0 +1,57 @@ +#pragma once + +#ifdef __cpp_concepts +# include +# include "../ds/concept.h" + +namespace snmalloc +{ + class MetaEntry; + + /** + * The core of the static pagemap accessor interface: {get,set}_metadata. + * + * get_metadata takes a bool-ean template parameter indicating whether it may + * be accessing memory that is not known to be committed. + */ + template + concept ConceptBackendMeta = + requires(address_t addr, size_t sz, MetaEntry t) + { + { Meta::set_meta_data(addr, sz, t) } -> ConceptSame; + + { Meta::template get_meta_data(addr) } + -> ConceptSame; + + { Meta::template get_meta_data(addr) } + -> ConceptSame; + }; + + /** + * The pagemap can also be told to commit backing storage for a range of + * addresses. This is broken out to a separate concept so that we can + * annotate which functions expect to do this vs. which merely use the core + * interface above. In practice, use ConceptBackendMetaRange (without the + * underscore) below, which combines this and the core concept, above. + */ + template + concept ConceptBackendMeta_Range = + requires(address_t addr, size_t sz) + { + { Meta::register_range(addr, sz) } -> ConceptSame; + }; + + /** + * The full pagemap accessor interface, with all of {get,set}_metadata and + * register_range. Use this to annotate callers that need the full interface + * and use ConceptBackendMeta for callers that merely need {get,set}_metadata, + * but note that the difference is just for humans and not compilers (since + * concept checking is lower bounding and does not constrain the templatized + * code to use only those affordances given by the concept). + */ + template + concept ConceptBackendMetaRange = + ConceptBackendMeta && ConceptBackendMeta_Range; +} // namespace snmalloc + +#endif diff --git a/src/mem/corealloc.h b/src/mem/corealloc.h index b67df18..588ed54 100644 --- a/src/mem/corealloc.h +++ b/src/mem/corealloc.h @@ -377,7 +377,7 @@ namespace snmalloc { auto p = message_queue().peek(); auto& entry = - SharedStateHandle::get_meta_data(snmalloc::address_cast(p)); + SharedStateHandle::Pagemap::get_meta_data(snmalloc::address_cast(p)); auto r = message_queue().dequeue(key_global); @@ -538,7 +538,8 @@ namespace snmalloc SNMALLOC_FAST_PATH void dealloc_local_object(void* p) { - auto entry = SharedStateHandle::get_meta_data(snmalloc::address_cast(p)); + auto entry = + SharedStateHandle::Pagemap::get_meta_data(snmalloc::address_cast(p)); if (likely(dealloc_local_object_fast(entry, p, entropy))) return; @@ -664,8 +665,8 @@ namespace snmalloc { bool need_post = true; // Always going to post, so ignore. auto n = p->atomic_read_next(key_global); - auto& entry = - SharedStateHandle::get_meta_data(snmalloc::address_cast(p)); + auto& entry = SharedStateHandle::Pagemap::get_meta_data( + snmalloc::address_cast(p)); handle_dealloc_remote(entry, p, need_post); p = n; } diff --git a/src/mem/localalloc.h b/src/mem/localalloc.h index df0ce29..3e0b2d2 100644 --- a/src/mem/localalloc.h +++ b/src/mem/localalloc.h @@ -260,7 +260,8 @@ namespace snmalloc std::cout << "Remote dealloc post" << p << " size " << alloc_size(p) << std::endl; #endif - MetaEntry entry = SharedStateHandle::get_meta_data(address_cast(p)); + MetaEntry entry = + SharedStateHandle::Pagemap::get_meta_data(address_cast(p)); local_cache.remote_dealloc_cache.template dealloc( entry.get_remote()->trunc_id(), CapPtr(p), key_global); post_remote_cache(); @@ -472,7 +473,7 @@ namespace snmalloc // in thread local state. const MetaEntry& entry = - SharedStateHandle::get_meta_data(address_cast(p)); + SharedStateHandle::Pagemap::get_meta_data(address_cast(p)); if (likely(local_cache.remote_allocator == entry.get_remote())) { if (likely(CoreAlloc::dealloc_local_object_fast( @@ -582,7 +583,8 @@ 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. - MetaEntry entry = SharedStateHandle::get_meta_data(address_cast(p_raw)); + MetaEntry entry = + SharedStateHandle::Pagemap::get_meta_data(address_cast(p_raw)); if (likely(entry.get_remote() != SharedStateHandle::fake_large_remote)) return sizeclass_to_size(entry.get_sizeclass()); @@ -608,7 +610,8 @@ namespace snmalloc #ifndef SNMALLOC_PASS_THROUGH // TODO bring back the CHERI bits. Wes to review if required. MetaEntry entry = - SharedStateHandle::template get_meta_data(address_cast(p_raw)); + SharedStateHandle::Pagemap::template get_meta_data( + address_cast(p_raw)); auto sizeclass = entry.get_sizeclass(); if (likely(entry.get_remote() != SharedStateHandle::fake_large_remote)) { diff --git a/src/mem/remotecache.h b/src/mem/remotecache.h index a062b9a..a397feb 100644 --- a/src/mem/remotecache.h +++ b/src/mem/remotecache.h @@ -95,7 +95,7 @@ namespace snmalloc { auto [first, last] = list[i].extract_segment(key); MetaEntry entry = - SharedStateHandle::get_meta_data(address_cast(first)); + SharedStateHandle::Pagemap::get_meta_data(address_cast(first)); entry.get_remote()->enqueue(first, last, key); sent_something = true; } @@ -117,7 +117,8 @@ namespace snmalloc // Use the next N bits to spread out remote deallocs in our own // slot. auto r = resend.take(key); - MetaEntry entry = SharedStateHandle::get_meta_data(address_cast(r)); + MetaEntry entry = + SharedStateHandle::Pagemap::get_meta_data(address_cast(r)); auto i = entry.get_remote()->trunc_id(); size_t slot = get_slot(i, post_round); list[slot].add(r, key); diff --git a/src/mem/slaballocator.h b/src/mem/slaballocator.h index 4341ff1..f9886b1 100644 --- a/src/mem/slaballocator.h +++ b/src/mem/slaballocator.h @@ -107,7 +107,8 @@ namespace snmalloc << std::endl; #endif MetaEntry entry{meta, remote, sizeclass}; - SharedStateHandle::set_meta_data(address_cast(slab), slab_size, entry); + SharedStateHandle::Pagemap::set_meta_data( + address_cast(slab), slab_size, entry); return {slab, meta}; }