From da01d5b4ca79a3b6a3f352486d6b01c681500894 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 16 Jul 2021 10:36:13 +0100 Subject: [PATCH] Remove address space usage from Pagemap. The pagemap allocates it self directly either from * the original fixed address range it is supplied, and returns the remaining space after the pagemap is removed; or * directly allocated from the PAL without using the address space manager. This change in layering is required for the next commit, which imposes the invariant that the pagemap has been committed for all spaced managed by the address space manager. --- src/backend/backend.h | 19 +++++-- src/backend/pagemap.h | 90 +++++++++++++++++++++++--------- src/ds/address.h | 7 +++ src/test/func/pagemap/pagemap.cc | 19 +++++-- 4 files changed, 101 insertions(+), 34 deletions(-) diff --git a/src/backend/backend.h b/src/backend/backend.h index 34541b3..c5eb247 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -50,9 +50,13 @@ namespace snmalloc FlatPagemap pagemap; public: - void init() + template + std::enable_if_t init() { - pagemap.init(&address_space); + static_assert( + fixed_range_ == fixed_range, "Don't set SFINAE parameter!"); + + pagemap.init(); if constexpr (fixed_range) { @@ -60,10 +64,15 @@ namespace snmalloc } } - void init(CapPtr base, size_t length) + template + std::enable_if_t + init(CapPtr base, size_t length) { - address_space.add_range(base, length); - pagemap.init(&address_space, address_cast(base), length); + static_assert( + fixed_range_ == fixed_range, "Don't set SFINAE parameter!"); + + auto [heap_base, heap_length] = pagemap.init(base, length); + address_space.add_range(heap_base, heap_length); if constexpr (!fixed_range) { diff --git a/src/backend/pagemap.h b/src/backend/pagemap.h index 727e3b4..06252d5 100644 --- a/src/backend/pagemap.h +++ b/src/backend/pagemap.h @@ -19,17 +19,30 @@ namespace snmalloc private: static constexpr size_t SHIFT = GRANULARITY_BITS; - // Before init is called will contain a single entry - // that is the default value. This is needed so that - // various calls do not have to check for nullptr. - // free(nullptr) - // and - // malloc_usable_size(nullptr) - // do not require an allocation to have ocurred before - // they are called. + /** + * Before init is called will contain a single entry + * that is the default value. This is needed so that + * various calls do not have to check for nullptr. + * free(nullptr) + * and + * malloc_usable_size(nullptr) + * do not require an allocation to have ocurred before + * they are called. + */ inline static const T default_value{}; + + /** + * The representation of the page map. + * + * Initially a single element to ensure nullptr operations + * work. + */ T* body{const_cast(&default_value)}; + /** + * If `has_bounds` is set, then these should contain the + * bounds of the heap that is being managed by this pagemap. + */ address_t base{0}; size_t size{0}; @@ -51,37 +64,66 @@ namespace snmalloc public: constexpr FlatPagemap() = default; - template - std::enable_if_t init(ASM* a, address_t b, size_t s) + /** + * Initialise the pagemap with bounds. + * + * Returns usable range after pagemap has been allocated + */ + template + std::enable_if_t, size_t>> + init(CapPtr b, size_t s) { static_assert( has_bounds_ == has_bounds, "Don't set SFINAE template parameter!"); #ifdef SNMALLOC_TRACING - std::cout << "Pagemap.init " << (void*)b << " (" << s << ")" << std::endl; + std::cout << "Pagemap.init " << b.unsafe_ptr() << " (" << s << ")" + << std::endl; #endif SNMALLOC_ASSERT(s != 0); + // TODO take account of pagemap size in the calculation of how big it + // needs to be. + // Align the start and end. We won't store for the very ends as they // are not aligned to a chunk boundary. - base = bits::align_up(b, bits::one_at_bit(GRANULARITY_BITS)); - auto end = bits::align_down(b + s, bits::one_at_bit(GRANULARITY_BITS)); - size = end - base; - body = a->template reserve( - bits::next_pow2((size >> SHIFT) * sizeof(T))) - .template as_static() - .unsafe_ptr(); + auto heap_base = pointer_align_up(b, bits::one_at_bit(GRANULARITY_BITS)); + auto end = pointer_align_down( + pointer_offset(b, s), bits::one_at_bit(GRANULARITY_BITS)); + size = pointer_diff(heap_base, end); + + // Put pagemap at start of range. + // TODO CHERI capability bound here! + body = b.as_reinterpret().unsafe_ptr(); + + // Advance by size of pagemap. + // TODO CHERI capability bound here! + heap_base = pointer_align_up( + pointer_offset(b, (size >> SHIFT) * sizeof(T)), + bits::one_at_bit(GRANULARITY_BITS)); + base = address_cast(heap_base); + SNMALLOC_ASSERT( + base == bits::align_up(base, bits::one_at_bit(GRANULARITY_BITS))); + return {heap_base, pointer_diff(heap_base, end)}; } - template - std::enable_if_t init(ASM* a) + /** + * Initialise the pagemap without bounds. + */ + template + std::enable_if_t init() { static_assert( has_bounds_ == has_bounds, "Don't set SFINAE template parameter!"); static constexpr size_t COVERED_BITS = bits::ADDRESS_BITS - GRANULARITY_BITS; static constexpr size_t ENTRIES = bits::one_at_bit(COVERED_BITS); - auto new_body = (a->template reserve(ENTRIES * sizeof(T))) - .template as_static() - .unsafe_ptr(); + + // TODO request additional space, and move to random offset. + + // TODO wasting space if size2 bigger than needed. + auto [new_body_untyped, size2] = + Pal::reserve_at_least(ENTRIES * sizeof(T)); + + auto new_body = reinterpret_cast(new_body_untyped); // Ensure bottom page is committed commit_entry(&new_body[0]); @@ -90,8 +132,6 @@ namespace snmalloc new_body[0] = body[0]; body = new_body; - // TODO this is pretty sparse, should we ignore huge pages for it? - // madvise(body, size, MADV_NOHUGEPAGE); } /** diff --git a/src/ds/address.h b/src/ds/address.h index d46da59..15fc47b 100644 --- a/src/ds/address.h +++ b/src/ds/address.h @@ -194,6 +194,13 @@ namespace snmalloc #endif } + template + inline CapPtr + pointer_align_down(CapPtr p, size_t alignment) + { + return CapPtr(pointer_align_down(p.unsafe_ptr(), alignment)); + } + /** * Align a pointer up to a dynamically specified granularity, which must * be a power of two. diff --git a/src/test/func/pagemap/pagemap.cc b/src/test/func/pagemap/pagemap.cc index 5f02c0e..7758b4f 100644 --- a/src/test/func/pagemap/pagemap.cc +++ b/src/test/func/pagemap/pagemap.cc @@ -76,11 +76,22 @@ void test_pagemap(bool bounded) // Initialise the pagemap if (bounded) { - pagemap_test_bound.init(&address_space, low, high); + auto size = bits::one_at_bit(30); + auto base = address_space.reserve(size); + std::cout << "Fixed base: " << base.unsafe_ptr() << " (" << size << ") " + << " end: " << pointer_offset(base, size).unsafe_ptr() + << std::endl; + auto [heap_base, heap_size] = pagemap_test_bound.init(base, size); + std::cout << "Heap base: " << heap_base.unsafe_ptr() << " (" << heap_size + << ") " + << " end: " << pointer_offset(heap_base, heap_size).unsafe_ptr() + << std::endl; + low = address_cast(heap_base); + high = low + heap_size; } else { - pagemap_test_unbound.init(&address_space); + pagemap_test_unbound.init(); } // Nullptr should still work after init. @@ -95,7 +106,7 @@ void test_pagemap(bool bounded) value.v++; if (value.v == T().v) value = 0; - if ((ptr % (1ULL << 26)) == 0) + if (((ptr - low) % (1ULL << 26)) == 0) std::cout << "." << std::flush; } @@ -110,7 +121,7 @@ void test_pagemap(bool bounded) if (value.v == T().v) value = 0; - if ((ptr % (1ULL << 26)) == 0) + if (((ptr - low) % (1ULL << 26)) == 0) std::cout << "." << std::flush; } std::cout << std::endl;