From: Dmitry Vyukov Date: Fri, 22 Jun 2018 09:00:13 +0000 (+0200) Subject: sync: fix deficiency in RWMutex race annotations X-Git-Tag: go1.11beta1~27 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f451a318bbc8f8cc6afb5a6d2dcb2234924b6271;p=gostls13.git sync: fix deficiency in RWMutex race annotations Remove unnecessary race.Release annotation from Unlock. For RWMutex we want to establish the following happens-before (HB) edges: 1. Between Unlock and the subsequent Lock. 2. Between Unlock and the subsequent RLock. 3. Between batch of RUnlock's and the subsequent Lock. 1 is provided by Release(&rw.readerSem) in Unlock and Acquire(&rw.readerSem) in Lock. 2 is provided by Release(&rw.readerSem) in Unlock and Acquire(&rw.readerSem) in RLock. 3 is provided by ReleaseMerge(&rw.writerSem) in RUnlock in Acquire(&rw.writerSem) in Lock, since we want to establish HB between a batch of RUnlock's this uses ReleaseMerge instead of Release. Release(&rw.writerSem) in Unlock is simply not needed. FWIW this is also how C++ tsan handles mutexes, not a proof but at least something. Making 2 implementations consistent also simplifies any kind of reasoning against both of them. Since this only affects performance, a reasonable test is not possible. Everything should just continue to work but slightly faster. Credit for discovering this goes to Jamie Liu. Change-Id: Ice37d29ecb7a5faed3f7781c38dd32c7469b2735 Reviewed-on: https://go-review.googlesource.com/120495 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- diff --git a/src/sync/rwmutex.go b/src/sync/rwmutex.go index 9dbebfeed7..16a2f9227c 100644 --- a/src/sync/rwmutex.go +++ b/src/sync/rwmutex.go @@ -114,7 +114,6 @@ func (rw *RWMutex) Unlock() { if race.Enabled { _ = rw.w.state race.Release(unsafe.Pointer(&rw.readerSem)) - race.Release(unsafe.Pointer(&rw.writerSem)) race.Disable() }