|
From: Konstantin S. <kon...@gm...> - 2010-01-29 09:39:09
|
And of course, we need 3 annotations:
ANNOTATE_CYCLIC_BARRIER_INIT(obj, n)
ANNOTATE_CYCLIC_BARRIER_WAIT_BEFORE(obj)
ANNOTATE_CYCLIC_BARRIER_WAIT_AFTER(obj)
Here are the relevant bits of tsan code to support them:
------------------------------------------------------------------------------------------
// Support for Cyclic Barrier, e.g. pthread_barrier_t.
// We need to create (barrier_count-1)^2 h-b arcs between
// threads blocking on a barrier. We should not create any h-b arcs
// for two calls to barrier_wait if the barrier was reset between then.
struct CyclicBarrierInfo {
// The value given to barrier_init.
uint32_t barrier_count;
// How many times we may block on this barrier before resetting.
int32_t calls_before_reset;
};
// Maps the barrier pointer to CyclicBarrierInfo.
typedef hash_map<uintptr_t, CyclicBarrierInfo> CyclicBarrierMap;
CyclicBarrierInfo &GetCyclicBarrierInfo(uintptr_t barrier) {
if (cyclic_barrier_map_ == NULL) {
cyclic_barrier_map_ = new CyclicBarrierMap;
}
return (*cyclic_barrier_map_)[barrier];
}
void HandleBarrierInit(uintptr_t barrier, uint32_t n) {
CyclicBarrierInfo &info = GetCyclicBarrierInfo(barrier);
info.barrier_count = n;
info.calls_before_reset = 0;
}
void HandleBarrierWaitBefore(uintptr_t barrier) {
CyclicBarrierInfo &info = GetCyclicBarrierInfo(barrier);
CHECK(info.calls_before_reset >= 0);
if (info.calls_before_reset == 0) {
// We are blocking the first time after reset. Clear the VTS.
info.calls_before_reset = info.barrier_count;
Signaller &signaller = (*signaller_map_)[barrier];
VTS::Delete(signaller.vts);
signaller.vts = NULL;
}
info.calls_before_reset--;
// Signal to all threads that blocked on this barrier.
HandleSignal(barrier);
}
void HandleBarrierWaitAfter(uintptr_t barrier) {
HandleWait(barrier, 0, false);
}
------------------------------------------------------------------------------------------
Here is the report:
==5414== WARNING: Expected data race during write of size 4 at 0xC968EE0:
{{{
==5414== T7 (locks held: {}):
==5414== #0 PositiveTests_CyclicBarrierTest::b1()
/home/kcc/drt/trunk/unittest/posix_tests.cc:726
==5414== #1 MyThread::ThreadBody(MyThread*)
/home/kcc/drt/trunk/unittest/thread_wrappers_pthread.h:328
==5414== #2 ThreadSanitizerStartThread
/home/kcc/drt/trunk/tsan/ts_valgrind_intercepts.c:535
==5414== Concurrent write(s) happened at (OR AFTER) these points:
==5414== T4 (locks held: {}):
==5414== #0 PositiveTests_CyclicBarrierTest::a1()
/home/kcc/drt/trunk/unittest/posix_tests.cc:712
==5414== #1 MyThread::ThreadBody(MyThread*)
/home/kcc/drt/trunk/unittest/thread_wrappers_pthread.h:328
==5414== #2 ThreadSanitizerStartThread
/home/kcc/drt/trunk/tsan/ts_valgrind_intercepts.c:535
==5414== Address 0xC968EE0 is 0 bytes inside data symbol
"_ZN31PositiveTests_CyclicBarrierTest1LE"
==5414== Description: "real race, may be hidden by incorrect implementation
of barrier"
==5414== }}}
--kcc
On Fri, Jan 29, 2010 at 10:15 AM, Konstantin Serebryany <
kon...@gm...> wrote:
>
>
> On Fri, Jan 29, 2010 at 10:12 AM, Bart Van Assche <bva...@ac...>wrote:
>
>> On Thu, Jan 28, 2010 at 8:17 PM, Konstantin Serebryany
>> <kon...@gm...> wrote:
>> > I again forgot that pthread_barrier is cyclic (resettable).
>> > imho this is error prone and dull, but that's what we have.
>> > I think we will need two annotations to fully support it:
>> > // inserted before the actual barrier_init code
>> > ANNOTATE_CYCLIC_BARRIER_INIT(obj, n)
>> > // inserted before the actual barrier_wait code.
>> > ANNOTATE_CYCLIC_BARRIER_WAIT(obj)
>>
>> Hello Konstantin,
>>
>> Why do feel that there is a need to tell a thread-checking tool on
>> beforehand whether a barrier will be used only once or multiple times
>>
>
> There is not such need.
> I think the name should contain 'CYCLIC' to make it clear that the
> annotation supports cyclic barriers.
> It will of course work with non-resettable barriers as well.
>
> --kcc
>
>
>> ?
>>
>> Bart.
>>
>
>
|