runtime: free spanQueue on P destroy

Span queues must be empty when destroying a P since we are outside of
the mark phase. But we don't actually free them, so they simply sit
around using memory. More importantly, they are still in
work.spanSPMCs.all, so freeDeadSpanSPMCs must continue traversing past
them until the end of time.

Prior to CL 709575, keeping them in work.spanSPMCs.all allowed programs
with low GOMAXPROCS to continue triggering the bug if they ever had high
GOMAXPROCS in the past.

The spanSPMCs list is singly-linked, so it is not efficient to remove a
random element from the middle. Instead, we simply mark it as dead to
all freeDeadSpanSPMCs to free it when it scans the full list.

For #75771.

Change-Id: I6a6a636cfa22a4bdef0c273d083c91553e923fe5
Reviewed-on: https://go-review.googlesource.com/c/go/+/709656
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/runtime/mgcmark_greenteagc.go b/src/runtime/mgcmark_greenteagc.go
index 7f8d60349ffb6723ef72107fa1a06700272e18b9..6ebd7ced81b9018885ae4df5681309463fbaefa4 100644 (file)
--- a/src/runtime/mgcmark_greenteagc.go
+++ b/src/runtime/mgcmark_greenteagc.go
@@ -618,6 +618,40 @@ func (q *spanQueue) refill(r *spanSPMC) objptr {
        return q.tryGetFast()
 }
 
+// destroy frees all chains in an empty spanQueue.
+//
+// Preconditions:
+// - World is stopped.
+// - GC is outside of the mark phase.
+// - (Therefore) the queue is empty.
+func (q *spanQueue) destroy() {
+       assertWorldStopped()
+       if gcphase != _GCoff {
+               throw("spanQueue.destroy during the mark phase")
+       }
+       if !q.empty() {
+               throw("spanQueue.destroy on non-empty queue")
+       }
+
+       lock(&work.spanSPMCs.lock)
+
+       // Mark each ring as dead. The sweeper will actually free them.
+       //
+       // N.B., we could free directly here, but work.spanSPMCs.all is a
+       // singly-linked list, so we'd need to walk the entire list to find the
+       // previous node. If the list becomes doubly-linked, we can free
+       // directly.
+       for r := (*spanSPMC)(q.chain.tail.Load()); r != nil; r = (*spanSPMC)(r.prev.Load()) {
+               r.dead.Store(true)
+       }
+
+       q.chain.head = nil
+       q.chain.tail.Store(nil)
+       q.putsSinceDrain = 0
+
+       unlock(&work.spanSPMCs.lock)
+}
+
 // spanSPMC is a ring buffer of objptrs that represent spans.
 // Accessed without a lock.
 //

diff --git a/src/runtime/mgcmark_nogreenteagc.go b/src/runtime/mgcmark_nogreenteagc.go
index 883c3451abec6bfba30952acba380e76248cd33a..024565ef3e250d221ee7ed7250be3f5534aa4dd9 100644 (file)
--- a/src/runtime/mgcmark_nogreenteagc.go
+++ b/src/runtime/mgcmark_nogreenteagc.go
@@ -63,6 +63,9 @@ func (q *spanQueue) empty() bool {
        return true
 }
 
+func (q *spanQueue) destroy() {
+}
+
 type spanSPMC struct {
        _ sys.NotInHeap
 }

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index d36895b046c8c60c9f19c4100d7eda5dea3758d6..fadd9a59639f87f8b2b0f8aa41a5dbe40ee2f9cc 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -5824,6 +5824,8 @@ func (pp *p) destroy() {
                println("runtime: p id", pp.id, "destroyed during GC phase", phase)
                throw("P destroyed while GC is running")
        }
+       // We should free the queues though.
+       pp.gcw.spanq.destroy()
 
        clear(pp.sudogbuf[:])
        pp.sudogcache = pp.sudogbuf[:0]