Use std::atomic also for the tagged ptr.

6 files changed, 49 insertions, 79 deletions

diff --git a/vespamalloc/src/tests/allocfree/linklist.cpp b/vespamalloc/src/tests/allocfree/linklist.cpp
index 5dabdca65d6..8e33b684f3a 100644
--- a/vespamalloc/src/tests/allocfree/linklist.cpp
+++ b/vespamalloc/src/tests/allocfree/linklist.cpp
@@ -60,9 +60,9 @@ List globalList[NumBlocks];
 
 class LinkIn : public Consumer {
 public:
-    LinkIn(List::HeadPtr & list, uint32_t maxQueue, bool inverse);
+    LinkIn(List::AtomicHeadPtr & list, uint32_t maxQueue, bool inverse);
 private:
-    List::HeadPtr & _head;
+    List::AtomicHeadPtr & _head;
     virtual void consume(void * p) {
         List * l((List *) p);
         if ( ! ((l >= &globalList[0]) && (l < &globalList[NumBlocks]))) { abort(); }
@@ -70,7 +70,7 @@ private:
     }
 };
 
-LinkIn::LinkIn(List::HeadPtr & list, uint32_t maxQueue, bool inverse) :
+LinkIn::LinkIn(List::AtomicHeadPtr & list, uint32_t maxQueue, bool inverse) :
     Consumer (maxQueue, inverse),
     _head(list)
 {
@@ -80,10 +80,10 @@ LinkIn::LinkIn(List::HeadPtr & list, uint32_t maxQueue, bool inverse) :
 
 class LinkOut : public Producer {
 public:
-    LinkOut(List::HeadPtr & list, uint32_t cnt, LinkIn &target)
+    LinkOut(List::AtomicHeadPtr & list, uint32_t cnt, LinkIn &target)
         : Producer(cnt, target), _head(list) {}
 private:
-    List::HeadPtr & _head;
+    List::AtomicHeadPtr & _head;
     virtual void * produce()       {
         void *p = List::linkOut(_head);
         List *l((List *)p);
@@ -96,10 +96,10 @@ private:
 
 class LinkInOutAndIn : public ProducerConsumer {
 public:
-    LinkInOutAndIn(List::HeadPtr & list, uint32_t cnt, bool inverse)
+    LinkInOutAndIn(List::AtomicHeadPtr & list, uint32_t cnt, bool inverse)
         : ProducerConsumer(cnt, inverse), _head(list) { }
 private:
-    List::HeadPtr & _head;
+    List::AtomicHeadPtr & _head;
     virtual void * produce()       {
         void *p = List::linkOut(_head);
         List *l((List *)p);
@@ -125,8 +125,7 @@ int Test::Main() {
     ASSERT_EQUAL(1024ul, sizeof(List));
 
     FastOS_ThreadPool      pool(128000);
-    List::HeadPtr    sharedList;
-    sharedList._tag = 1;
+    List::AtomicHeadPtr    sharedList(List::HeadPtr(nullptr, 1));
     fprintf(stderr, "Start populating list\n");
     for (size_t i=0; i < NumBlocks; i++) {
         List * l(&globalList[i]);
@@ -141,7 +140,9 @@ int Test::Main() {
     List *n =  List::linkOut(sharedList);
     ASSERT_TRUE(n == NULL);
 
-    sharedList._tag = 1;
+    List::HeadPtr tmp(sharedList.load());
+    tmp._tag = 1;
+    sharedList.store(tmp);
     fprintf(stderr, "Start populating list\n");
     for (size_t i=0; i < NumBlocks; i++) {
         List * l(&globalList[i]);
diff --git a/vespamalloc/src/tests/test1/testatomic.cpp b/vespamalloc/src/tests/test1/testatomic.cpp
index 8d72e560bf1..78d94429a3f 100644
--- a/vespamalloc/src/tests/test1/testatomic.cpp
+++ b/vespamalloc/src/tests/test1/testatomic.cpp
@@ -110,8 +110,8 @@ int Test::Main()
         ASSERT_TRUE(uint64V.is_lock_free());
     }
     {
-        std::atomic<vespamalloc::Atomic::TaggedPtr> taggedPtr;
-        ASSERT_EQUAL(16, sizeof(vespamalloc::Atomic::TaggedPtr));
+        std::atomic<vespamalloc::TaggedPtr> taggedPtr;
+        ASSERT_EQUAL(16, sizeof(vespamalloc::TaggedPtr));
         ASSERT_TRUE(taggedPtr.is_lock_free());
     }
 
diff --git a/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp b/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp
index f9cb104169a..12f680bac88 100644
--- a/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp
+++ b/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp
@@ -4,35 +4,35 @@
 namespace vespamalloc {
 
 
-void AFListBase::linkInList(HeadPtr & head, AFListBase * list)
+void AFListBase::linkInList(AtomicHeadPtr & head, AFListBase * list)
 {
     AFListBase * tail;
     for (tail = list; tail->_next != NULL ;tail = tail->_next) { }
     linkIn(head, list, tail);
 }
 
-void AFListBase::linkIn(HeadPtr & head, AFListBase * csl, AFListBase * tail)
+void AFListBase::linkIn(AtomicHeadPtr & head, AFListBase * csl, AFListBase * tail)
 {
-    HeadPtr oldHead = head;
+    HeadPtr oldHead = head.load(std::memory_order_relaxed);
     HeadPtr newHead(csl, oldHead._tag + 1);
     tail->_next = static_cast<AFListBase *>(oldHead._ptr);
-    while ( ! Atomic::cmpSwap(&head, newHead, oldHead) ) {
-        oldHead = head;
+    while ( ! head.compare_exchange_weak(oldHead, newHead, std::memory_order_release, std::memory_order_relaxed) ) {
+        oldHead = head.load(std::memory_order_relaxed);
         newHead._tag =  oldHead._tag + 1;
         tail->_next = static_cast<AFListBase *>(oldHead._ptr);
     }
 }
 
-AFListBase * AFListBase::linkOut(HeadPtr & head)
+AFListBase * AFListBase::linkOut(AtomicHeadPtr & head)
 {
-    HeadPtr oldHead = head;
+    HeadPtr oldHead = head.load(std::memory_order_relaxed);
     AFListBase *csl = static_cast<AFListBase *>(oldHead._ptr);
     if (csl == NULL) {
         return NULL;
     }
     HeadPtr newHead(csl->_next, oldHead._tag + 1);
-    while ( ! Atomic::cmpSwap(&head, newHead, oldHead) ) {
-        oldHead = head;
+    while ( ! head.compare_exchange_weak(oldHead, newHead, std::memory_order_release, std::memory_order_relaxed) ) {
+        oldHead = head.load(std::memory_order_relaxed);
         csl = static_cast<AFListBase *>(oldHead._ptr);
         if (csl == NULL) {
             return NULL;
diff --git a/vespamalloc/src/vespamalloc/malloc/allocchunk.h b/vespamalloc/src/vespamalloc/malloc/allocchunk.h
index fb3defa64bb..6db29678b3d 100644
--- a/vespamalloc/src/vespamalloc/malloc/allocchunk.h
+++ b/vespamalloc/src/vespamalloc/malloc/allocchunk.h
@@ -6,68 +6,37 @@
 
 namespace vespamalloc {
 
-#define ATOMIC_TAGGEDPTR_ALIGNMENT __attribute__ ((aligned (16)))
-
 /**
- * Copied from vespalib to avoid code dependencies.
- */
-class Atomic {
-public:
-    /**
-     * @brief Pointer and tag - use instead of bare pointer for cmpSwap()
-     *
-     * When making a lock-free data structure by using cmpSwap
-     * on pointers, you'll often run into the "ABA problem", see
-     * http://en.wikipedia.org/wiki/ABA_problem for details.
-     * The TaggedPtr makes it easy to do the woraround with tag bits,
-     * but requires the double-word compare-and-swap instruction.
-     * Very early Amd K7/8 CPUs are lacking this and will fail (Illegal Instruction).
-     **/
-    struct TaggedPtr {
-        TaggedPtr() noexcept : _ptr(nullptr), _tag(0) { }
-        TaggedPtr(void *h, size_t t) noexcept : _ptr(h), _tag(t) {}
+ * @brief Pointer and tag - use instead of bare pointer for cmpSwap()
+ *
+ * When making a lock-free data structure by using cmpSwap
+ * on pointers, you'll often run into the "ABA problem", see
+ * http://en.wikipedia.org/wiki/ABA_problem for details.
+ * The TaggedPtr makes it easy to do the woraround with tag bits,
+ * but requires the double-word compare-and-swap instruction.
+ * Very early Amd K7/8 CPUs are lacking this and will fail (Illegal Instruction).
+ **/
+struct TaggedPtr {
+    TaggedPtr() noexcept : _ptr(nullptr), _tag(0) { }
+    TaggedPtr(void *h, size_t t) noexcept : _ptr(h), _tag(t) {}
 
-        void *_ptr;
-        size_t _tag;
-    };
-
-    static bool cmpSwap(volatile TaggedPtr *dest, TaggedPtr newVal, TaggedPtr oldVal) {
-        char result;
-        void *ptr;
-        size_t tag;
-#if defined(__x86_64__)
-        __asm__ volatile ("lock ;"
-                          "cmpxchg16b %8;"
-                          "setz %1;"
-                          : "=m" (*dest),
-                            "=q" (result),
-                            "=a" (ptr),
-                            "=d" (tag)
-                          : "a" (oldVal._ptr),
-                            "d" (oldVal._tag),
-                            "b" (newVal._ptr),
-                            "c" (newVal._tag),
-                            "m" (*dest)
-                          : "memory");
-#else
-#error "Only supports X86_64"
-#endif
-        return result;
-    }
+    void *_ptr;
+    size_t _tag;
 };
 
 class AFListBase
 {
 public:
-    using HeadPtr = Atomic::TaggedPtr;
+    using HeadPtr = TaggedPtr;
+    using AtomicHeadPtr = std::atomic<HeadPtr>;
     AFListBase() : _next(NULL) { }
     void setNext(AFListBase * csl)           { _next = csl; }
     static void init();
-    static void linkInList(HeadPtr & head, AFListBase * list);
-    static void linkIn(HeadPtr & head, AFListBase * csl, AFListBase * tail);
+    static void linkInList(AtomicHeadPtr & head, AFListBase * list);
+    static void linkIn(AtomicHeadPtr & head, AFListBase * csl, AFListBase * tail);
 protected:
     AFListBase * getNext()                      { return _next; }
-    static AFListBase * linkOut(HeadPtr & head);
+    static AFListBase * linkOut(AtomicHeadPtr & head);
 private:
     AFListBase       *_next;
 };
@@ -95,7 +64,7 @@ public:
     bool full()                 const { return (_count == NumBlocks); }
     size_t fill(void * mem, SizeClassT sc, size_t blocksPerChunk = NumBlocks);
     AFList * getNext()                { return static_cast<AFList *>(AFListBase::getNext()); }
-    static AFList * linkOut(HeadPtr & head) {
+    static AFList * linkOut(AtomicHeadPtr & head) {
         return static_cast<AFList *>(AFListBase::linkOut(head));
     }
 private:
diff --git a/vespamalloc/src/vespamalloc/malloc/globalpool.h b/vespamalloc/src/vespamalloc/malloc/globalpool.h
index 41b1418ccbf..a8dc7539d00 100644
--- a/vespamalloc/src/vespamalloc/malloc/globalpool.h
+++ b/vespamalloc/src/vespamalloc/malloc/globalpool.h
@@ -46,8 +46,8 @@ private:
     {
     public:
         AllocFree() : _full(), _empty() { }
-        typename ChunkSList::HeadPtr _full;
-        typename ChunkSList::HeadPtr _empty;
+        typename ChunkSList::AtomicHeadPtr _full;
+        typename ChunkSList::AtomicHeadPtr _empty;
     };
     class Stat
     {
@@ -73,7 +73,7 @@ private:
 
     Mutex                       _mutex;
     ChunkSList                * _chunkPool;
-    AllocFree                   _scList[NUM_SIZE_CLASSES] ATOMIC_TAGGEDPTR_ALIGNMENT;
+    AllocFree                   _scList[NUM_SIZE_CLASSES];
     DataSegment<MemBlockPtrT> & _dataSegment;
     std::atomic<size_t>         _getChunks;
     size_t                      _getChunksSum;
diff --git a/vespamalloc/src/vespamalloc/malloc/globalpool.hpp b/vespamalloc/src/vespamalloc/malloc/globalpool.hpp
index 153fd131f66..f1a7959bba6 100644
--- a/vespamalloc/src/vespamalloc/malloc/globalpool.hpp
+++ b/vespamalloc/src/vespamalloc/malloc/globalpool.hpp
@@ -38,11 +38,11 @@ template <typename MemBlockPtrT>
 typename AllocPoolT<MemBlockPtrT>::ChunkSList *
 AllocPoolT<MemBlockPtrT>::getFree(SizeClassT sc)
 {
-    typename ChunkSList::HeadPtr & empty = _scList[sc]._empty;
+    typename ChunkSList::AtomicHeadPtr & empty = _scList[sc]._empty;
     ChunkSList * csl(NULL);
     while ((csl = ChunkSList::linkOut(empty)) == NULL) {
         Guard sync(_mutex);
-        if (empty._ptr == NULL) {
+        if (empty.load(std::memory_order_relaxed)._ptr == NULL) {
             ChunkSList * ncsl(getChunks(sync, 1));
             if (ncsl) {
                 ChunkSList::linkInList(empty, ncsl);
@@ -61,10 +61,10 @@ typename AllocPoolT<MemBlockPtrT>::ChunkSList *
 AllocPoolT<MemBlockPtrT>::getAlloc(SizeClassT sc)
 {
     ChunkSList * csl(NULL);
-    typename ChunkSList::HeadPtr & full = _scList[sc]._full;
+    typename ChunkSList::AtomicHeadPtr & full = _scList[sc]._full;
     while ((csl = ChunkSList::linkOut(full)) == NULL) {
         Guard sync(_mutex);
-        if (full._ptr == NULL) {
+        if (full.load(std::memory_order_relaxed)._ptr == NULL) {
             ChunkSList * ncsl(malloc(sync, sc));
             if (ncsl) {
                 ChunkSList::linkInList(full, ncsl);