Repository navigation
x86/mem_sharing: complete a fork without re-walking both p2ms per frame - #52
Merged
Merged
Conversation
azenla
requested review from
alexandermerritt,
bleggett,
kaniini and
tycho
as code owners
October 17, 2026 02:25
azenla
force-pushed
the
azenla/perf/fork-complete-speed
branch
from
October 17, 2026 02:26
4ae0032 to
eedcd0c
Compare
…p2ms Completing a fork is the parent's downtime, and almost all of it: 111ms of the 131ms a 512 MiB PVH guest's parent spends suspended, at some 850ns for each of its 131072 frames. Most of that is lookups the loop already has the answers to. Reaching mem_sharing_fork_page() through get_gfn() costs three walks of the child's p2m per frame -- one to find the hole, one inside add_to_physmap()'s get_two_gfns(), and one to read back what was just written -- plus a lock, an unlock and a deferred TLB flush around each. A bulk materialisation knows all of that without asking: it is walking the parent's physmap in order, and every frame it has not reached yet is a hole. So look the child's entry up once, call the fork-in page directly, and hold the child's p2m lock across a run of frames instead of taking it per frame -- which also means the flush the unlock performs happens a few times over a guest rather than 131072 times. Holding the lock is safe for the same reason the whole operation is: the child is paused and not yet reachable by anything else. It is dropped before every preemption, because a continuation returns to the caller, and dropped and retaken at the same boundary when the call is not preempted, so the hold stays bounded either way. The nesting is unchanged. get_gfn() already held the child's p2m across nominate_page() and add_to_physmap() taking the parent's; this holds the same two in the same order, just for longer. Worth 19ms of the parent's downtime: complete goes 111.5ms -> 92.8ms for the first fork of a parent, and 86ms -> 68.6ms for later ones, where the parent's frames are already shared and only the child-side insert is left. Also say when a frame could not be materialised. It leaves a hole in a child that is about to lose the parent it would have resolved that hole through, and it was silently ignored before. Still not fatal -- the guest may never touch the frame -- but not something to first learn about from the guest. Verified against copy-on-write rather than only against the clock: a parent seeded with 48 MiB of a known pattern, forked, then both sides diverged. The child reads the fork-time contents after the parent overwrites all of it, the parent is unaffected by the child's writes, a fork of the fork inherits the child's state, and a child whose parent has been destroyed still reads 48 MiB it never touched since the fork. No frames went unmaterialised and the hypervisor console is clean. Signed-off-by: Alex Zenla <alex@edera.dev>
Completing a fork took the parent's p2m lock once per frame. That lock is recursive, so holding it across a run of frames makes nominate_page()'s lookup and add_to_physmap()'s insert find it already held and only count the recursion, and the deferred TLB flush each of their unlocks would have performed happens once at the end of the run instead of twice per frame. Holding both across the run is safe for the same reason the operation is: both domains are paused, and the child is not reachable by anything else yet. The order is the child's and then the parent's, which is the order get_gfn() already nested them in, and both are dropped before every preemption.
azenla
force-pushed
the
azenla/perf/fork-complete-speed
branch
from
October 17, 2026 02:26
eedcd0c to
761e04b
Compare
alexandermerritt
approved these changes
Oct 17, 2026
1 task done
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
2 participants
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.
Description
mem_sharing_fork_complete()walks the parent's whole physmap and forks every frame into the child. It did that throughget_gfn()for each 4K frame, which costs three walks of the child's p2m plus a lock, unlock and deferred TLB flush per frame.This takes both p2m locks once per run of frames and calls
mem_sharing_fork_page()directly for each hole, so the lookups the loop already has are not repeated and the flush happens once per run. Both locks are dropped before every preemption and every 8192 frames. A frame that cannot be materialised now logs a warning instead of being skipped silently.Measured on a PVH dom0 with protect, 1 vCPU PVH zones, forking a freshly launched parent. "complete" is the time
fork_completetakes, as logged by the toolstack:Over 40 forks of a 512 MiB zone,
protect zone fork --waitwent from a mean of 308 ms (median 328 ms) to a mean of 129 ms (median 121 ms, p90 147 ms).Copy-on-write checked with a 64 MiB random file in the parent's tmpfs: the child reads the fork-time contents after the parent overwrites them, the parent does not see the child's writes, and the child still reads correctly after the parent is destroyed and can allocate fresh memory. No warnings or asserts appeared in the Xen console.
The measured build also carried
8522d31c98, which this branch does not include.