Modified Hazard Pointer SMR to conform C++11 memory model more strictly

diff --git a/cds/gc/details/hp.h b/cds/gc/details/hp.h
index c7065ce74ea4cb21c11def460d9fb19c6599fd4b..8170b8f1aa5ad8da481029db2bf29871a0bd269b 100644 (file)
--- a/cds/gc/details/hp.h
+++ b/cds/gc/details/hp.h
@@ -6,6 +6,7 @@
 #include <cds/algo/atomic.h>
 #include <cds/os/thread.h>
 #include <cds/details/bounded_array.h>
+#include <cds/user_setup/cache_line.h>
 
 #include <cds/gc/details/hp_type.h>
 #include <cds/gc/details/hp_alloc.h>
@@ -148,9 +149,12 @@ namespace cds {
                 other threads have read-only access.
             */
             struct hp_record {
-                hp_allocator<>    m_hzp; ///< array of hazard pointers. Implicit \ref CDS_DEFAULT_ALLOCATOR dependency
+                hp_allocator<>    m_hzp;         ///< array of hazard pointers. Implicit \ref CDS_DEFAULT_ALLOCATOR dependency
                 retired_vector    m_arrRetired ; ///< Retired pointer array
 
+                char padding[cds::c_nCacheLineSize];
+                atomics::atomic<unsigned int> m_nSync; ///< dummy var to introduce synchronizes-with relationship between threads
+
                 /// Ctor
                 hp_record( const cds::gc::hp::GarbageCollector& HzpMgr );    // inline
                 ~hp_record()
@@ -161,6 +165,11 @@ namespace cds {
                 {
                     m_hzp.clear();
                 }
+
+                void sync()
+                {
+                    m_nSync.fetch_add( 1, atomics::memory_order_acq_rel );
+                }
             };
         }    // namespace details
 
@@ -616,6 +625,12 @@ namespace cds {
                 m_HzpManager.Scan( m_pHzpRec );
                 m_HzpManager.HelpScan( m_pHzpRec );
             }
+
+            void sync()
+            {
+                assert( m_pHzpRec != nullptr );
+                m_pHzpRec->sync();
+            }
         };
 
         /// Auto hp_guard.
@@ -701,8 +716,9 @@ namespace cds {
         {}
 
         inline hp_record::hp_record( const cds::gc::hp::GarbageCollector& HzpMgr )
-            : m_hzp( HzpMgr.getHazardPointerCount() ),
-            m_arrRetired( HzpMgr )
+            : m_hzp( HzpMgr.getHazardPointerCount() )
+            , m_arrRetired( HzpMgr )
+            , m_nSync( 0 )
         {}
 
     }}} // namespace gc::hp::details

diff --git a/cds/gc/details/hp_alloc.h b/cds/gc/details/hp_alloc.h
index 888e7159b392bd9a2354931ebc166ca8d30868c9..fb0b0886a0a423f0923ba3c5b38abe31f07c108e 100644 (file)
--- a/cds/gc/details/hp_alloc.h
+++ b/cds/gc/details/hp_alloc.h
@@ -85,7 +85,7 @@ namespace cds {
             /**
                 Clearing has relaxed semantics.
             */
-            void clear( atomics::memory_order order = atomics::memory_order_relaxed ) CDS_NOEXCEPT
+            void clear( atomics::memory_order order = atomics::memory_order_release ) CDS_NOEXCEPT
             {
                 // memory order is not necessary here
                 base_class::store( nullptr, order );

diff --git a/cds/gc/impl/hp_decl.h b/cds/gc/impl/hp_decl.h
index 302829cfe5f01e212851ee5295c582119e4266f3..4625cc06efe11dbfe5b2a5ef158ad76db268fcd7 100644 (file)
--- a/cds/gc/impl/hp_decl.h
+++ b/cds/gc/impl/hp_decl.h
@@ -181,10 +181,7 @@ namespace cds { namespace gc {
                 Can be used for a pointer that cannot be changed concurrently
             */
             template <typename T>
-            T * assign( T * p )
-            {
-                return base_class::operator =(p);
-            }
+            T * assign( T * p );    // inline in hp_impl.h
 
             //@cond
             std::nullptr_t assign( std::nullptr_t )
@@ -261,7 +258,7 @@ namespace cds { namespace gc {
             GuardArray( GuardArray const& ) = delete;
             GuardArray( GuardArray&& ) = delete;
             GuardArray& operator=(GuardArray const&) = delete;
-            GuardArray& operator-(GuardArray&&) = delete;
+            GuardArray& operator=(GuardArray&&) = delete;
             //@endcond
 
             /// Protects a pointer of type \p atomic<T*>
@@ -277,7 +274,7 @@ namespace cds { namespace gc {
                 T pRet;
                 do {
                     pRet = assign( nIndex, toGuard.load(atomics::memory_order_acquire) );
-                } while ( pRet != toGuard.load(atomics::memory_order_relaxed));
+                } while ( pRet != toGuard.load(atomics::memory_order_acquire));
 
                 return pRet;
             }
@@ -305,7 +302,7 @@ namespace cds { namespace gc {
                 T pRet;
                 do {
                     assign( nIndex, f( pRet = toGuard.load(atomics::memory_order_acquire) ));
-                } while ( pRet != toGuard.load(atomics::memory_order_relaxed));
+                } while ( pRet != toGuard.load(atomics::memory_order_acquire));
 
                 return pRet;
             }
@@ -315,11 +312,7 @@ namespace cds { namespace gc {
                 The function equals to a simple assignment, no loop is performed.
             */
             template <typename T>
-            T * assign( size_t nIndex, T * p )
-            {
-                base_class::set(nIndex, p);
-                return p;
-            }
+            T * assign( size_t nIndex, T * p ); // inline in hp_impl.h
 
             /// Store marked pointer \p p to the guard
             /**

diff --git a/cds/gc/impl/hp_impl.h b/cds/gc/impl/hp_impl.h
index d3b85b85dff81353aac8998ed93dfa0aab7ce030..4f30f8b1ec497688165463337948b5ed051151ef 100644 (file)
--- a/cds/gc/impl/hp_impl.h
+++ b/cds/gc/impl/hp_impl.h
@@ -67,6 +67,23 @@ namespace cds { namespace gc {
         cds::threading::getGC<HP>().freeGuard( g );
     }
 
+    template <typename T>
+    inline T * HP::Guard::assign( T * p )
+    {
+        T * pp = base_class::operator =(p);
+        cds::threading::getGC<HP>().sync();
+        return pp;
+    }
+
+    template <size_t Count>
+    template <typename T>
+    inline T * HP::GuardArray<Count>::assign( size_t nIndex, T * p )
+    {
+        base_class::set(nIndex, p);
+        cds::threading::getGC<HP>().sync();
+        return p;
+    }
+
     template <typename T>
     inline void HP::retire( T * p, void (* pFunc)(T *) )
     {

diff --git a/cds/opt/options.h b/cds/opt/options.h
index c893cddb823ee6fc0f62567caa37ca79c6281f32..a14c7054ccd562c4d48c0871465e591c4583c9e6 100644 (file)
--- a/cds/opt/options.h
+++ b/cds/opt/options.h
@@ -605,6 +605,7 @@ namespace opt {
             //@endcond
         };
 
+        //@cond
         /// Totally relaxed memory ordering model (do not use!)
         /**
             In this memory model any memory constraint is equivalent to \p memory_order_relaxed.
@@ -614,15 +615,14 @@ namespace opt {
             See \p opt::memory_model for explanations
         */
         struct total_relaxed_ordering {
-            //@cond
             static const atomics::memory_order memory_order_relaxed    = atomics::memory_order_relaxed;
             static const atomics::memory_order memory_order_consume    = atomics::memory_order_relaxed;
             static const atomics::memory_order memory_order_acquire    = atomics::memory_order_relaxed;
             static const atomics::memory_order memory_order_release    = atomics::memory_order_relaxed;
             static const atomics::memory_order memory_order_acq_rel    = atomics::memory_order_relaxed;
             static const atomics::memory_order memory_order_seq_cst    = atomics::memory_order_relaxed;
-            //@endcond
         };
+        //@endcond
     } // namespace v
 
     /// [type-option] Base type traits option setter

diff --git a/src/hp_gc.cpp b/src/hp_gc.cpp
index 632704810c451e509e18339e8a7feb359887cf99..4d9efec9c10d5d7b25e342ef945a3dcccf45541b 100644 (file)
--- a/src/hp_gc.cpp
+++ b/src/hp_gc.cpp
@@ -117,10 +117,8 @@ namespace cds { namespace gc {
             // No HP records available for reuse
             // Allocate and push a new HP record
             hprec = NewHPRec();
-            hprec->m_idOwner.store( curThreadId, atomics::memory_order_relaxed );
-            hprec->m_bFree.store( false, atomics::memory_order_relaxed );
-
-            atomics::atomic_thread_fence( atomics::memory_order_release );
+            hprec->m_idOwner.store( curThreadId, atomics::memory_order_release );
+            hprec->m_bFree.store( false, atomics::memory_order_release );
 
             hplist_node * pOldHead = m_pListHead.load( atomics::memory_order_acquire );
             do {
@@ -168,6 +166,7 @@ namespace cds { namespace gc {
 
             while ( pNode ) {
                 for ( size_t i = 0; i < m_nHazardPointerCount; ++i ) {
+                    pRec->sync();
                     void * hptr = pNode->m_hzp[i];
                     if ( hptr )
                         plist.push_back( hptr );
@@ -249,6 +248,7 @@ namespace cds { namespace gc {
                 while ( pNode ) {
                     if ( !pNode->m_bFree.load( atomics::memory_order_acquire ) ) {
                         for ( size_t i = 0; i < m_nHazardPointerCount; ++i ) {
+                            pRec->sync();
                             void * hptr = pNode->m_hzp[i];
                             if ( hptr ) {
                                 dummyRetired.m_p = hptr;