WIP

Author: mjp41

src/snmalloc/ds_core/cheri.h

@@ -70,7 +70,7 @@ namespace snmalloc
     /*
      * At present we check for the pointer also being the start of an
      * allocation closer to dealloc; for small objects, that happens in
-     * dealloc_local_object_fast, either below or *on the far end of message
+     * dealloc_local_object, either below or *on the far end of message
      * receipt*.  For large objects, it happens below by directly rounding to
      * power of two rather than using the is_start_of_object helper.
      * (XXX This does mean that we might end up threading our remote queue

src/snmalloc/mem/corealloc.h

@@ -357,13 +357,14 @@ namespace snmalloc
     /**
      * Handles the messages in the queue if there are any,
      * and then calls the action.
-     * 
+     *
      * This is designed to provide a fast path common case that simply
      * checks the queue and calls the action if it is empty. If there
      * are messages, then it goes to the slow path, handle_message_queue_slow.
-     * 
+     *
      * This is heavily templated to cause inlining, and passing of arguments on
-     * the stack as often closing over the arguments would cause less good codegen.
+     * the stack as often closing over the arguments would cause less good
+     * codegen.
      */
     template<typename Action, typename... Args>
     SNMALLOC_FAST_PATH decltype(auto)
@@ -485,6 +486,46 @@ namespace snmalloc
         entry.get_remote()->trunc_id(), msg);
     }
 
+    template<typename Domesticator>
+    SNMALLOC_FAST_PATH static auto dealloc_local_objects_fast(
+      capptr::Alloc<RemoteMessage> msg,
+      const PagemapEntry& entry,
+      BackendSlabMetadata* meta,
+      LocalEntropy& entropy,
+      Domesticator domesticate,
+      size_t& bytes_freed)
+    {
+      SNMALLOC_ASSERT(!meta->is_unused());
+
+      snmalloc_check_client(
+        mitigations(sanity_checks),
+        is_start_of_object(entry.get_sizeclass(), address_cast(msg)),
+        "Not deallocating start of an object");
+
+      size_t objsize = sizeclass_full_to_size(entry.get_sizeclass());
+
+      auto [curr, length] = RemoteMessage::template open_free_ring<Config>(
+        msg,
+        objsize,
+        freelist::Object::key_root,
+        meta->as_key_tweak(),
+        domesticate);
+
+      // TODO this should be fixed in a separate PR.
+      bytes_freed += objsize * length;
+
+      // Update the head and the next pointer in the free list.
+      meta->free_queue.append_segment(
+        curr,
+        msg.template as_reinterpret<freelist::Object::T<>>(),
+        length,
+        freelist::Object::key_root,
+        meta->as_key_tweak(),
+        entropy);
+
+      return meta->return_objects(length);
+    }
+
     /*************************************************************************
      * Allocation Code Overview
      *
@@ -879,7 +920,16 @@ namespace snmalloc
     /***************************************************************************
      *   Deallocation code
      *
+     * The main deallocation code is in dealloc.  This, like alloc, takes a
+     * template to initialise the state of the allocator.  It is possible for
+     *the first operation on a thread to be a deallocation.
      *
+     * The algorithm is
+     * - dealloc(void*)
+     *  - If object, originated from this allocator, then
+     *    - dealloc_local_object(void*)
+     *  - Otherwise
+     *    - dealloc_remote(void*)
      ***************************************************************************/
     template<typename CheckInit = CheckInitNoOp>
     SNMALLOC_FAST_PATH void dealloc(void* p_raw)
@@ -935,27 +985,6 @@ namespace snmalloc
     {
       auto meta = entry.get_slab_metadata();
 
-      if (SNMALLOC_LIKELY(dealloc_local_object_fast(p, entry, meta, entropy)))
-        return;
-
-      dealloc_local_object_slow(p, entry, meta);
-    }
-
-    SNMALLOC_FAST_PATH void
-    dealloc_local_object(CapPtr<void, capptr::bounds::Alloc> p)
-    {
-      // PagemapEntry-s seen here are expected to have meaningful Remote
-      // pointers
-      dealloc_local_object(
-        p, Config::Backend::get_metaentry(snmalloc::address_cast(p)));
-    }
-
-    SNMALLOC_FAST_PATH static bool dealloc_local_object_fast(
-      CapPtr<void, capptr::bounds::Alloc> p,
-      const PagemapEntry& entry,
-      BackendSlabMetadata* meta,
-      LocalEntropy& entropy)
-    {
       SNMALLOC_ASSERT(!meta->is_unused());
 
       snmalloc_check_client(
@@ -969,47 +998,10 @@ namespace snmalloc
       meta->free_queue.add(
         cp, freelist::Object::key_root, meta->as_key_tweak(), entropy);
 
-      return SNMALLOC_LIKELY(!meta->return_object());
-    }
-
-    template<typename Domesticator>
-    SNMALLOC_FAST_PATH static auto dealloc_local_objects_fast(
-      capptr::Alloc<RemoteMessage> msg,
-      const PagemapEntry& entry,
-      BackendSlabMetadata* meta,
-      LocalEntropy& entropy,
-      Domesticator domesticate,
-      size_t& bytes_freed)
-    {
-      SNMALLOC_ASSERT(!meta->is_unused());
-
-      snmalloc_check_client(
-        mitigations(sanity_checks),
-        is_start_of_object(entry.get_sizeclass(), address_cast(msg)),
-        "Not deallocating start of an object");
-
-      size_t objsize = sizeclass_full_to_size(entry.get_sizeclass());
-
-      auto [curr, length] = RemoteMessage::template open_free_ring<Config>(
-        msg,
-        objsize,
-        freelist::Object::key_root,
-        meta->as_key_tweak(),
-        domesticate);
-
-      // TODO this should be fixed in a separate PR.
-      bytes_freed += objsize * length;
-
-      // Update the head and the next pointer in the free list.
-      meta->free_queue.append_segment(
-        curr,
-        msg.template as_reinterpret<freelist::Object::T<>>(),
-        length,
-        freelist::Object::key_root,
-        meta->as_key_tweak(),
-        entropy);
+      if (SNMALLOC_LIKELY(!meta->return_object()))
+        return;
 
-      return meta->return_objects(length);
+      dealloc_local_object_slow(p, entry, meta);
     }
 
     /**
@@ -1339,15 +1331,20 @@ namespace snmalloc
 
       for (size_t i = 0; i < NUM_SMALL_SIZECLASSES; i++)
       {
-        // TODO could optimise this, to return the whole list in one append
-        // call.
-        while (!small_fast_free_lists[i].empty())
+        if (small_fast_free_lists[i].empty())
+          continue;
+
+        // All elements should have the same entry.
+        const auto& entry =
+          Config::Backend::get_metaentry(small_fast_free_lists[i].peek());
+        do
         {
+          // return each element
           auto p = small_fast_free_lists[i].take(key, domesticate);
           SNMALLOC_ASSERT(is_start_of_object(
             sizeclass_t::from_small_class(i), address_cast(p)));
-          dealloc_local_object(p.as_void());
-        }
+          dealloc_local_object(p.as_void(), entry);
+        } while (!small_fast_free_lists[i].empty());
       }
 
       auto posted = remote_dealloc_cache.template post<sizeof(Allocator)>(

src/test/func/malloc/malloc.cc

@@ -15,11 +15,7 @@ constexpr int SUCCESS = 0;
 void check_result(size_t size, size_t align, void* p, int err, bool null)
 {
   bool failed = false;
-  EXPECT(
-    (errno == err),
-    "Expected error: {} but got {}",
-    err,
-    errno);
+  EXPECT((errno == err), "Expected error: {} but got {}", err, errno);
   if (null)
   {
     EXPECT(p == nullptr, "Expected null but got {}", p);