|
From: Konstantin S. <kon...@gm...> - 2009-06-25 10:19:13
|
On Thu, Jun 25, 2009 at 1:29 PM, Bart Van Assche<bar...@gm...> wrote: > On Thu, Jun 25, 2009 at 11:15 AM, Konstantin Serebryany > <kon...@gm...> wrote: >> >> On Wed, Jun 24, 2009 at 11:11 PM, Bart Van >> Assche<bar...@gm...> wrote: >> > On Fri, May 29, 2009 at 12:58 PM, Konstantin >> > Serebryany<kon...@gm...> wrote: >> >> [changed subject] >> >> >> >> Julian, Bart, >> >> >> >> Do you plan to support annotations (aka client requests) in Helgrind >> >> and DRD in a compatible way (and possibly, in a way compatible with >> >> ThreadSanitizer)? >> >> Something like http://code.google.com/p/google-perftools/source/browse/trunk/src/base/dynamic_annotations.h, >> >> or completely different. >> >> Our experience shows that even a pure-happens-before race detector is >> >> completely useless w/o annotations if your code has lock-less >> >> synchronization and hundreds of benign races. >> > >> > My last set of remarks on the annotations defined in dynamic_annotations.h: >> > * Currently there are two macro's defined for annotating acquiring and >> > releasing an rwlock, namely ANNOTATE_RWLOCK_ACQUIRED(lock, is_w) >> > ANNOTATE_RWLOCK_RELEASED(lock, is_w). The second argument, is_w, is a >> > boolean. Having to specify 1 for a writer lock and 0 for a reader lock >> > makes readability of the annotations subobtimal. Has it already been >> > considered to replace these two macro's by e.g. the following four >> > macro's: ANNOTATE_ACQUIRED_READERLOCK(lock), >> > ANNOTATE_ACQUIRED_WRITERLOCK(lock), ANNOTATE_RELEASED_READERLOCK(lock) >> > and ANNOTATE_RELEASED_WRITERLOCK(lock) ? >> >> Instead of replacing, we can create aliases, e.g. >> #define ANNOTATE_READERLOCK_ACQUIRED(lock) ANNOTATE_RWLOCK_ACQUIRED(lock, 0) > > That's also fine for me. > >> > * The source file thread_wrappers_pthread.h makes it clear that the >> > ANNOTATE_RWLOCK_* macro's are used both for annotating reader-writer >> > locks and for annotating spinlocks. >> ... or any other kind of mutex lock. >> >> > It might be a good idea to define >> > separate annotations for reader-writer locks and for spinlocks. >> >> Why? I don't really see a difference from a race detector's point of view. > > From a data race detector's point of view this indeed doesn't make a > big difference. But defining separate annotations for spinlocks makes > the intention more clear in the annotated source code -- seeing > ANNOTATE_RWLOCK_...() around spinlocks looks a little bit strange to > me. We can do this with aliases, no? > And defining separate annotations would also slightly simplify the > implementation of the rwlock annotations in data race detectors that > already support spinlocks (currently only DRD). You mean, the pthread_spin_lock()? It should be supported directly, not annotated. There is a problem however: pthread_spin_unlock and pthread_spin_init are the same symbol in some libc implementations. That's another story... --kcc > > Bart. > |