bmalloc, libpas: stop fragmenting the heap into VMAs, and bound the EAGAIN retry loop#275
bmalloc, libpas: stop fragmenting the heap into VMAs, and bound the EAGAIN retry loop#275robobun wants to merge 2 commits into
Conversation
vmDeallocatePhysicalPages() followed MADV_DONTNEED with MADV_DONTDUMP, and vmAllocatePhysicalPages() followed MADV_NORMAL with MADV_DODUMP. libpas did the same in decommit_impl()/commit_impl(). Both toggle VM_DONTDUMP over a sub-range of the reservation, which splits the VMA, and VMAs that differ in their flags cannot merge back. Over a fragmented heap the reservation shreds into one VMA per chunk. A 32MB reservation cycled through four scavenger rounds at 64KB granularity ends up with 512 VMAs instead of 1, and a multi-GB JS heap reaches tens of thousands. Once the process hits vm.max_map_count every further madvise() that needs a split fails. MADV_DONTDUMP also takes mmap_write_lock on Linux, which the other GC threads then contend on; MADV_DONTNEED only needs the read lock. The pages these calls mark were just released by MADV_DONTNEED, so they are not resident and a core dump already skips them: dump_user_range() writes a hole for any page get_dump_page() cannot fault in, and dump_skip() does not charge RLIMIT_CORE. Dropping the pair costs nothing in dump size and removes both the fragmentation and the write-lock traffic. MADV_NORMAL stays: it clears VM_RAND_READ/VM_SEQ_READ, which bmalloc never sets, so madvise_update_vma() returns early without touching the VMA.
Both macros retried forever with no delay:
#define SYSCALL(x) do { \
while ((x) == -1 && errno == EAGAIN) { } \
} while (0);
madvise() does not only return EAGAIN for transient conditions. mm/madvise.c
reports a failed VMA operation as EAGAIN rather than ENOMEM:
out:
/* madvise() returns EAGAIN if kernel resources, such as
slab, are temporarily unavailable. */
if (error == -ENOMEM)
error = -EAGAIN;
so a process sitting at vm.max_map_count gets EAGAIN on every call that needs
a split, permanently. The loop then spins on the syscall at roughly 3M
calls/sec per thread, pinning a core and holding the allocator lock, until
something kills the process. Reported as a multi-hour livelock with the main
thread parked in futex_wait behind the allocator.
Cap the retries at 100 and back off 1ms between attempts. The syscalls these
macros wrap are advisory or already validated elsewhere, so giving up after
100ms is recoverable where an unbounded loop is not. The fast path is
unchanged: on success the loop body never runs.
nanosleep() over usleep(): it is not marked obsolescent, and bmalloc already
builds with _GNU_SOURCE so <time.h> declares it. The Windows branches of both
headers never expand these macros.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR modifies two independent areas: syscall retry macros in bmalloc and libpas gain bounded EAGAIN retry with nanosleep backoff instead of tight infinite loops, and madvise calls pairing MADV_DONTNEED/MADV_NORMAL with MADV_DONTDUMP/MADV_DODUMP are removed on Linux across VMAllocate.h, pas_page_malloc.c, with a related comment update in pas_thread_local_cache.c. ChangesBounded EAGAIN Retry Macros
MADV_DONTDUMP/MADV_DODUMP Removal
Related Issues: None provided. Related PRs: None provided. Suggested Labels: bmalloc, libpas, memory-management, performance Suggested Reviewers: None provided. Poem: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs, but this changes core allocator behavior (syscall retry semantics and Linux commit/decommit paths in bmalloc + libpas), so it warrants a human sign-off.
Extended reasoning...
Overview
This PR touches five files in bmalloc and libpas: BSyscall.h, VMAllocate.h, pas_page_malloc.c, pas_thread_local_cache.c, and pas_utils.h. It makes two related changes: (1) removes the MADV_DONTDUMP/MADV_DODUMP toggling paired with every Linux decommit/commit, and (2) rewrites the SYSCALL/PAS_SYSCALL macros to cap EAGAIN retries at 100 with a 1ms nanosleep backoff instead of spinning forever. A comment in pas_thread_local_cache.c is updated to reflect that the Linux asymmetric commit is now a no-op.
Security risks
None identified. The change removes advisory madvise calls and bounds a retry loop; it does not touch permissions, auth, or data handling. The one behavioral edge is that SYSCALL also wraps mprotect in commit_impl/decommit_impl, so a persistent-EAGAIN mprotect would now silently give up rather than spin — but that is strictly less bad than the current livelock, and mprotect doesn't split VMAs the way madvise(MADV_DONTDUMP) does.
Level of scrutiny
High. bmalloc/libpas is the process-wide allocator; every allocation and the GC scavenger go through these paths. The changes are small and very well-argued (kernel source cited, reproduction cases included, links to two open Bun hangs), but they encode design decisions a human should confirm: the 100×1ms retry budget, silently proceeding after the cap is hit, and dropping MADV_DONTDUMP (which slightly changes core-dump semantics for decommitted-but-later-touched pages, even if the practical effect is argued to be nil).
Other factors
The author explicitly notes they have not verified under a real JSC workload that the VMA count stays flat, deferring to CI + stress suite. The macro change introduces block-scoped locals inside do { }, so adjacent expansions in one scope are safe (and the PR description says this was tested). No prior reviews or comments exist on the PR, and the automated bug hunt found nothing. Given the criticality of the code path, I'm deferring rather than approving.
1 participant
bmalloc pairs every physical-page decommit with
madvise(MADV_DONTDUMP)and every commit withmadvise(MADV_DODUMP), on Linux only. Both toggleVM_DONTDUMPover a sub-range of the reservation, which splits the VMA. VMAs whose flags differ cannot merge back, so a heap that is repeatedly committed and decommitted at page granularity shreds its own reservation into one VMA per chunk.Once the process reaches
vm.max_map_count, every subsequentmadvise()that needs a split fails. The kernel reports that failure asEAGAIN, notENOMEM(mm/madvise.c,madvise_vma_behavior()):And
SYSCALL/PAS_SYSCALLretryEAGAINforever, with no delay and no cap:The EAGAIN never clears, so the loop never exits. A GC thread pins a core spinning on
madvisewhile holding the allocator lock, and the main thread parks infutex_waitbehind it. The process stays alive and stops doing anything.Reproduction
The fragmentation, modelling bmalloc's commit/decommit over a single reservation (32MB, 64KB chunks, four scavenger rounds):
vma_fragmentation.c
And the livelock itself, on kernel 6.17:
madvise_eagain.c
The fix
Stop toggling
VM_DONTDUMP. This is the cause, both of the fragmentation and of themmap_write_lockcontention (MADV_DONTDUMPneeds the write lock;MADV_DONTNEEDonly needs the read lock). It is also nearly free to remove: the pages it marks were just released byMADV_DONTNEED, so they are not resident, and a core dump already skips them.dump_user_range()writes a hole for any pageget_dump_page()cannot fault in, anddump_skip()advancescprm->poswithout chargingcprm->written, soRLIMIT_COREis not consumed either.MADV_NORMALstays. It clearsVM_RAND_READ/VM_SEQ_READ, which bmalloc never sets, somadvise_update_vma()returns early without touching the VMA. The measurement above confirms it: the run withoutMADV_DONTDUMPstill callsMADV_NORMALand still ends with one VMA.Cap the
EAGAINretry. 100 retries, 1ms apart. These syscalls are advisory, or validated elsewhere, so giving up after 100ms is recoverable where an unbounded loop is not.nanosleep()rather thanusleep()because it is not marked obsolescent, and bmalloc already builds with-D_GNU_SOURCE(Source/bmalloc/CMakeLists.txt) so<time.h>declares it. The Windows branches of both headers never expand these macros.Verification
Both macros exercised against the real headers, compiled with the flags bmalloc uses (
-std=c++20and-std=gnu11 -D_GNU_SOURCE):Source/bmalloc/libpas/src/libpas/{pas_page_malloc,pas_committed_pages_vector,pas_thread_local_cache}.cand a TU includingVMAllocate.hall compile with no new warnings againstorigin/main.Not covered here: a full JSC run proving the
[anon:WKFastMalloc]mapping count stays flat under a real workload. CI build + the JSC stress suite is the check I can offer.Credit
#169 by @coleleavitt diagnosed the spin loop and proposed both halves of this fix in February. This PR rebases that work onto current
main, adds thevm.max_map_countcase (where theEAGAINis permanent, so the retry cap rather than the backoff is what ends the hang) and the VMA-fragmentation measurement that explains how a process gets there in the first place.Related: oven-sh/bun#17723, oven-sh/bun#27490.