Fix fork reset mmap source#88
Open
perbu wants to merge 2 commits into
Open
Conversation
…e tables The reset_keep_all_work_memory fast path restored each dirty bank page with main_memory().safely_at(virtual_addr), treating the recorded virtual address as physical. That only holds for identity-mapped main memory: pages backed by the mmap physical region (MMAP_PHYS_BASE) are not identity-mapped, so every recorded page living there was silently "restored" from an unpopulated main-memory page — writing zeros into the fork's bank page. The 2MB-leaf case was doubly wrong: the shadowed lambda parameter made the copy source the bank's physical block address rather than any guest virtual address. CPython 3.14 places PyInterpreterState on mmap-region pages; a recycled fork then read interp->obmalloc as NULL and faulted in obmalloc's arena_map_get on its next turn — deterministically, every second call. 3.12 survived only because nothing it reads back post-reset happens to live on mmap-backed pages. Resolve each 4K source page through the master's page tables with readable_page_at, which handles both identity-mapped and mmap-backed physical pages, and returns the correct 4K segment when the master holds a 2MB huge page. A 2MB fork leaf is restored one 4K page at a time, covering masters with either page size underneath. The loop is arch-neutral (the arm64 backend has its own readable_page_at); the PRESENT/VALID flag ifdef follows the existing pattern in this file. Repro: fast-agent's fa-exec-bench with CPython 3.14 failed exactly half its calls (fresh mint OK, first recycle faults); with this fix 3.14 runs 400/400 clean at threads=4, 3.12 unchanged, reset p50 still ~15-20us. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
readable_page_at throws when the master's entry is not present, and unpresent-but-cloneable master entries are real: anonymous memory the master mapped but never touched keeps an unpresent CoW entry, and a fork writing to such a page records it in cow_written_pages like any other. Before this change, one such recorded page made every fork_reset throw mid-loop and land in the full bank-reset fallback -- silently and permanently demoting that fork off the fast recycle path. The master is frozen after fork, so an entry unpresent at reset time was unpresent when the fork wrote, i.e. exactly the unpresent copy-on-write case in writable_page_at. A fresh fork writing to such a page goes through zero_and_update_entry (an unpresent entry carries no DIRTY bit) and observes zeros, so zeroing the recycled fork's page reproduces fresh-fork semantics bit-for-bit. Catch per page and page_memzero the destination; the happy path pays nothing, and the outer catch remains the last resort for genuinely unexpected failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I ran into an issue with Python 3.14. First attempt at a fix had some potential performance on a throw, that is addressed in the second commit.
The
reset_keep_all_work_memory=truefast recycle path invMemory::fork_resetrestores each dirty bank page with:
safely_at(virtual_addr)treats the recorded guest-virtual address as aphysical one. That only holds for identity-mapped main memory. Pages backed by
the mmap physical region (
MMAP_PHYS_BASE) are not identity-mapped, so forevery recorded page living there the copy sourced an unpopulated main-memory
page — silently writing zeros into the fork's bank page on every reset.
The 2MB-leaf case was doubly wrong: the lambda parameter shadowed the loop
variable, so the copy source was the bank's physical block address rather
than any guest virtual address.
Observed impact
CPython 3.14 places
PyInterpreterStateon mmap-region pages. A recycled forkreads
interp->obmallocas NULL on its next turn and faults in obmalloc'sarena_map_get— deterministically, every second call (fresh mint OK →recycled fork faults → retire/remint repeats). CPython 3.12 survives only
because nothing it reads back post-reset happens to live on mmap-backed pages;
the copy-back corrupts them there too.
page-table walk and from the vCPU (not stale TLB); the master's true page
(PTE → mmap region) holds the pointer while the identity read returns zeros;
and
reset_keep_all_work_memory=falsemakes the failures vanish.Fix
Commit 1 — resolve each 4K source page through the master's page tables
with
readable_page_at, which handles identity-mapped and mmap-backedphysical pages alike and returns the correct 4K segment when the master holds
a 2MB huge page. A 2MB fork leaf is restored one 4K page at a time, covering
masters with either page size underneath. The loop is arch-neutral (the arm64
backend has its own
readable_page_at); the PRESENT/VALID flag ifdef followsthe existing pattern in this file.
Commit 2 —
readable_page_atthrows when the master's entry is unpresent,and unpresent-but-cloneable entries are real (anonymous memory the master
mapped but never touched). Before this commit, one such recorded page made
every
fork_resetthrow mid-loop into the full bank-reset fallback — silentlyand permanently demoting that fork off the fast recycle path. The master is
frozen after fork, so an entry unpresent at reset time was unpresent at write
time — exactly the unpresent-CoW case in
writable_page_at, where a freshfork gets a zeroed page (
zero_and_update_entry, no DIRTY bit). Catching perpage and
page_memzero-ing the destination reproduces fresh-fork semanticsbit-for-bit; the happy path pays nothing and the outer catch remains the last
resort for genuinely unexpected failures.