From 3b2d8145a4c349058d76ce299ea7eea605572004 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 21 Dec 2020 21:20:59 -0800 Subject: runtime: add "success" field to sudog This is the gofrontend version of https://golang.org/cl/245019. Original CL description: The current wakeup protocol for channel communications is that the second goroutine sets gp.param to the sudog when a value is successfully communicated over the channel, and to nil when the wakeup is due to closing the channel. Setting nil to indicate channel closure works okay for chansend and chanrecv, because they're only communicating with one channel, so they know it must be the channel that was closed. However, it means selectgo has to re-poll all of the channels to figure out which one was closed. This commit adds a "success" field to sudog, and changes the wakeup protocol to always set gp.param to sg, and to use sg.success to indicate successful communication vs channel closure. While here, this also reorganizes the chansend code slightly so that the sudog is still released to the pool if the send blocks and then is awoken because the channel closed. For golang/go#40410 This is being brought over to gofrontend as a step toward upgrading to Go1.16beta1, setting up for more compiler changes related to select handling. Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/279734 --- libgo/go/runtime/chan.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'libgo/go/runtime/chan.go') diff --git a/libgo/go/runtime/chan.go b/libgo/go/runtime/chan.go index 8e104f1..1b20aff 100644 --- a/libgo/go/runtime/chan.go +++ b/libgo/go/runtime/chan.go @@ -285,18 +285,19 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool { } gp.waiting = nil gp.activeStackChans = false - if gp.param == nil { - if c.closed == 0 { - throw("chansend: spurious wakeup") - } - panic(plainError("send on closed channel")) - } + closed := !mysg.success gp.param = nil if mysg.releasetime > 0 { blockevent(mysg.releasetime-t0, 2) } mysg.c = nil releaseSudog(mysg) + if closed { + if c.closed == 0 { + throw("chansend: spurious wakeup") + } + panic(plainError("send on closed channel")) + } return true } @@ -333,6 +334,7 @@ func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) { gp := sg.g unlockf() gp.param = unsafe.Pointer(sg) + sg.success = true if sg.releasetime != 0 { sg.releasetime = cputicks() } @@ -406,7 +408,8 @@ func closechan(c *hchan) { sg.releasetime = cputicks() } gp := sg.g - gp.param = nil + gp.param = unsafe.Pointer(sg) + sg.success = false if raceenabled { raceacquireg(gp, c.raceaddr()) } @@ -424,7 +427,8 @@ func closechan(c *hchan) { sg.releasetime = cputicks() } gp := sg.g - gp.param = nil + gp.param = unsafe.Pointer(sg) + sg.success = false if raceenabled { raceacquireg(gp, c.raceaddr()) } @@ -607,11 +611,11 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool) if mysg.releasetime > 0 { blockevent(mysg.releasetime-t0, 2) } - closed := gp.param == nil + success := mysg.success gp.param = nil mysg.c = nil releaseSudog(mysg) - return true, !closed + return true, success } // recv processes a receive operation on a full channel c. @@ -664,6 +668,7 @@ func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) { gp := sg.g unlockf() gp.param = unsafe.Pointer(sg) + sg.success = true if sg.releasetime != 0 { sg.releasetime = cputicks() } -- cgit v1.1 From cfcbb4227fb20191e04eb8d7766ae6202f526afd Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 23 Dec 2020 09:57:37 -0800 Subject: libgo: update to Go1.16beta1 release This does not yet include support for the //go:embed directive added in this release. * Makefile.am (check-runtime): Don't create check-runtime-dir. (mostlyclean-local): Don't remove check-runtime-dir. (check-go-tool, check-vet): Copy in go.mod and modules.txt. (check-cgo-test, check-carchive-test): Add go.mod file. * Makefile.in: Regenerate. Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/280172 --- libgo/go/runtime/chan.go | 54 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 13 deletions(-) (limited to 'libgo/go/runtime/chan.go') diff --git a/libgo/go/runtime/chan.go b/libgo/go/runtime/chan.go index 1b20aff..7878a8f 100644 --- a/libgo/go/runtime/chan.go +++ b/libgo/go/runtime/chan.go @@ -232,8 +232,7 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool { // Space is available in the channel buffer. Enqueue the element to send. qp := chanbuf(c, c.sendx) if raceenabled { - raceacquire(qp) - racerelease(qp) + racenotify(c, c.sendx, nil) } typedmemmove(c.elemtype, qp, ep) c.sendx++ @@ -315,11 +314,8 @@ func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) { // Pretend we go through the buffer, even though // we copy directly. Note that we need to increment // the head/tail locations only when raceenabled. - qp := chanbuf(c, c.recvx) - raceacquire(qp) - racerelease(qp) - raceacquireg(sg.g, qp) - racereleaseg(sg.g, qp) + racenotify(c, c.recvx, nil) + racenotify(c, c.recvx, sg) c.recvx++ if c.recvx == c.dataqsiz { c.recvx = 0 @@ -557,8 +553,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool) // Receive directly from queue qp := chanbuf(c, c.recvx) if raceenabled { - raceacquire(qp) - racerelease(qp) + racenotify(c, c.recvx, nil) } if ep != nil { typedmemmove(c.elemtype, ep, qp) @@ -647,10 +642,8 @@ func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) { // queue is full, those are both the same slot. qp := chanbuf(c, c.recvx) if raceenabled { - raceacquire(qp) - racerelease(qp) - raceacquireg(sg.g, qp) - racereleaseg(sg.g, qp) + racenotify(c, c.recvx, nil) + racenotify(c, c.recvx, sg) } // copy data from queue to receiver if ep != nil { @@ -861,3 +854,38 @@ func racesync(c *hchan, sg *sudog) { racereleaseg(sg.g, chanbuf(c, 0)) raceacquire(chanbuf(c, 0)) } + +// Notify the race detector of a send or receive involving buffer entry idx +// and a channel c or its communicating partner sg. +// This function handles the special case of c.elemsize==0. +func racenotify(c *hchan, idx uint, sg *sudog) { + // We could have passed the unsafe.Pointer corresponding to entry idx + // instead of idx itself. However, in a future version of this function, + // we can use idx to better handle the case of elemsize==0. + // A future improvement to the detector is to call TSan with c and idx: + // this way, Go will continue to not allocating buffer entries for channels + // of elemsize==0, yet the race detector can be made to handle multiple + // sync objects underneath the hood (one sync object per idx) + qp := chanbuf(c, idx) + // When elemsize==0, we don't allocate a full buffer for the channel. + // Instead of individual buffer entries, the race detector uses the + // c.buf as the only buffer entry. This simplification prevents us from + // following the memory model's happens-before rules (rules that are + // implemented in racereleaseacquire). Instead, we accumulate happens-before + // information in the synchronization object associated with c.buf. + if c.elemsize == 0 { + if sg == nil { + raceacquire(qp) + racerelease(qp) + } else { + raceacquireg(sg.g, qp) + racereleaseg(sg.g, qp) + } + } else { + if sg == nil { + racereleaseacquire(qp) + } else { + racereleaseacquireg(sg.g, qp) + } + } +} -- cgit v1.1