Skip to content

Commit 2509776

Browse files
committed
runtime: fix memory allocator for GOMAXPROCS > 1
Bitmaps were not being updated safely. Depends on 4188053. Fixes #1504. May fix issue 1479. R=r, r2 CC=golang-dev https://golang.org/cl/4184048
1 parent 6779350 commit 2509776

File tree

2 files changed

+56
-10
lines changed

2 files changed

+56
-10
lines changed

src/pkg/runtime/malloc.goc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ runtime·free(void *v)
146146
// Large object.
147147
size = s->npages<<PageShift;
148148
*(uintptr*)(s->start<<PageShift) = 1; // mark as "needs to be zeroed"
149+
// Must mark v freed before calling unmarkspan and MHeap_Free:
150+
// they might coalesce v into other spans and change the bitmap further.
151+
runtime·markfreed(v, size);
149152
runtime·unmarkspan(v, 1<<PageShift);
150153
runtime·MHeap_Free(&runtime·mheap, s, 1);
151154
} else {
@@ -154,10 +157,13 @@ runtime·free(void *v)
154157
size = runtime·class_to_size[sizeclass];
155158
if(size > sizeof(uintptr))
156159
((uintptr*)v)[1] = 1; // mark as "needs to be zeroed"
160+
// Must mark v freed before calling MCache_Free:
161+
// it might coalesce v and other blocks into a bigger span
162+
// and change the bitmap further.
163+
runtime·markfreed(v, size);
157164
mstats.by_size[sizeclass].nfree++;
158165
runtime·MCache_Free(c, v, sizeclass, size);
159166
}
160-
runtime·markfreed(v, size);
161167
mstats.alloc -= size;
162168
if(prof)
163169
runtime·MProf_Free(v, size);

src/pkg/runtime/mgc0.c

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ runfinq(void)
663663
void
664664
runtime·markallocated(void *v, uintptr n, bool noptr)
665665
{
666-
uintptr *b, bits, off, shift;
666+
uintptr *b, obits, bits, off, shift;
667667

668668
if(0)
669669
runtime·printf("markallocated %p+%p\n", v, n);
@@ -675,17 +675,27 @@ runtime·markallocated(void *v, uintptr n, bool noptr)
675675
b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
676676
shift = off % wordsPerBitmapWord;
677677

678-
bits = (*b & ~(bitMask<<shift)) | (bitAllocated<<shift);
679-
if(noptr)
680-
bits |= bitNoPointers<<shift;
681-
*b = bits;
678+
for(;;) {
679+
obits = *b;
680+
bits = (obits & ~(bitMask<<shift)) | (bitAllocated<<shift);
681+
if(noptr)
682+
bits |= bitNoPointers<<shift;
683+
if(runtime·gomaxprocs == 1) {
684+
*b = bits;
685+
break;
686+
} else {
687+
// gomaxprocs > 1: use atomic op
688+
if(runtime·casp((void**)b, (void*)obits, (void*)bits))
689+
break;
690+
}
691+
}
682692
}
683693

684694
// mark the block at v of size n as freed.
685695
void
686696
runtime·markfreed(void *v, uintptr n)
687697
{
688-
uintptr *b, off, shift;
698+
uintptr *b, obits, bits, off, shift;
689699

690700
if(0)
691701
runtime·printf("markallocated %p+%p\n", v, n);
@@ -697,7 +707,18 @@ runtime·markfreed(void *v, uintptr n)
697707
b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
698708
shift = off % wordsPerBitmapWord;
699709

700-
*b = (*b & ~(bitMask<<shift)) | (bitBlockBoundary<<shift);
710+
for(;;) {
711+
obits = *b;
712+
bits = (obits & ~(bitMask<<shift)) | (bitBlockBoundary<<shift);
713+
if(runtime·gomaxprocs == 1) {
714+
*b = bits;
715+
break;
716+
} else {
717+
// gomaxprocs > 1: use atomic op
718+
if(runtime·casp((void**)b, (void*)obits, (void*)bits))
719+
break;
720+
}
721+
}
701722
}
702723

703724
// check that the block at v of size n is marked freed.
@@ -739,6 +760,10 @@ runtime·markspan(void *v, uintptr size, uintptr n, bool leftover)
739760
if(leftover) // mark a boundary just past end of last block too
740761
n++;
741762
for(; n-- > 0; p += size) {
763+
// Okay to use non-atomic ops here, because we control
764+
// the entire span, and each bitmap word has bits for only
765+
// one span, so no other goroutines are changing these
766+
// bitmap words.
742767
off = (uintptr*)p - (uintptr*)runtime·mheap.arena_start; // word offset
743768
b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
744769
shift = off % wordsPerBitmapWord;
@@ -763,6 +788,10 @@ runtime·unmarkspan(void *v, uintptr n)
763788
n /= PtrSize;
764789
if(n%wordsPerBitmapWord != 0)
765790
runtime·throw("unmarkspan: unaligned length");
791+
// Okay to use non-atomic ops here, because we control
792+
// the entire span, and each bitmap word has bits for only
793+
// one span, so no other goroutines are changing these
794+
// bitmap words.
766795
n /= wordsPerBitmapWord;
767796
while(n-- > 0)
768797
*b-- = 0;
@@ -783,13 +812,24 @@ runtime·blockspecial(void *v)
783812
void
784813
runtime·setblockspecial(void *v)
785814
{
786-
uintptr *b, off, shift;
815+
uintptr *b, off, shift, bits, obits;
787816

788817
off = (uintptr*)v - (uintptr*)runtime·mheap.arena_start;
789818
b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
790819
shift = off % wordsPerBitmapWord;
791820

792-
*b |= bitSpecial<<shift;
821+
for(;;) {
822+
obits = *b;
823+
bits = obits | (bitSpecial<<shift);
824+
if(runtime·gomaxprocs == 1) {
825+
*b = bits;
826+
break;
827+
} else {
828+
// gomaxprocs > 1: use atomic op
829+
if(runtime·casp((void**)b, (void*)obits, (void*)bits))
830+
break;
831+
}
832+
}
793833
}
794834

795835
void

0 commit comments

Comments
 (0)