Commit c0f14991 authored by Kirill Smelkov's avatar Kirill Smelkov

tracing: Part 3 - Silence race-detector about probe.Detach

Race-detector does not know Probe.Detach works under world stopped and
that this way it cannot break consistency of probes list attached to a
trace event - on event signalling either a probe will be run or not run
at all.

And we do not mind that e.g. while Detach was in progress a probe was
read from traceevent list and decided to be run and the probe
function was actually called just after Detach finished.

For this reason tell race-detector to not take into account all memory
read/write that are performed while the world is stopped.

If we do not it complains e.g. this way:

    ==================
    WARNING: DATA RACE
    Read at 0x00c42000d760 by goroutine 7:
      lab.nexedi.com/kirr/neo/go/zodb/storage._traceCacheGCFinish_run()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/xcommon/tracing/tracing.go:265 +0x81
      lab.nexedi.com/kirr/neo/go/zodb/storage.traceCacheGCFinish()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/ztrace.go:22 +0x63
      lab.nexedi.com/kirr/neo/go/zodb/storage.(*Cache).gc()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/cache.go:497 +0x62c
      lab.nexedi.com/kirr/neo/go/zodb/storage.(*Cache).gcmain()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/cache.go:478 +0x4c

    Previous write at 0x00c42000d760 by goroutine 6:
      lab.nexedi.com/kirr/neo/go/xcommon/tracing.(*Probe).Detach()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/xcommon/tracing/tracing.go:319 +0x103
      lab.nexedi.com/kirr/neo/go/xcommon/tracing.(*ProbeGroup).Done()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/xcommon/tracing/tracing.go:344 +0xa5
      lab.nexedi.com/kirr/neo/go/zodb/storage.TestCache()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/cache_test.go:576 +0x7f94
      testing.tRunner()
          /home/kirr/src/tools/go/go/src/testing/testing.go:746 +0x16c

    Goroutine 7 (running) created at:
      lab.nexedi.com/kirr/neo/go/zodb/storage.NewCache()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/cache.go:129 +0x227
      lab.nexedi.com/kirr/neo/go/zodb/storage.TestCache()
          /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/cache_test.go:165 +0x7b1
      testing.tRunner()
          /home/kirr/src/tools/go/go/src/testing/testing.go:746 +0x16c

    Goroutine 6 (finished) created at:
      testing.(*T).Run()
          /home/kirr/src/tools/go/go/src/testing/testing.go:789 +0x568
      testing.runTests.func1()
          /home/kirr/src/tools/go/go/src/testing/testing.go:1004 +0xa7
      testing.tRunner()
          /home/kirr/src/tools/go/go/src/testing/testing.go:746 +0x16c
      testing.runTests()
          /home/kirr/src/tools/go/go/src/testing/testing.go:1002 +0x521
      testing.(*M).Run()
          /home/kirr/src/tools/go/go/src/testing/testing.go:921 +0x206
      main.main()
          lab.nexedi.com/kirr/neo/go/zodb/storage/_test/_testmain.go:44 +0x1d3
    ==================
parent d89b1be1
// Copyright (C) 2017 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com>
//
// This program is free software: you can Use, Study, Modify and Redistribute
// it under the terms of the GNU General Public License version 3, or (at your
// option) any later version, as published by the Free Software Foundation.
//
// You can also Link and Combine this program with other software covered by
// the terms of any of the Free Software licenses or any of the Open Source
// Initiative approved licenses and Convey the resulting work. Corresponding
// source of such a combination shall include the source code for all other
// software used.
//
// This program is distributed WITHOUT ANY WARRANTY; without even the implied
// warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
//
// See COPYING file for full licensing terms.
// See https://www.nexedi.com/licensing for rationale and options.
// +build race
package race
import "unsafe"
/*
// symbols are e.g. in go/src/runtime/race/race_linux_amd64.syso
#cgo LDFLAGS: -Wl,--unresolved-symbols=ignore-in-object-files
// __tsan::ThreadIgnoreBegin(__tsan::ThreadState*, unsigned long)
// __tsan::ThreadIgnoreEnd(__tsan::ThreadState*, unsigned long)
extern void _ZN6__tsan17ThreadIgnoreBeginEPNS_11ThreadStateEm(void *, unsigned long);
extern void _ZN6__tsan15ThreadIgnoreEndEPNS_11ThreadStateEm(void *, unsigned long);
*/
import "C"
// Ways to tell race-detector to ignore "read/write" events from current thread.
// NOTE runtime.RaceDisable disables only "sync" part, not "read/write".
func IgnoreBegin(racectx uintptr) {
C._ZN6__tsan17ThreadIgnoreBeginEPNS_11ThreadStateEm(unsafe.Pointer(racectx), 0)
}
func IgnoreEnd(racectx uintptr) {
C._ZN6__tsan15ThreadIgnoreEndEPNS_11ThreadStateEm(unsafe.Pointer(racectx), 0)
}
// Copyright (C) 2017 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com>
//
// This program is free software: you can Use, Study, Modify and Redistribute
// it under the terms of the GNU General Public License version 3, or (at your
// option) any later version, as published by the Free Software Foundation.
//
// You can also Link and Combine this program with other software covered by
// the terms of any of the Free Software licenses or any of the Open Source
// Initiative approved licenses and Convey the resulting work. Corresponding
// source of such a combination shall include the source code for all other
// software used.
//
// This program is distributed WITHOUT ANY WARRANTY; without even the implied
// warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
//
// See COPYING file for full licensing terms.
// See https://www.nexedi.com/licensing for rationale and options.
// +build !race
package race
// race ignore begin/end stubs
func IgnoreBegin(racectx uintptr) {}
func IgnoreEnd(racectx uintptr) {}
#!/bin/bash
# g_typedef -- generate type definition of runtime.g
goexec= # set by main driver to current go compiler
gover= # go<maj>.<min> version (without .patch)
gomaj= # go<maj>
gomin= # <min>
govern= # e.g. 109 for go1.9
# goset <goexec> - set <goexec> as current go
goset() {
goexec=$1
# go1.1 go1.2 go1.3 go1.4 go1.5 go1.6 go1.7 go1.8 go1.9 go1.10 -> go1.10
gover=`$goexec list -f '{{ range context.ReleaseTags }} {{ .}}{{end}}' runtime |awk '{print $NF}'`
IFS=. read gomaj gomin < <(echo "$gover")
govern=$((${gomaj#go} * 100 + $gomin))
}
# typedef <type> - print type definition
typedef() {
$goexec doc -c -u $1 |sed -n -e '/^type /,/^}/p'
}
# typedef_g - print <g> & friends definitions
typedef_g() {
typedef runtime.g
typedef runtime.stack
typedef runtime._panic
typedef runtime._defer
typedef runtime.gobuf
typedef runtime.funcval
#typedef runtime.sudog
#typedef runtime.hchan
typedef runtime.timer
#typedef runtime.guintptr
#typedef runtime.m
if (( $govern < 109 )); then
typedef runtime.stkbar
fi
}
# typedef_g_fixed - print adjusted <g> & friends definitions
typedef_g_fixed() {
typedef_g $1 | \
sed -e 's/\<sys.Uintreg\>/uintreg/'
echo "type guintptr uintptr // XXX stub"
echo "type puintptr uintptr // XXX stub"
echo "type uintreg uint // FIXME wrong on amd64p32"
echo "type m struct {} // FIXME stub"
echo "type sudog struct {} // FIXME stub"
}
# gen_zruntime - generate zruntime_g_<gover>.go
gen_zruntime() {
out="zruntime_g_$gover.go"
echo >$out "// Code generated by g_typedef; DO NOT EDIT."
echo >>$out
echo >>$out "// +build $gover,!$gomaj.$((gomin+1))"
echo >>$out
echo >>$out "package xruntime"
echo >>$out
echo >>$out 'import "unsafe"'
echo >>$out
typedef_g_fixed $go >>$out
}
# main driver
gov="go18 go"
for g in $gov; do
goset $g
gen_zruntime
done
// Copyright (C) 2017 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com>
//
// This program is free software: you can Use, Study, Modify and Redistribute
// it under the terms of the GNU General Public License version 3, or (at your
// option) any later version, as published by the Free Software Foundation.
//
// You can also Link and Combine this program with other software covered by
// the terms of any of the Free Software licenses or any of the Open Source
// Initiative approved licenses and Convey the resulting work. Corresponding
// source of such a combination shall include the source code for all other
// software used.
//
// This program is distributed WITHOUT ANY WARRANTY; without even the implied
// warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
//
// See COPYING file for full licensing terms.
// See https://www.nexedi.com/licensing for rationale and options.
// +build race
package xruntime
import "lab.nexedi.com/kirr/go123/tracing/internal/race"
// RaceIgnoreBegin instructs race-detector to ignore memory read/write events from current goroutine.
//
// The events will be back to handled after call to RaceIgnoreEnd.
//
// NOTE runtime.RaceDisable disables "sync" events, not memory read/write.
func RaceIgnoreBegin() {
race.IgnoreBegin(getg().racectx)
}
// RaceIgnoreEnd instructs race-detector to stop ignoring memory read/write events from current goroutine.
//
// NOTE runtime RaceEnable enables "sync" events, not memory read/write.
func RaceIgnoreEnd() {
race.IgnoreEnd(getg().racectx)
}
// Copyright (C) 2017 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com>
//
// This program is free software: you can Use, Study, Modify and Redistribute
// it under the terms of the GNU General Public License version 3, or (at your
// option) any later version, as published by the Free Software Foundation.
//
// You can also Link and Combine this program with other software covered by
// the terms of any of the Free Software licenses or any of the Open Source
// Initiative approved licenses and Convey the resulting work. Corresponding
// source of such a combination shall include the source code for all other
// software used.
//
// This program is distributed WITHOUT ANY WARRANTY; without even the implied
// warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
//
// See COPYING file for full licensing terms.
// See https://www.nexedi.com/licensing for rationale and options.
// +build !race
package xruntime
// empty stubs for Race*()
func RaceIgnoreBegin() {}
func RaceIgnoreEnd() {}
// Copyright (C) 2017 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com>
//
// This program is free software: you can Use, Study, Modify and Redistribute
// it under the terms of the GNU General Public License version 3, or (at your
// option) any later version, as published by the Free Software Foundation.
//
// You can also Link and Combine this program with other software covered by
// the terms of any of the Free Software licenses or any of the Open Source
// Initiative approved licenses and Convey the resulting work. Corresponding
// source of such a combination shall include the source code for all other
// software used.
//
// This program is distributed WITHOUT ANY WARRANTY; without even the implied
// warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
//
// See COPYING file for full licensing terms.
// See https://www.nexedi.com/licensing for rationale and options.
package xruntime
//go:generate ./g_typedef
// getg returns pointer to current goroutine descriptor.
func getg() *g
#include "textflag.h"
// +build 386
// func getg() *g
TEXT ·getg(SB),NOSPLIT,$0-8
MOVL (TLS), AX
MOVL AX, ret+0(FP)
RET
#include "textflag.h"
// +build amd64 amd64p
// func getg() *g
TEXT ·getg(SB),NOSPLIT,$0-8
MOVQ (TLS), R14
MOVQ R14, ret+0(FP)
RET
// Code generated by g_typedef; DO NOT EDIT.
// +build go1.8,!go1.9
package xruntime
import "unsafe"
type g struct {
// Stack parameters.
// stack describes the actual stack memory: [stack.lo, stack.hi).
// stackguard0 is the stack pointer compared in the Go stack growth prologue.
// It is stack.lo+StackGuard normally, but can be StackPreempt to trigger a preemption.
// stackguard1 is the stack pointer compared in the C stack growth prologue.
// It is stack.lo+StackGuard on g0 and gsignal stacks.
// It is ~0 on other goroutine stacks, to trigger a call to morestackc (and crash).
stack stack // offset known to runtime/cgo
stackguard0 uintptr // offset known to liblink
stackguard1 uintptr // offset known to liblink
_panic *_panic // innermost panic - offset known to liblink
_defer *_defer // innermost defer
m *m // current m; offset known to arm liblink
stackAlloc uintptr // stack allocation is [stack.lo,stack.lo+stackAlloc)
sched gobuf
syscallsp uintptr // if status==Gsyscall, syscallsp = sched.sp to use during gc
syscallpc uintptr // if status==Gsyscall, syscallpc = sched.pc to use during gc
stkbar []stkbar // stack barriers, from low to high (see top of mstkbar.go)
stkbarPos uintptr // index of lowest stack barrier not hit
stktopsp uintptr // expected sp at top of stack, to check in traceback
param unsafe.Pointer // passed parameter on wakeup
atomicstatus uint32
stackLock uint32 // sigprof/scang lock; TODO: fold in to atomicstatus
goid int64
waitsince int64 // approx time when the g become blocked
waitreason string // if status==Gwaiting
schedlink guintptr
preempt bool // preemption signal, duplicates stackguard0 = stackpreempt
paniconfault bool // panic (instead of crash) on unexpected fault address
preemptscan bool // preempted g does scan for gc
gcscandone bool // g has scanned stack; protected by _Gscan bit in status
gcscanvalid bool // false at start of gc cycle, true if G has not run since last scan; transition from true to false by calling queueRescan and false to true by calling dequeueRescan
throwsplit bool // must not split stack
raceignore int8 // ignore race detection events
sysblocktraced bool // StartTrace has emitted EvGoInSyscall about this goroutine
sysexitticks int64 // cputicks when syscall has returned (for tracing)
traceseq uint64 // trace event sequencer
tracelastp puintptr // last P emitted an event for this goroutine
lockedm *m
sig uint32
writebuf []byte
sigcode0 uintptr
sigcode1 uintptr
sigpc uintptr
gopc uintptr // pc of go statement that created this goroutine
startpc uintptr // pc of goroutine function
racectx uintptr
waiting *sudog // sudog structures this g is waiting on (that have a valid elem ptr); in lock order
cgoCtxt []uintptr // cgo traceback context
// gcRescan is this G's index in work.rescan.list. If this is
// -1, this G is not on the rescan list.
//
// If gcphase != _GCoff and this G is visible to the garbage
// collector, writes to this are protected by work.rescan.lock.
gcRescan int32
// gcAssistBytes is this G's GC assist credit in terms of
// bytes allocated. If this is positive, then the G has credit
// to allocate gcAssistBytes bytes without assisting. If this
// is negative, then the G must correct this by performing
// scan work. We track this in bytes to make it fast to update
// and check for debt in the malloc hot path. The assist ratio
// determines how this corresponds to scan work debt.
gcAssistBytes int64
}
type stack struct {
lo uintptr
hi uintptr
}
type _panic struct {
argp unsafe.Pointer // pointer to arguments of deferred call run during panic; cannot move - known to liblink
arg interface{} // argument to panic
link *_panic // link to earlier panic
recovered bool // whether this panic is over
aborted bool // the panic was aborted
}
type _defer struct {
siz int32
started bool
sp uintptr // sp at time of defer
pc uintptr
fn *funcval
_panic *_panic // panic that is running defer
link *_defer
}
type gobuf struct {
// The offsets of sp, pc, and g are known to (hard-coded in) libmach.
//
// ctxt is unusual with respect to GC: it may be a
// heap-allocated funcval so write require a write barrier,
// but gobuf needs to be cleared from assembly. We take
// advantage of the fact that the only path that uses a
// non-nil ctxt is morestack. As a result, gogo is the only
// place where it may not already be nil, so gogo uses an
// explicit write barrier. Everywhere else that resets the
// gobuf asserts that ctxt is already nil.
sp uintptr
pc uintptr
g guintptr
ctxt unsafe.Pointer // this has to be a pointer so that gc scans it
ret uintreg
lr uintptr
bp uintptr // for GOEXPERIMENT=framepointer
}
type funcval struct {
fn uintptr
}
type timer struct {
i int // heap index
// Timer wakes up at when, and then at when+period, ... (period > 0 only)
// each time calling f(arg, now) in the timer goroutine, so f must be
// a well-behaved function and not block.
when int64
period int64
f func(interface{}, uintptr)
arg interface{}
seq uintptr
}
type stkbar struct {
savedLRPtr uintptr // location overwritten by stack barrier PC
savedLRVal uintptr // value overwritten at savedLRPtr
}
type guintptr uintptr // XXX stub
type puintptr uintptr // XXX stub
type uintreg uint // FIXME wrong on amd64p32
type m struct {} // FIXME stub
type sudog struct {} // FIXME stub
// Code generated by g_typedef; DO NOT EDIT.
// +build go1.9,!go1.10
package xruntime
import "unsafe"
type g struct {
// Stack parameters.
// stack describes the actual stack memory: [stack.lo, stack.hi).
// stackguard0 is the stack pointer compared in the Go stack growth prologue.
// It is stack.lo+StackGuard normally, but can be StackPreempt to trigger a preemption.
// stackguard1 is the stack pointer compared in the C stack growth prologue.
// It is stack.lo+StackGuard on g0 and gsignal stacks.
// It is ~0 on other goroutine stacks, to trigger a call to morestackc (and crash).
stack stack // offset known to runtime/cgo
stackguard0 uintptr // offset known to liblink
stackguard1 uintptr // offset known to liblink
_panic *_panic // innermost panic - offset known to liblink
_defer *_defer // innermost defer
m *m // current m; offset known to arm liblink
sched gobuf
syscallsp uintptr // if status==Gsyscall, syscallsp = sched.sp to use during gc
syscallpc uintptr // if status==Gsyscall, syscallpc = sched.pc to use during gc
stktopsp uintptr // expected sp at top of stack, to check in traceback
param unsafe.Pointer // passed parameter on wakeup
atomicstatus uint32
stackLock uint32 // sigprof/scang lock; TODO: fold in to atomicstatus
goid int64
waitsince int64 // approx time when the g become blocked
waitreason string // if status==Gwaiting
schedlink guintptr
preempt bool // preemption signal, duplicates stackguard0 = stackpreempt
paniconfault bool // panic (instead of crash) on unexpected fault address
preemptscan bool // preempted g does scan for gc
gcscandone bool // g has scanned stack; protected by _Gscan bit in status
gcscanvalid bool // false at start of gc cycle, true if G has not run since last scan; TODO: remove?
throwsplit bool // must not split stack
raceignore int8 // ignore race detection events
sysblocktraced bool // StartTrace has emitted EvGoInSyscall about this goroutine
sysexitticks int64 // cputicks when syscall has returned (for tracing)
traceseq uint64 // trace event sequencer
tracelastp puintptr // last P emitted an event for this goroutine
lockedm *m
sig uint32
writebuf []byte
sigcode0 uintptr
sigcode1 uintptr
sigpc uintptr
gopc uintptr // pc of go statement that created this goroutine
startpc uintptr // pc of goroutine function
racectx uintptr
waiting *sudog // sudog structures this g is waiting on (that have a valid elem ptr); in lock order
cgoCtxt []uintptr // cgo traceback context
labels unsafe.Pointer // profiler labels
timer *timer // cached timer for time.Sleep
// gcAssistBytes is this G's GC assist credit in terms of
// bytes allocated. If this is positive, then the G has credit
// to allocate gcAssistBytes bytes without assisting. If this
// is negative, then the G must correct this by performing
// scan work. We track this in bytes to make it fast to update
// and check for debt in the malloc hot path. The assist ratio
// determines how this corresponds to scan work debt.
gcAssistBytes int64
}
type stack struct {
lo uintptr
hi uintptr
}
type _panic struct {
argp unsafe.Pointer // pointer to arguments of deferred call run during panic; cannot move - known to liblink
arg interface{} // argument to panic
link *_panic // link to earlier panic
recovered bool // whether this panic is over
aborted bool // the panic was aborted
}
type _defer struct {
siz int32
started bool
sp uintptr // sp at time of defer
pc uintptr
fn *funcval
_panic *_panic // panic that is running defer
link *_defer
}
type gobuf struct {
// The offsets of sp, pc, and g are known to (hard-coded in) libmach.
//
// ctxt is unusual with respect to GC: it may be a
// heap-allocated funcval so write require a write barrier,
// but gobuf needs to be cleared from assembly. We take
// advantage of the fact that the only path that uses a
// non-nil ctxt is morestack. As a result, gogo is the only
// place where it may not already be nil, so gogo uses an
// explicit write barrier. Everywhere else that resets the
// gobuf asserts that ctxt is already nil.
sp uintptr
pc uintptr
g guintptr
ctxt unsafe.Pointer // this has to be a pointer so that gc scans it
ret uintreg
lr uintptr
bp uintptr // for GOEXPERIMENT=framepointer
}
type funcval struct {
fn uintptr
}
type timer struct {
i int // heap index
// Timer wakes up at when, and then at when+period, ... (period > 0 only)
// each time calling f(arg, now) in the timer goroutine, so f must be
// a well-behaved function and not block.
when int64
period int64
f func(interface{}, uintptr)
arg interface{}
seq uintptr
}
type guintptr uintptr // XXX stub
type puintptr uintptr // XXX stub
type uintreg uint // FIXME wrong on amd64p32
type m struct {} // FIXME stub
type sudog struct {} // FIXME stub
......@@ -220,10 +220,14 @@ func Lock() {
traceMu.Lock()
xruntime.StopTheWorld("tracing lock")
atomic.StoreInt32(&traceLocked, 1)
// we synchronized with everyone via stopping the world - there is now
// no other goroutines running to race with.
xruntime.RaceIgnoreBegin()
}
// Unlock is the opposite to Lock and returns with the world resumed
func Unlock() {
xruntime.RaceIgnoreEnd()
atomic.StoreInt32(&traceLocked, 0)
xruntime.StartTheWorld()
traceMu.Unlock()
......
......@@ -21,7 +21,9 @@ package tracing
import (
"reflect"
"runtime"
"testing"
"time"
"unsafe"
"github.com/kylelemons/godebug/pretty"
......@@ -93,3 +95,51 @@ func TestAttachDetach(t *testing.T) {
detach(p3)
checkX()
}
// Test use vs concurent detach.
//
// Detach works under tracing lock (= world stopped) - so changing a probe list
// should be ok, but since race detector does not know we stopped the world it
// could complain.
func TestUseDetach(t *testing.T) {
var traceX *Probe // list head of a tracing event
// attach probe to traceX
probe := Probe{}
Lock()
AttachProbe(nil, &traceX, &probe)
Unlock()
// simulate traceX signalling and so probe usage and concurrent probe detach
go func() {
// delay a bit so that main goroutine first spins some time
// with non-empty probe list
time.Sleep(1 * time.Millisecond)
Lock()
probe.Detach()
Unlock()
}()
loop:
for {
np := 0
for p := traceX; p != nil; p = p.Next() {
np++
}
switch np {
case 1:
// ok - not yet detached
case 0:
// ok - detached
break loop
default:
t.Fatalf("probe seen %d times; must be either 1 or 0", np)
}
// XXX as of go19 tight loops are not preemptible (golang.org/issues/10958)
// and Lock does stop-the-world -> make this loop explicitly preemtible.
runtime.Gosched()
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment