From: khizmax Date: Wed, 29 Apr 2015 20:30:19 +0000 (+0300) Subject: TSan exam: fixed flat combining data race; eliminated false positive warnings X-Git-Tag: v2.1.0~245^2~7 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=8169c4e54d1787f659232372e0da27c8265c1de7;p=libcds.git TSan exam: fixed flat combining data race; eliminated false positive warnings --- diff --git a/cds/algo/flat_combining.h b/cds/algo/flat_combining.h index 1a3d02d0..b239ef42 100644 --- a/cds/algo/flat_combining.h +++ b/cds/algo/flat_combining.h @@ -95,7 +95,7 @@ namespace cds { namespace algo { struct publication_record { atomics::atomic nRequest; ///< Request field (depends on data structure) atomics::atomic nState; ///< Record state: inactive, active, removed - unsigned int nAge; ///< Age of the record + atomics::atomic nAge; ///< Age of the record atomics::atomic pNext; ///< Next record in publication list void * pOwner; ///< [internal data] Pointer to \ref kernel object that manages the publication list @@ -338,7 +338,7 @@ namespace cds { namespace algo { void release_record( publication_record_type * pRec ) { assert( pRec->is_done() ); - pRec->nRequest.store( req_EmptyRecord, memory_model::memory_order_relaxed ); + pRec->nRequest.store( req_EmptyRecord, memory_model::memory_order_release ); m_Stat.onReleasePubRecord(); } @@ -565,7 +565,7 @@ namespace cds { namespace algo { { assert( pRec->nState.load( memory_model::memory_order_relaxed ) == inactive ); - pRec->nAge = m_nCount; + pRec->nAge.store( m_nCount, memory_model::memory_order_release ); pRec->nState.store( active, memory_model::memory_order_release ); // Insert record to publication list @@ -673,7 +673,7 @@ namespace cds { namespace algo { switch ( p->nState.load( memory_model::memory_order_acquire )) { case active: if ( p->op() >= req_Operation ) { - p->nAge = nCurAge; + p->nAge.store( nCurAge, memory_model::memory_order_release ); owner.fc_apply( static_cast(p) ); operation_done( *p ); bOpDone = true; @@ -741,7 +741,9 @@ namespace cds { namespace algo { // Thinning publication list publication_record * pPrev = nullptr; for ( publication_record * p = m_pHead; p; ) { - if ( p->nState.load( memory_model::memory_order_acquire ) == active && p->nAge + m_nCompactFactor < nCurAge ) { + if ( p->nState.load( memory_model::memory_order_acquire ) == active + && p->nAge.load( memory_model::memory_order_acquire ) + m_nCompactFactor < nCurAge ) + { if ( pPrev ) { publication_record * pNext = p->pNext.load( memory_model::memory_order_acquire ); if ( pPrev->pNext.compare_exchange_strong( p, pNext, diff --git a/cds/container/fcdeque.h b/cds/container/fcdeque.h index 55dbb33b..9d75864a 100644 --- a/cds/container/fcdeque.h +++ b/cds/container/fcdeque.h @@ -356,6 +356,9 @@ 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 ); @@ -397,12 +400,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 ) { 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() ) { case op_push_front: @@ -451,6 +459,7 @@ 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 3d3d5e4d..0f6aeb4d 100644 --- a/cds/container/fcstack.h +++ b/cds/container/fcstack.h @@ -269,6 +269,8 @@ 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 ); @@ -294,11 +296,14 @@ 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; typedef typename fc_kernel::iterator fc_iterator; for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) { switch ( it->op() ) { @@ -312,6 +317,7 @@ namespace cds { namespace container { break; } } + CDS_TSAN_ANNOTATE_IGNORE_RW_END; } //@endcond diff --git a/cds/intrusive/fcstack.h b/cds/intrusive/fcstack.h index f03dd3fe..3bdf1242 100644 --- a/cds/intrusive/fcstack.h +++ b/cds/intrusive/fcstack.h @@ -247,6 +247,8 @@ 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 ); @@ -269,11 +271,15 @@ 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() ) { @@ -286,6 +292,8 @@ namespace cds { namespace intrusive { break; } } + + CDS_TSAN_ANNOTATE_IGNORE_RW_END; } //@endcond diff --git a/src/hp_gc.cpp b/src/hp_gc.cpp index 15166790..ae97cb3e 100644 --- a/src/hp_gc.cpp +++ b/src/hp_gc.cpp @@ -321,7 +321,15 @@ namespace cds { namespace gc { details::retired_vector& dest = pThis->m_arrRetired; assert( !dest.isFull()); details::retired_vector::iterator itRetired = src.begin(); + + // TSan can issue a warning here: + // read src.m_nSize in src.end() + // write src.m_nSize in src.clear() + // This is false positive since we own hprec + CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN; details::retired_vector::iterator itRetiredEnd = src.end(); + CDS_TSAN_ANNOTATE_IGNORE_READS_END; + while ( itRetired != itRetiredEnd ) { dest.push( *itRetired ); if ( dest.isFull()) { @@ -330,7 +338,11 @@ namespace cds { namespace gc { } ++itRetired; } + + // TSan: write src.m_nSize, see a comment above + CDS_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN; src.clear(); + CDS_TSAN_ANNOTATE_IGNORE_WRITES_END; hprec->m_bFree.store(true, atomics::memory_order_release); hprec->m_idOwner.store( nullThreadId, atomics::memory_order_release );