diff options
| author | Michael Anthony Knyszek <mknyszek@google.com> | 2024-08-27 21:02:02 +0000 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2025-02-03 08:21:09 -0800 |
| commit | b25b5f3ff4e671aa4f5897c788137fe91f62cf57 (patch) | |
| tree | ef37f5db04b45dad5303ba33911a9ff133348705 /src/runtime/malloc.go | |
| parent | 4f11d5879a01e64cb8bd59911bb205ffedd4f265 (diff) | |
| download | go-b25b5f3ff4e671aa4f5897c788137fe91f62cf57.tar.xz | |
runtime: fix GODEBUG=gccheckmark=1 and add smoke test
This change fixes GODEBUG=gccheckmark=1 which seems to have bit-rotted.
Because the root jobs weren't being reset, it wasn't doing anything.
Then, it turned out that checkmark mode would queue up noscan objects in
workbufs, which caused it to fail. Then it turned out checkmark mode was
broken with user arenas, since their heap arenas are not registered
anywhere. Then, it turned out that checkmark mode could just not run
properly if the goroutine's preemption flag was set (since
sched.gcwaiting is true during the STW). And lastly, it turned out that
async preemption could cause erroneous checkmark failures.
This change fixes all these issues and adds a simple smoke test to dist
to run the runtime tests under gccheckmark, which exercises all of these
issues.
Fixes #69074.
Fixes #69377.
Fixes #69376.
Change-Id: Iaa0bb7b9e63ed4ba34d222b47510d6292ce168bc
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/608915
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Diffstat (limited to 'src/runtime/malloc.go')
| -rw-r--r-- | src/runtime/malloc.go | 43 |
1 files changed, 20 insertions, 23 deletions
diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 73d663f7f5..60ea2f5188 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -640,14 +640,13 @@ func mallocinit() { // hintList is a list of hint addresses for where to allocate new // heap arenas. It must be non-nil. // -// register indicates whether the heap arena should be registered -// in allArenas. -// // sysAlloc returns a memory region in the Reserved state. This region must // be transitioned to Prepared and then Ready before use. // +// arenaList is the list the arena should be added to. +// // h must be locked. -func (h *mheap) sysAlloc(n uintptr, hintList **arenaHint, register bool) (v unsafe.Pointer, size uintptr) { +func (h *mheap) sysAlloc(n uintptr, hintList **arenaHint, arenaList *[]arenaIdx) (v unsafe.Pointer, size uintptr) { assertLockHeld(&h.lock) n = alignUp(n, heapArenaBytes) @@ -790,27 +789,25 @@ mapped: } // Register the arena in allArenas if requested. - if register { - if len(h.allArenas) == cap(h.allArenas) { - size := 2 * uintptr(cap(h.allArenas)) * goarch.PtrSize - if size == 0 { - size = physPageSize - } - newArray := (*notInHeap)(persistentalloc(size, goarch.PtrSize, &memstats.gcMiscSys)) - if newArray == nil { - throw("out of memory allocating allArenas") - } - oldSlice := h.allArenas - *(*notInHeapSlice)(unsafe.Pointer(&h.allArenas)) = notInHeapSlice{newArray, len(h.allArenas), int(size / goarch.PtrSize)} - copy(h.allArenas, oldSlice) - // Do not free the old backing array because - // there may be concurrent readers. Since we - // double the array each time, this can lead - // to at most 2x waste. + if len((*arenaList)) == cap((*arenaList)) { + size := 2 * uintptr(cap((*arenaList))) * goarch.PtrSize + if size == 0 { + size = physPageSize + } + newArray := (*notInHeap)(persistentalloc(size, goarch.PtrSize, &memstats.gcMiscSys)) + if newArray == nil { + throw("out of memory allocating allArenas") } - h.allArenas = h.allArenas[:len(h.allArenas)+1] - h.allArenas[len(h.allArenas)-1] = ri + oldSlice := (*arenaList) + *(*notInHeapSlice)(unsafe.Pointer(&(*arenaList))) = notInHeapSlice{newArray, len((*arenaList)), int(size / goarch.PtrSize)} + copy((*arenaList), oldSlice) + // Do not free the old backing array because + // there may be concurrent readers. Since we + // double the array each time, this can lead + // to at most 2x waste. } + (*arenaList) = (*arenaList)[:len((*arenaList))+1] + (*arenaList)[len((*arenaList))-1] = ri // Store atomically just in case an object from the // new heap arena becomes visible before the heap lock |
