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

diff --git a/src/pkg/runtime/malloc.goc b/src/pkg/runtime/malloc.goc
index 70b85d68d77c6de7c45bd1b10fb3eda66cf38c46..745e18ca0da7a786de6c7a75b52c2da10916a27a 100644 (file)
--- a/src/pkg/runtime/malloc.goc
+++ b/src/pkg/runtime/malloc.goc
@@ -146,6 +146,9 @@ runtime·free(void *v)
                // Large object.
                size = s->npages<<PageShift;
                *(uintptr*)(s->start<<PageShift) = 1;   // mark as "needs to be zeroed"
+               // Must mark v freed before calling unmarkspan and MHeap_Free:
+               // they might coalesce v into other spans and change the bitmap further.
+               runtime·markfreed(v, size);
                runtime·unmarkspan(v, 1<<PageShift);
                runtime·MHeap_Free(&runtime·mheap, s, 1);
        } else {
@@ -154,10 +157,13 @@ runtime·free(void *v)
                size = runtime·class_to_size[sizeclass];
                if(size > sizeof(uintptr))
                        ((uintptr*)v)[1] = 1;   // mark as "needs to be zeroed"
+               // Must mark v freed before calling MCache_Free:
+               // it might coalesce v and other blocks into a bigger span
+               // and change the bitmap further.
+               runtime·markfreed(v, size);
                mstats.by_size[sizeclass].nfree++;
                runtime·MCache_Free(c, v, sizeclass, size);
        }
-       runtime·markfreed(v, size);
        mstats.alloc -= size;
        if(prof)
                runtime·MProf_Free(v, size);

diff --git a/src/pkg/runtime/mgc0.c b/src/pkg/runtime/mgc0.c
index 232c6cdcd5b052f6fe72ae0fea3e25c3bd35aceb..c471fff5e87a63af351da7a9c52df5d575e10152 100644 (file)
--- a/src/pkg/runtime/mgc0.c
+++ b/src/pkg/runtime/mgc0.c
@@ -663,7 +663,7 @@ runfinq(void)
 void
 runtime·markallocated(void *v, uintptr n, bool noptr)
 {
-       uintptr *b, bits, off, shift;
+       uintptr *b, obits, bits, off, shift;
 
        if(0)
                runtime·printf("markallocated %p+%p\n", v, n);
@@ -675,17 +675,27 @@ runtime·markallocated(void *v, uintptr n, bool noptr)
        b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
        shift = off % wordsPerBitmapWord;
 
-       bits = (*b & ~(bitMask<<shift)) | (bitAllocated<<shift);
-       if(noptr)
-               bits |= bitNoPointers<<shift;
-       *b = bits;
+       for(;;) {
+               obits = *b;
+               bits = (obits & ~(bitMask<<shift)) | (bitAllocated<<shift);
+               if(noptr)
+                       bits |= bitNoPointers<<shift;
+               if(runtime·gomaxprocs == 1) {
+                       *b = bits;
+                       break;
+               } else {
+                       // gomaxprocs > 1: use atomic op
+                       if(runtime·casp((void**)b, (void*)obits, (void*)bits))
+                               break;
+               }
+       }
 }
 
 // mark the block at v of size n as freed.
 void
 runtime·markfreed(void *v, uintptr n)
 {
-       uintptr *b, off, shift;
+       uintptr *b, obits, bits, off, shift;
 
        if(0)
                runtime·printf("markallocated %p+%p\n", v, n);
@@ -697,7 +707,18 @@ runtime·markfreed(void *v, uintptr n)
        b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
        shift = off % wordsPerBitmapWord;
 
-       *b = (*b & ~(bitMask<<shift)) | (bitBlockBoundary<<shift);
+       for(;;) {
+               obits = *b;
+               bits = (obits & ~(bitMask<<shift)) | (bitBlockBoundary<<shift);
+               if(runtime·gomaxprocs == 1) {
+                       *b = bits;
+                       break;
+               } else {
+                       // gomaxprocs > 1: use atomic op
+                       if(runtime·casp((void**)b, (void*)obits, (void*)bits))
+                               break;
+               }
+       }
 }
 
 // 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)
        if(leftover)    // mark a boundary just past end of last block too
                n++;
        for(; n-- > 0; p += size) {
+               // Okay to use non-atomic ops here, because we control
+               // the entire span, and each bitmap word has bits for only
+               // one span, so no other goroutines are changing these
+               // bitmap words.
                off = (uintptr*)p - (uintptr*)runtime·mheap.arena_start;  // word offset
                b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
                shift = off % wordsPerBitmapWord;
@@ -763,6 +788,10 @@ runtime·unmarkspan(void *v, uintptr n)
        n /= PtrSize;
        if(n%wordsPerBitmapWord != 0)
                runtime·throw("unmarkspan: unaligned length");
+       // Okay to use non-atomic ops here, because we control
+       // the entire span, and each bitmap word has bits for only
+       // one span, so no other goroutines are changing these
+       // bitmap words.
        n /= wordsPerBitmapWord;
        while(n-- > 0)
                *b-- = 0;
@@ -783,13 +812,24 @@ runtime·blockspecial(void *v)
 void
 runtime·setblockspecial(void *v)
 {
-       uintptr *b, off, shift;
+       uintptr *b, off, shift, bits, obits;
 
        off = (uintptr*)v - (uintptr*)runtime·mheap.arena_start;
        b = (uintptr*)runtime·mheap.arena_start - off/wordsPerBitmapWord - 1;
        shift = off % wordsPerBitmapWord;
 
-       *b |= bitSpecial<<shift;
+       for(;;) {
+               obits = *b;
+               bits = obits | (bitSpecial<<shift);
+               if(runtime·gomaxprocs == 1) {
+                       *b = bits;
+                       break;
+               } else {
+                       // gomaxprocs > 1: use atomic op
+                       if(runtime·casp((void**)b, (void*)obits, (void*)bits))
+                               break;
+               }
+       }
 }
  
 void