Fixed missing acquire read in Flat-Combining algorithm (found by TSan)

Removed redundant TSan annotation from FC-based containers

diff --git a/cds/algo/flat_combining/kernel.h b/cds/algo/flat_combining/kernel.h
index 30dcc7dcc20b2818e5764b3535ba8c15d7f2b57a..202293b4f19a14f270be6abfa5388ec8d776e480 100644 (file)
--- a/cds/algo/flat_combining/kernel.h
+++ b/cds/algo/flat_combining/kernel.h
@@ -708,7 +708,7 @@ namespace cds { namespace algo {
                 while ( p ) {
                     switch ( p->nState.load( memory_model::memory_order_acquire )) {
                     case active:
-                        if ( p->op() >= req_Operation ) {
+                        if ( p->op( memory_model::memory_order_acquire ) >= req_Operation ) {
                             p->nAge.store( nCurAge, memory_model::memory_order_relaxed );
                             owner.fc_apply( static_cast<publication_record_type*>( p ));
                             operation_done( *p );
@@ -748,7 +748,7 @@ namespace cds { namespace algo {
                     compact_list( nCurAge );
             }
 
-            bool wait_for_combining( publication_record_type * pRec )
+            bool wait_for_combining( publication_record_type* pRec )
             {
                 m_waitStrategy.prepare( *pRec );
                 m_Stat.onPassiveWait();
@@ -864,4 +864,37 @@ namespace cds { namespace algo {
     } // namespace flat_combining
 }} // namespace cds::algo
 
+/*
+  CppMem model  (http://svr-pes20-cppmem.cl.cam.ac.uk/cppmem/)
+
+  // Combiner thread - slave (waiting) thread
+int main() {
+      atomic_int y = 0;  // pRec->op
+      int x = 0;         // pRec->data
+   {{{
+      { // slave thread (not combiner)
+        // Op data
+        x = 1;
+        // Annotate request (op)
+        y.store(1, release);
+        // Wait while request done
+        y.load(acquire).readsvalue(2);
+        // Read result
+        r2=x;
+      }
+   |||
+      { // Combiner thread
+        // Read request (op)
+        r1=y.load(acquire).readsvalue(1);
+        // Execute request - change request data
+        x = 2;
+        // store "request processed" flag (pRec->op := req_Response)
+        y.store(2, release);
+      }
+   }}};
+   return 0;
+}
+
+*/
+
 #endif // #ifndef CDSLIB_ALGO_FLAT_COMBINING_KERNEL_H

diff --git a/cds/container/fcdeque.h b/cds/container/fcdeque.h
index 3270be7bc7e58f0fa89f1a5b25181a16f267d0f0..d081ee0edb7bb73af4e56ef28cda53f6db1477e7 100644 (file)
--- a/cds/container/fcdeque.h
+++ b/cds/container/fcdeque.h
@@ -381,9 +381,6 @@ namespace cds { namespace container {
         {
             assert( pRec );
 
-            // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
-
             switch ( pRec->op()) {
             case op_push_front:
                 assert( pRec->pValPush );
@@ -425,7 +422,6 @@ namespace cds { namespace container {
                 assert(false);
                 break;
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
@@ -433,11 +429,8 @@ namespace cds { namespace container {
         {
             typedef typename fc_kernel::iterator fc_iterator;
 
-            // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
-
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
-                switch ( it->op()) {
+                switch ( it->op( atomics::memory_order_acquire )) {
                 case op_push_front:
                     if ( itPrev != itEnd
                         && (itPrev->op() == op_pop_front || (m_Deque.empty() && itPrev->op() == op_pop_back)))
@@ -552,7 +545,6 @@ namespace cds { namespace container {
                     break;
                 }
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 

diff --git a/cds/container/fcpriority_queue.h b/cds/container/fcpriority_queue.h
index 83fd323bf920c9aa3f48617ac511a5e1be959f64..97983ff2e7bdaca33ab4028609510b18db37eeae 100644 (file)
--- a/cds/container/fcpriority_queue.h
+++ b/cds/container/fcpriority_queue.h
@@ -280,7 +280,7 @@ namespace cds { namespace container {
             assert( pRec );
 
             // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
 
             switch ( pRec->op()) {
             case op_push:
@@ -308,7 +308,7 @@ namespace cds { namespace container {
                 break;
             }
 
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
     };

diff --git a/cds/container/fcqueue.h b/cds/container/fcqueue.h
index df86bda87d30d35898096e8b39847734e7b53e31..5c5d76478087413fd848233c25690b942a718444 100644 (file)
--- a/cds/container/fcqueue.h
+++ b/cds/container/fcqueue.h
@@ -315,9 +315,6 @@ namespace cds { namespace container {
         {
             assert( pRec );
 
-            // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
-
             switch ( pRec->op()) {
             case op_enq:
                 assert( pRec->pValEnq );
@@ -343,7 +340,6 @@ namespace cds { namespace container {
                 assert(false);
                 break;
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
@@ -351,11 +347,8 @@ namespace cds { namespace container {
         {
             typedef typename fc_kernel::iterator fc_iterator;
 
-            // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
-
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
-                switch ( it->op()) {
+                switch ( it->op( atomics::memory_order_acquire )) {
                 case op_enq:
                 case op_enq_move:
                 case op_deq:
@@ -368,7 +361,6 @@ namespace cds { namespace container {
                     break;
                 }
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 

diff --git a/cds/container/fcstack.h b/cds/container/fcstack.h
index 1b3aa45f7d02962d0177344dedf98b2b93f2d33b..65994be7eeeba487fdb45d5ff71d5c1280394d9b 100644 (file)
--- a/cds/container/fcstack.h
+++ b/cds/container/fcstack.h
@@ -301,8 +301,6 @@ namespace cds { namespace container {
         {
             assert( pRec );
 
-            // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
             switch ( pRec->op()) {
             case op_push:
                 assert( pRec->pValPush );
@@ -331,18 +329,17 @@ namespace cds { namespace container {
                 assert(false);
                 break;
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
         void fc_process( typename fc_kernel::iterator itBegin, typename fc_kernel::iterator itEnd )
         {
             // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
 
             typedef typename fc_kernel::iterator fc_iterator;
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
-                switch ( it->op()) {
+                switch ( it->op( atomics::memory_order_acquire )) {
                 case op_push:
                 case op_push_move:
                 case op_pop:
@@ -353,7 +350,7 @@ namespace cds { namespace container {
                     break;
                 }
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 

diff --git a/cds/intrusive/fcqueue.h b/cds/intrusive/fcqueue.h
index 6fe82547a5c0116145fed4d1b7e17e3676baf7c2..5ec67cd66f6313f95063b0ace6e3bc54e6355b68 100644 (file)
--- a/cds/intrusive/fcqueue.h
+++ b/cds/intrusive/fcqueue.h
@@ -287,7 +287,7 @@ namespace cds { namespace intrusive {
 
             // this function is called under FC mutex, so switch TSan off
             // All TSan warnings are false positive
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
 
             switch ( pRec->op()) {
             case op_enq:
@@ -311,7 +311,7 @@ namespace cds { namespace intrusive {
                 assert(false);
                 break;
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
@@ -319,11 +319,11 @@ namespace cds { namespace intrusive {
         {
             // this function is called under FC mutex, so switch TSan off
             // All TSan warnings are false positive
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
 
             typedef typename fc_kernel::iterator fc_iterator;
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
-                switch ( it->op()) {
+                switch ( it->op( atomics::memory_order_acquire )) {
                 case op_enq:
                 case op_deq:
                     if ( m_Queue.empty()) {
@@ -335,7 +335,7 @@ namespace cds { namespace intrusive {
                     break;
                 }
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
+            //CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 

diff --git a/cds/intrusive/fcstack.h b/cds/intrusive/fcstack.h
index 57d1fbd607608d8c05650c3a49ebd669193abf47..257079b03a758935b36ae96bcb93d0651769cab1 100644 (file)
--- a/cds/intrusive/fcstack.h
+++ b/cds/intrusive/fcstack.h
@@ -259,7 +259,6 @@ namespace cds { namespace intrusive {
             return m_FlatCombining.statistics();
         }
 
-
     public: // flat combining cooperation, not for direct use!
         //@cond
         /// Flat combining supporting function. Do not call it directly!
@@ -272,8 +271,6 @@ namespace cds { namespace intrusive {
         {
             assert( pRec );
 
-            // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
             switch ( pRec->op()) {
             case op_push:
                 assert( pRec->pVal );
@@ -296,18 +293,14 @@ namespace cds { namespace intrusive {
                 assert(false);
                 break;
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
         void fc_process( typename fc_kernel::iterator itBegin, typename fc_kernel::iterator itEnd )
         {
-            // this function is called under FC mutex, so switch TSan off
-            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
-
             typedef typename fc_kernel::iterator fc_iterator;
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
-                switch ( it->op()) {
+                switch ( it->op( atomics::memory_order_acquire )) {
                 case op_push:
                 case op_pop:
                     if ( itPrev != itEnd && collide( *itPrev, *it ))
@@ -317,7 +310,6 @@ namespace cds { namespace intrusive {
                     break;
                 }
             }
-            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond