Port the existing locking scheme to radix_entry

Need a silly radix_ptr to deal with our std::atomic not accepting (integer-sized) object types. We're also not using entry_dead anywhere. Also results in way more code since we don't have markptr's convenience methods anymore. Appropriate methods will be added when the full locking scheme is implemented.

Port the existing locking scheme to radix_entry
eae77331 · David Benjamin · 514a6275 · eae77331 · eae77331
--- a/include/radix.hh
+++ b/include/radix.hh
@@ -5,7 +5,6 @@
 */
 #include "gc.hh"
-#include "markptr.hh"
 enum { bits_per_level = 6 };
 enum { key_bits = 36 };
@@ -60,10 +59,10 @@ class radix_node;
 */
 enum entry_state {
-  entry_node = 0,
+  entry_unlocked = 0,
-  entry_unlocked = 1,
+  entry_locked = 1,
-  entry_locked = 2,
+  entry_dead = 2,
-  entry_dead = 3,
+  entry_node = 3,
  entry_mask = 3
 };
@@ -72,8 +71,15 @@ class radix_entry {
 public:
  radix_entry()
    : value_(0 | entry_unlocked) { }
+  explicit radix_entry(uptr value)
+    : value_(value) { }
  explicit radix_entry(radix_node *ptr)
-    : value_(reinterpret_cast<uptr>(ptr) | entry_unlocked) { }
+    : value_(reinterpret_cast<uptr>(ptr) | entry_node) {
+    // XXX: This is kinda wonky. Maybe switch the status to
+    // entry_unlocked is ptr is null, make null pass both is_elem()
+    // and is_node().
+    assert(ptr != nullptr);
+  }
  explicit radix_entry(radix_elem *ptr, entry_state state = entry_unlocked)
    : value_(reinterpret_cast<uptr>(ptr) | state) {
    assert(state != entry_node);
@@ -84,11 +90,21 @@ public:
    assert(state != entry_node);
  }
-  uptr state() const { return value_ & entry_mask; }
+  uptr value() const { return value_; }
+  uptr& value() { return value_; }
+  entry_state state() const {
+    return static_cast<entry_state>(value_ & entry_mask);
+  }
  uptr ptr() const { return value_ & ~entry_mask; }
  bool is_node() const { return state() == entry_node; }
  bool is_elem() const { return !is_node(); }
+  bool is_null() const { return ptr() == 0; }
+  // Convenience function
+  radix_entry with_state(entry_state state) {
+    return radix_entry(elem(), state);
+  }
  radix_elem *elem() const {
    assert(is_elem());
@@ -102,6 +118,23 @@ private:
  uptr value_;
 };
+// Our version of std::atomic doesn't work for structs, even if they
+// are integer sized.
+class radix_ptr {
+public:
+  radix_ptr() : ptr_(radix_entry().value()) { }
+  radix_ptr(radix_entry e) : ptr_(e.value()) { }
+  radix_entry load() const { return radix_entry(ptr_.load()); }
+  void store(radix_entry e) { ptr_.store(e.value()); }
+  bool compare_exchange_weak(radix_entry &old, radix_entry val) {
+    return ptr_.compare_exchange_weak(old.value(), val.value());
+  }
+private:
+  static_assert(sizeof(uptr) == sizeof(radix_entry),
+                "radix_entry is a uptr");
+  std::atomic<uptr> ptr_;
+};
 class radix_elem : public rcu_freed {
 private:
  bool deleted_;
@@ -115,10 +148,8 @@ class radix_elem : public rcu_freed {
 };
 struct radix_node : public rcu_freed {
-  markptr<void> ptr[1 << bits_per_level];
+  radix_ptr child[1 << bits_per_level];
  radix_node() : rcu_freed("radix_node") {
-    for (int i = 0; i < sizeof(ptr) / sizeof(ptr[0]); i++)
-      ptr[i] = 0;
  }
  void delete_tree(u32 level);
@@ -153,11 +184,10 @@ struct radix_range {
 };
 struct radix {
-  markptr<void> root_;
+  radix_ptr root_;
  u32 shift_;
-  radix(u32 shift) : root_(0), shift_(shift) {
+  radix(u32 shift) : root_(radix_entry(new radix_node())), shift_(shift) {
-    root_.ptr() = new radix_node();
  }
  ~radix();
  radix_elem* search(u64 key);

--- a/kernel/radix.cc
+++ b/kernel/radix.cc
 #include "crange_arch.hh"
 #include "radix.hh"
-enum { crange_debug = 0 };
+enum { radix_debug = 0 };
-#define dprintf(...) do { if (crange_debug) cprintf(__VA_ARGS__); } while(0)
+#define dprintf(...) do { if (radix_debug) cprintf(__VA_ARGS__); } while(0)
 // Returns the index needed to reach |level| from |level+1|.
 static u32
@@ -17,28 +17,35 @@ index(u64 key, u32 level)
 // Returns the level we stopped at.
 template<class CB>
 u32
-descend(u64 key, markptr<void> *n, u32 level, CB cb, bool create)
+descend(u64 key, radix_ptr *n, u32 level, CB cb, bool create)
 {
  static_assert(key_bits == bits_per_level * radix_levels,
                "for now, we only support exact multiples of bits_per_level");
  assert(n);
-  void *v = n->ptr();
+  radix_entry v = n->load();
-  if (v == 0 && create) {
+  if (v.is_null() && create) {
+    assert(v.state() == entry_unlocked);
    radix_node *new_rn = new radix_node();
-    if (n->ptr().cmpxch_update(&v, (void*) new_rn))
+    radix_entry cur = v;
-      v = new_rn;
+    v = radix_entry(new_rn);
-    else
+    do {
-      delete new_rn;
+      if (!cur.is_null()) {
+        assert(cur.is_node());
+        v = cur;
+        delete new_rn;
+        break;
+      }
+    } while (!n->compare_exchange_weak(cur, v));
  }
  // Node isn't there. Just return.
-  if (v == 0) {
+  if (v.is_null()) {
    return level+1;
  }
-  radix_node *rn = (radix_node*) v;
+  radix_node *rn = v.node();
-  markptr<void> *vptr = &rn->ptr[index(key, level)];
+  radix_ptr *vptr = &rn->child[index(key, level)];
  if (level == 0) {
    cb(vptr);
    return level;
@@ -50,13 +57,16 @@ descend(u64 key, markptr<void> *n, u32 level, CB cb, bool create)
 void
 radix_node::delete_tree(u32 level)
 {
+  // FIXME: Put this in the destructor and stuff. Maybe even make the
+  // dispatch a method of radix_entry. Could even make radix_entry a
+  // unique_ptr-type of thing, but that's probably a bit much.
  for (int i = 0; i < (1<<bits_per_level); i++) {
-    void *child = ptr[i].ptr();
+    radix_entry entry = child[i].load();
-    if (child != nullptr) {
+    if (!entry.is_null()) {
-      if (level > 1)
+      if (entry.is_node())
-        static_cast<radix_node*>(child)->delete_tree(level-1);
+        entry.node()->delete_tree(level-1);
      else
-        static_cast<radix_elem*>(child)->decref();
+        entry.elem()->decref();
    }
  }
  do_gc();
@@ -64,8 +74,8 @@ radix_node::delete_tree(u32 level)
 radix::~radix()
 {
-  void *root = root_.ptr();
+  // FIXME: See above
-  static_cast<radix_node*>(root)->delete_tree(radix_levels);
+  root_.load().node()->delete_tree(radix_levels);
 }
 radix_elem*
@@ -73,8 +83,8 @@ radix::search(u64 key)
 {
  radix_elem *result = 0;
  scoped_gc_epoch gc;
-  descend(key >> shift_, &root_, radix_levels-1, [&result](markptr<void> *v) {
+  descend(key >> shift_, &root_, radix_levels-1, [&result](radix_ptr *v) {
-      result = (radix_elem*) v->ptr().load();
+      result = v->load().elem();
    }, false);
  dprintf("%p: search(%lu) -> %p\n", this, key >> shift_, result);
  return result;
@@ -93,8 +103,8 @@ radix::skip_empty(u64 k) const
  while (next_k < (1UL<<key_bits)) {
    // Does next_k exist?
    // FIXME: evil evil const_cast
-    u32 level = descend(next_k, const_cast<markptr<void>*>(&root_),
+    u32 level = descend(next_k, const_cast<radix_ptr*>(&root_),
-                        radix_levels-1, [](markptr<void> *v){}, false);
+                        radix_levels-1, [](radix_ptr *v){}, false);
    if (level == 0) {
      return next_k;
    }
@@ -110,8 +120,10 @@ radix_range::radix_range(radix *r, u64 start, u64 size)
  : r_(r), start_(start), size_(size)
 {
  for (u64 k = start_; k != start_ + size_; k++) {
-    if (descend(k, &r_->root_, radix_levels-1, [](markptr<void> *v) {
+    if (descend(k, &r_->root_, radix_levels-1, [](radix_ptr *v) {
-          while (!v->mark().xchg(true))
+          radix_entry cur = v->load();
+          while (cur.state() == entry_locked ||
+                 !v->compare_exchange_weak(cur, cur.with_state(entry_locked)))
            ; // spin
        }, true) != 0) {
      panic("radix_range");
@@ -125,8 +137,11 @@ radix_range::~radix_range()
    return;
  for (u64 k = start_; k != start_ + size_; k++) {
-    if (descend(k, &r_->root_, radix_levels-1, [](markptr<void> *v) {
+    if (descend(k, &r_->root_, radix_levels-1, [](radix_ptr *v) {
-          v->mark() = false;
+          radix_entry cur = v->load();
+          do {
+            assert(cur.state() == entry_locked);
+          } while (!v->compare_exchange_weak(cur, cur.with_state(entry_unlocked)));
        }, true) != 0) {
      panic("~radix_range");
    }
@@ -144,14 +159,15 @@ radix_range::replace(u64 start, u64 size, radix_elem *val)
  assert(start + size <= start_ + size_);
  for (u64 k = start; k != start + size; k++) {
-    if (descend(k, &r_->root_, radix_levels-1, [val](markptr<void> *v) {
+    if (descend(k, &r_->root_, radix_levels-1, [val](radix_ptr *v) {
-          void* cur = v->ptr().load();
+          radix_entry cur = v->load();
-          while (!v->ptr().cmpxch_update(&cur, val))
+          do {
-            ; // spin
+            assert(cur.state() == entry_locked);
+          } while (!v->compare_exchange_weak(cur, radix_entry(val, entry_locked)));
          if (val)
            val->incref();
-          if (cur)
+          if (!cur.is_null())
-            ((radix_elem*) cur)->decref();
+            cur.elem()->decref();
        }, true)) {
      panic("radix_range::replace");
    }
@@ -162,8 +178,8 @@ radix_elem*
 radix_iterator::operator*()
 {
  radix_elem *result = 0;
-  descend(k_, (markptr<void>*) &r_->root_, radix_levels-1, [&result](markptr<void> *v) {
+  descend(k_, (radix_ptr*) &r_->root_, radix_levels-1, [&result](radix_ptr *v) {
-      result = (radix_elem*) v->ptr().load();
+      result = v->load().elem();
    }, false);
  return result;
 }
@@ -172,7 +188,7 @@ radix_iterator2::radix_iterator2(const radix* r, u64 k)
  : r_(r), k_(k) {
  dprintf("%p: Made iterator with k = %lu\n", r_, k_);
  if (k_ != ~0ULL) {
-    path_[radix_levels] = r_->root_.ptr().load();
+    path_[radix_levels] = (void*)r_->root_.load().ptr();
    if (!find_first_leaf(radix_levels - 1))
      k_ = ~0ULL;
  }
@@ -205,7 +221,7 @@ radix_iterator2::find_first_leaf(u32 level)
 {
  // Find the first non-empty node after k_ on this level.
  for (u32 idx = index(k_, level); idx < (1<<bits_per_level); idx++) {
-    void *next = node(level+1)->ptr[idx].ptr().load();
+    void *next = (void*) node(level+1)->child[idx].load().ptr();
    if (next != nullptr) {
      if (index(k_, level) != idx) {
        // We had to advance; clear everything this level and under