Change representation of the SeqSet (#568)

This changes the representation of SeqSet to be doubly linked.  This is
required to enable tracking fully used slabs.

src/snmalloc/backend/backend.h

@@ -25,6 +25,8 @@ namespace snmalloc
 #ifdef __cpp_concepts
     static_assert(IsSlabMeta_Arena<SlabMetadata>);
 #endif
+    static constexpr size_t SizeofMetadata =
+      bits::next_pow2_const(sizeof(SlabMetadata));
 
   public:
     /**
@@ -85,8 +87,7 @@ namespace snmalloc
       SNMALLOC_ASSERT(bits::is_pow2(size));
       SNMALLOC_ASSERT(size >= MIN_CHUNK_SIZE);
 
-      auto meta_cap =
-        local_state.get_meta_range().alloc_range(sizeof(SlabMetadata));
+      auto meta_cap = local_state.get_meta_range().alloc_range(SizeofMetadata);
 
       auto meta = meta_cap.template as_reinterpret<SlabMetadata>().unsafe_ptr();
 
@@ -103,8 +104,7 @@ namespace snmalloc
 #endif
       if (p == nullptr)
       {
-        local_state.get_meta_range().dealloc_range(
-          meta_cap, sizeof(SlabMetadata));
+        local_state.get_meta_range().dealloc_range(meta_cap, SizeofMetadata);
         errno = ENOMEM;
 #ifdef SNMALLOC_TRACING
         message<1024>("Out of memory");
@@ -160,7 +160,7 @@ namespace snmalloc
       capptr::Arena<void> arena = slab_metadata.arena_get(alloc);
 
       local_state.get_meta_range().dealloc_range(
-        capptr::Arena<void>::unsafe_from(&slab_metadata), sizeof(SlabMetadata));
+        capptr::Arena<void>::unsafe_from(&slab_metadata), SizeofMetadata);
 
       local_state.get_object_range()->dealloc_range(arena, size);
     }

src/snmalloc/backend_helpers/buddy.h

@@ -19,9 +19,13 @@ namespace snmalloc
 
     size_t to_index(size_t size)
     {
+      SNMALLOC_ASSERT(size != 0);
+      SNMALLOC_ASSERT(bits::is_pow2(size));
       auto log = snmalloc::bits::next_pow2_bits(size);
-      SNMALLOC_ASSERT(log >= MIN_SIZE_BITS);
-      SNMALLOC_ASSERT(log < MAX_SIZE_BITS);
+      SNMALLOC_ASSERT_MSG(
+        log >= MIN_SIZE_BITS, "Size too big: {} log {}.", size, log);
+      SNMALLOC_ASSERT_MSG(
+        log < MAX_SIZE_BITS, "Size too small: {} log {}.", size, log);
 
       return log - MIN_SIZE_BITS;
     }

src/snmalloc/backend_helpers/defaultpagemapentry.h

@@ -60,9 +60,19 @@ namespace snmalloc
     SNMALLOC_FAST_PATH DefaultPagemapEntryT() = default;
   };
 
+  class StrictProvenanceSlabMetadata
+  : public StrictProvenanceSlabMetadataMixin<
+      FrontendSlabMetadata<StrictProvenanceSlabMetadata>>
+  {};
+
+  class LaxProvenanceSlabMetadata
+  : public LaxProvenanceSlabMetadataMixin<
+      FrontendSlabMetadata<LaxProvenanceSlabMetadata>>
+  {};
+
   using DefaultPagemapEntry = DefaultPagemapEntryT<std::conditional_t<
     backend_strict_provenance,
-    StrictProvenanceSlabMetadataMixin<FrontendSlabMetadata>,
-    LaxProvenanceSlabMetadataMixin<FrontendSlabMetadata>>>;
+    StrictProvenanceSlabMetadata,
+    LaxProvenanceSlabMetadata>>;
 
 } // namespace snmalloc

src/snmalloc/backend_helpers/smallbuddyrange.h

@@ -244,6 +244,7 @@ namespace snmalloc
 
       void dealloc_range(CapPtr<void, ChunkBounds> base, size_t size)
       {
+        SNMALLOC_ASSERT(bits::is_pow2(size));
         add_range(base, size);
       }
     };

src/snmalloc/ds_core/seqset.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "../aal/aal.h"
 #include "../ds_core/ds_core.h"
 
 #include <cstdint>
@@ -10,72 +11,88 @@ namespace snmalloc
   /**
    * Simple sequential set of T.
    *
-   * Linked using the T::next field.
+   * Implemented as a doubly linked cyclic list.
+   * Linked using the T::node field.
    *
    * Can be used in either Fifo or Lifo mode, which is
-   * specified by template parameter.
+   * specified by template parameter to `pop`.
    */
-  template<typename T, bool Fifo = false>
+  template<typename T>
   class SeqSet
   {
+  public:
     /**
-     * This sequence structure is intrusive, in that it requires the use of a
-     * `next` field in the elements it manages, but, unlike some other intrusive
-     * designs, it does not require the use of a `container_of`-like construct,
-     * because its pointers point to the element, not merely the intrusive
-     * member.
-     *
-     * In some cases, the next pointer is provided by a superclass but the list
-     * is templated over the subclass.  The `SeqSet` enforces the invariant that
-     * only instances of the subclass can be added to the list and so can safely
-     * down-cast the type of `.next` to `T*`.  As such, we require only that the
-     * `next` field is a pointer to `T` or some superclass of `T`.
-     * %{
-     */
-    using NextPtr = decltype(std::declval<T>().next);
-    static_assert(
-      std::is_base_of_v<std::remove_pointer_t<NextPtr>, T>,
-      "T->next must be a queue pointer to T");
-    ///@}
-
-    /**
-     * Field representation for Fifo behaviour.
+     * The doubly linked Node.
      */
-    struct FieldFifo
+    class Node
     {
-      NextPtr head{nullptr};
+      Node* next;
+      Node* prev;
+
+      friend class SeqSet;
+
+      constexpr Node(Node* next, Node* prev) : next(next), prev(prev) {}
+
+    public:
+      void invariant()
+      {
+        SNMALLOC_ASSERT(next != nullptr);
+        SNMALLOC_ASSERT(prev != nullptr);
+        SNMALLOC_ASSERT(next->prev == this);
+        SNMALLOC_ASSERT(prev->next == this);
+      }
+
+      void remove()
+      {
+        invariant();
+        next->invariant();
+        prev->invariant();
+        next->prev = prev;
+        prev->next = next;
+        next->invariant();
+        prev->invariant();
+      }
     };
 
+  private:
+    // Cyclic doubly linked list (initially empty)
+    Node head{&head, &head};
+
     /**
-     * Field representation for Lifo behaviour.
+     * Returns the containing object.
      */
-    struct FieldLifo
+    T* containing(Node* n)
     {
-      NextPtr head{nullptr};
-      NextPtr* end{&head};
-    };
+      // We could use -static_cast<ptrdiff_t>(offsetof(T, node)) here but CHERI
+      // compiler complains. So we restrict to first entries only.
+
+      static_assert(offsetof(T, node) == 0);
+
+      return pointer_offset<T>(n, 0);
+    }
 
     /**
-     * Field indirection to actual representation.
-     * Different numbers of fields are required for the
-     * two behaviours.
+     * Gets the doubly linked node for the object.
      */
-    std::conditional_t<Fifo, FieldFifo, FieldLifo> v;
+    Node* get_node(T* t)
+    {
+#ifdef __CHERI_PURE_CAPABILITY__
+      return &__builtin_no_change_bounds(t->node);
+#else
+      return &(t->node);
+#endif
+    }
 
     /**
      * Check for empty
      */
     SNMALLOC_FAST_PATH bool is_empty()
     {
-      if constexpr (Fifo)
-      {
-        return v.head == nullptr;
-      }
-      else
-      {
-        SNMALLOC_ASSERT(v.end != nullptr);
-        return &(v.head) == v.end;
-      }
+      static_assert(
+        std::is_same_v<Node, decltype(std::declval<T>().node)>,
+        "T->node must be Node for T");
+      head.invariant();
+      return head.next == &head;
     }
 
   public:
@@ -89,74 +106,60 @@ namespace snmalloc
      *
      * Assumes queue is non-empty
      */
-    SNMALLOC_FAST_PATH T* pop()
+    SNMALLOC_FAST_PATH T* pop_front()
     {
+      head.invariant();
       SNMALLOC_ASSERT(!this->is_empty());
-      auto result = v.head;
-      if constexpr (Fifo)
-      {
-        v.head = result->next;
-      }
+      auto node = head.next;
+      node->remove();
+      auto result = containing(node);
+      head.invariant();
+      return result;
+    }
+
+    /**
+     * Remove an element from the queue
+     *
+     * Assumes queue is non-empty
+     */
+    SNMALLOC_FAST_PATH T* pop_back()
+    {
+      head.invariant();
+      SNMALLOC_ASSERT(!this->is_empty());
+      auto node = head.prev;
+      node->remove();
+      auto result = containing(node);
+      head.invariant();
+      return result;
+    }
+
+    template<bool is_fifo>
+    SNMALLOC_FAST_PATH T* pop()
+    {
+      head.invariant();
+      if constexpr (is_fifo)
+        return pop_front();
       else
-      {
-        if (&(v.head->next) == v.end)
-          v.end = &(v.head);
-        else
-          v.head = v.head->next;
-      }
-      // This cast is safe if the ->next pointers in all of the objects in the
-      // list are managed by this class because object types are checked on
-      // insertion.
-      return static_cast<T*>(result);
+        return pop_back();
     }
 
     /**
-     * Filter
+     * Applies `f` to all the elements in the set.
      *
-     * Removes all elements that f returns true for.
-     * If f returns true, then filter is not allowed to look at the
-     * object again, and f is responsible for its lifetime.
+     * `f` is allowed to remove the element from the set.
      */
     template<typename Fn>
-    SNMALLOC_FAST_PATH void filter(Fn&& f)
+    SNMALLOC_FAST_PATH void iterate(Fn&& f)
     {
-      // Check for empty case.
-      if (is_empty())
-        return;
+      auto curr = head.next;
+      curr->invariant();
 
-      NextPtr* prev = &(v.head);
-
-      while (true)
+      while (curr != &head)
       {
-        if constexpr (Fifo)
-        {
-          if (*prev == nullptr)
-            break;
-        }
-
-        NextPtr curr = *prev;
-        // Note must read curr->next before calling `f` as `f` is allowed to
-        // mutate that field.
-        NextPtr next = curr->next;
-        if (f(static_cast<T*>(curr)))
-        {
-          // Remove element;
-          *prev = next;
-        }
-        else
-        {
-          // Keep element
-          prev = &(curr->next);
-        }
-        if constexpr (!Fifo)
-        {
-          if (&(curr->next) == v.end)
-            break;
-        }
-      }
-      if constexpr (!Fifo)
-      {
-        v.end = prev;
+        // Read next first, as f may remove curr.
+        auto next = curr->next;
+        f(containing(curr));
+        curr = next;
       }
     }
 
@@ -165,24 +168,24 @@ namespace snmalloc
      */
     SNMALLOC_FAST_PATH void insert(T* item)
     {
-      if constexpr (Fifo)
-      {
-        item->next = v.head;
-        v.head = item;
-      }
-      else
-      {
-        *(v.end) = item;
-        v.end = &(item->next);
-      }
+      auto n = get_node(item);
+
+      n->next = head.next;
+      head.next->prev = n;
+
+      n->prev = &head;
+      head.next = n;
+
+      n->invariant();
+      head.invariant();
     }
 
     /**
      * Peek at next element in the set.
      */
     SNMALLOC_FAST_PATH const T* peek()
     {
-      return static_cast<T*>(v.head);
+      return containing(head.next);
     }
   };
 } // namespace snmalloc