Commit 91e8554b authored by Keith Randall's avatar Keith Randall

runtime: delay freeing of shrunk stacks until gc is done.

This change prevents confusion in the garbage collector.
The collector wants to make sure that every pointer it finds
isn't junk.  Its criteria for junk is (among others) points
to a "free" span.

Because the stack shrinker modifies pointers in the heap,
there is a race condition between the GC scanner and the
shrinker.  The GC scanner can see old pointers (pointers to
freed stacks).  In particular this happens with SudoG.elem
pointers.

Normally this is not a problem, as pointers into stack spans
are ok.  But if the freed stack is the last one in its span,
the span is marked as "free" instead of "contains stacks".

This change makes sure that even if the GC scanner sees
an old pointer, the span into which it points is still
marked as "contains stacks", and thus the GC doesn't
complain about it.

This change will make the GC pause a tiny bit slower, as
the stack freeing now happens in serial with the mark pause.
We could delay the freeing until the mutators start back up,
but this is the simplest change for now.

TBR=dvyukov
CC=golang-codereviews
https://golang.org/cl/158750043
parent 6920b2a1
...@@ -1445,6 +1445,8 @@ gc(struct gc_args *args) ...@@ -1445,6 +1445,8 @@ gc(struct gc_args *args)
if(runtime·work.nproc > 1) if(runtime·work.nproc > 1)
runtime·notesleep(&runtime·work.alldone); runtime·notesleep(&runtime·work.alldone);
runtime·shrinkfinish();
cachestats(); cachestats();
// next_gc calculation is tricky with concurrent sweep since we don't know size of live heap // next_gc calculation is tricky with concurrent sweep since we don't know size of live heap
// estimate what was live heap size after previous GC (for tracing only) // estimate what was live heap size after previous GC (for tracing only)
......
...@@ -852,6 +852,7 @@ void runtime·stackinit(void); ...@@ -852,6 +852,7 @@ void runtime·stackinit(void);
Stack runtime·stackalloc(uint32); Stack runtime·stackalloc(uint32);
void runtime·stackfree(Stack); void runtime·stackfree(Stack);
void runtime·shrinkstack(G*); void runtime·shrinkstack(G*);
void runtime·shrinkfinish(void);
MCache* runtime·allocmcache(void); MCache* runtime·allocmcache(void);
void runtime·freemcache(MCache*); void runtime·freemcache(MCache*);
void runtime·mallocinit(void); void runtime·mallocinit(void);
......
...@@ -36,6 +36,8 @@ MSpan runtime·stackpool[NumStackOrders]; ...@@ -36,6 +36,8 @@ MSpan runtime·stackpool[NumStackOrders];
Mutex runtime·stackpoolmu; Mutex runtime·stackpoolmu;
// TODO: one lock per order? // TODO: one lock per order?
static Stack stackfreequeue;
void void
runtime·stackinit(void) runtime·stackinit(void)
{ {
...@@ -656,7 +658,24 @@ copystack(G *gp, uintptr newsize) ...@@ -656,7 +658,24 @@ copystack(G *gp, uintptr newsize)
while(p < ep) while(p < ep)
*p++ = 0xfc; *p++ = 0xfc;
} }
runtime·stackfree(old); if(newsize > old.hi-old.lo) {
// growing, free stack immediately
runtime·stackfree(old);
} else {
// shrinking, queue up free operation. We can't actually free the stack
// just yet because we might run into the following situation:
// 1) GC starts, scans a SudoG but does not yet mark the SudoG.elem pointer
// 2) The stack that pointer points to is shrunk
// 3) The old stack is freed
// 4) The containing span is marked free
// 5) GC attempts to mark the SudoG.elem pointer. The marking fails because
// the pointer looks like a pointer into a free span.
// By not freeing, we prevent step #4 until GC is done.
runtime·lock(&runtime·stackpoolmu);
*(Stack*)old.lo = stackfreequeue;
stackfreequeue = old;
runtime·unlock(&runtime·stackpoolmu);
}
} }
// round x up to a power of 2. // round x up to a power of 2.
...@@ -841,6 +860,23 @@ runtime·shrinkstack(G *gp) ...@@ -841,6 +860,23 @@ runtime·shrinkstack(G *gp)
copystack(gp, newsize); copystack(gp, newsize);
} }
// Do any delayed stack freeing that was queued up during GC.
void
runtime·shrinkfinish(void)
{
Stack s, t;
runtime·lock(&runtime·stackpoolmu);
s = stackfreequeue;
stackfreequeue = (Stack){0,0};
runtime·unlock(&runtime·stackpoolmu);
while(s.lo != 0) {
t = *(Stack*)s.lo;
runtime·stackfree(s);
s = t;
}
}
static void badc(void); static void badc(void);
#pragma textflag NOSPLIT #pragma textflag NOSPLIT
......
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