A paged read takes two passes of its pager's lock and populates nothing ahead

issues/a-large-spawn-on-the-amd-bench-is-a-seventh-slower-than-mains.md

@@ -9,18 +9,19 @@ opened: 2026-11-05
 `osbench`'s `proc.spawn_wait.large` spawns a 32 MiB program from a served
 file. The spawner reads the image through `SYS_PAGED_READ` into fresh shared
 memory. #1150 copies it a frame at a time through the reader's own mapping
-(`arch::uaccess`), after faulting each chunk of 16 frames in
-(`PagerObject::read`, `UserBytesMut::fault_in`). Main copied it through the
-kernel's direct map, reaching and pinning each page as it went.
+(`arch::uaccess`, `PagerObject::read`). Main copied it through the kernel's
+direct map, reaching and pinning each page as it went. The copy's fault on
+the shared memory maps its 64 KiB run (`kernel::vm::around`); the figures
+below were read when a pass of its own faulted each chunk of 16 frames in
+first.
 
 On the AMD bench (metal round 8; evidence `ToyOSOrg/evidence`, branch
 `pr-1150`, r15):
 - **The row:** 12.79 and 12.75 ms against main's 11.00 and 11.53, worse in
   both pairs.
 - **The copy:** 760 ns per page against 685 for main's whole reach, pin and
   copy; with 8192 pages a spawn, that is 0.6 ms.
-- **The populate:** `fault_in` adds 48 ns per page in a pass of its own,
-  0.4 ms a spawn.
+- **The populate:** that pass added 48 ns per page, 0.4 ms a spawn.
 - **Unaccounted:** about 0.5 ms of the 1.5 ms.
 
 Before #1150's accessed-and-dirty change to x86-64 user leaves the copy cost
@@ -29,10 +30,17 @@ better than main's.
 
 **Hypothesis, unmeasured:** the remaining copy cost is translating each
 destination page through the 4 KiB user mapping, where main's direct map uses
-huge pages. The separate populate pass touches each page's leaf a second time.
-No performance counter has been read.
+huge pages. No performance counter has been read.
 
 **Exit:** the row within main's run-to-run range on the AMD bench, or the
 per-page difference measured by counter (TLB misses and walk cycles, against
 main) and recorded here as the cost of copying through the user mapping that
 #1150's design accepts.
+
+- **Owed on metal:** #1245 copies the image through faults that map 64 KiB
+  runs of the shared memory, where a populate pass ran first. In the A/B of
+  main from before and after #1245
+  (`issues/user-memory-hot-paths-are-slower-than-2-mib-pages.md`), on both
+  benches, `proc.spawn_wait.large` and `ab.fs.memory.shm.16777216`, the same
+  copy without the spawn, must not regress: no rise in all four paired boots
+  with a median ratio above 1.03.

issues/user-memory-hot-paths-are-slower-than-2-mib-pages.md

@@ -10,11 +10,11 @@ User memory moved from 2 MiB pages filled when mapped to 4 KiB pages filled
 at first touch (`kernel/src/mm/space.rs`). Each of these costs more than it
 did; the T14's numbers are below:
 
-- **Faults.** A touch beside a populated page of a private range fills its
-  aligned 64 KiB run (`kernel::vm::around`), so memory touched in order takes
-  a fault per 16 pages; a lone touch is still one fault per page. Each fault
-  takes the space's lock, an inventory lookup, one table walk and, per frame,
-  the frame allocator's one lock
+- **Faults.** A touch beside a populated page fills its aligned 64 KiB run
+  (`kernel::vm::around`), or maps it in a range of a memory object, so memory
+  touched in order takes a fault per 16 pages; a lone touch is still one fault
+  per page. Each fault takes the space's lock, an inventory lookup, one table
+  walk and, per frame, the frame allocator's one lock
   (`issues/per-cpu-scheduler-and-pmm-state-shares-lines-every-cpu-writes.md`)
   and a 4 KiB zeroing. Probed on the T14 at `b48c7dd9d` (scratch probe, one
   frame per fault): about 345 ns from the trap handler's entry to its return —
@@ -46,8 +46,9 @@ did; the T14's numbers are below:
   held an owned frame (at most 512 frames each), whether or not anything waits.
 - **Commits.** A committed range's tables are built when it is made, under the
   space's lock: 1/512 of the range.
-- **Copies.** One round trip of the space's lock per 16 pages, plus an
-  inventory lookup and a table walk per page.
+- **Copies.** A copy into memory nothing has touched takes the faults a touch
+  would, one per 64 KiB run, each an exception's delivery and return on top
+  of a touch's. Into present pages it takes none.
 - **Spawn.** A segment is copied a page at a time at first touch, so while a
   child lives the image is held twice: once by the spawner's object or the
   `/tmp` file's snapshot, which the child's ranges keep alive, and once
@@ -130,3 +131,16 @@ accepted by the owner, or by the coordinator for an implementation tradeoff,
 which the owner delegated on 2026-10-23. Cheap remedies when measurement asks: a
 per-CPU frame cache, a count of armed futex waiters that lets a clear
 with none skip the buckets, and tables built at the first touch of their span.
+
+- **Owed on metal:** #1245, which drops the paged read's pass that faulted its
+  destination in and its third pass of the pager's lock, is not measured on
+  metal; its speed-up is a prediction. Owed: one A/B of main from before and
+  after it on both benches, four boots a side, each running `osbench pipeab`
+  (the measurement-only commit on `pipeperf-ab-base`). A row changes where all
+  four paired boots move one way and their median ratio is beyond 1 ± 3%.
+  Main after must show `ab.fs.memory.pread.8.d0` and
+  `ab.fs.memory.pread.1048576.d0` lower (predicted: about 45 ns of 470 to 520,
+  and 5 to 9%); `ab.fs.memory.fresh.*` and `ab.fs.memory.shm.*`, reads into
+  memory nothing has touched and the price of the change, unchanged or their
+  loss recorded here against the gain; and main's `testcases` and
+  `shared-debug` boots passing on both benches.

kernel/src/mm/space.rs

@@ -790,30 +790,32 @@ impl AddressSpace {
                 leaves.set(page, Leaf::Mapped(Mapping { phys, prot, owned: true, cache: CachePolicy::Normal }));
                 phys
             }
-            Leaf::Empty => match &entry.kind {
-                Kind::Private { committed: false } => unreachable!("space: a reservation at {page:#x} allows an access"),
-                Kind::Object { object, offset, .. } => {
-                    let phys = object.page(offset + (page - start));
-                    leaves.set(page, Leaf::Mapped(Mapping { phys, prot, owned: false, cache: object.cache() }));
-                    phys
-                }
-                Kind::Private { committed: true } | Kind::Copy { .. } => {
-                    // A neighbour in the next table down or up is another run's.
-                    let (lo, hi) = vm::around(start, entry.end, page, |at| leaves.covers(at) && leaves.get(at) != Leaf::Empty);
-                    let (mut filled, mut own) = (0, 0);
-                    leaves.fill(lo, hi, |at| {
-                        let phys = owned_frame(start, entry, at);
-                        filled += 1;
-                        if at == page {
-                            own = phys;
-                        }
-                        Leaf::Mapped(Mapping { phys, prot, owned: true, cache: CachePolicy::Normal })
-                    });
-                    assert_ne!(own, 0, "space: {page:#x} touched and not filled");
-                    self.usage.populated(filled);
-                    own
+            Leaf::Empty => {
+                // A neighbour in the next table down or up is another run's.
+                let (lo, hi) = vm::around(start, entry.end, page, |at| leaves.covers(at) && leaves.get(at) != Leaf::Empty);
+                match &entry.kind {
+                    Kind::Private { committed: false } => unreachable!("space: a reservation at {page:#x} allows an access"),
+                    Kind::Object { object, offset, .. } => {
+                        let phys = |at: u64| object.page(offset + (at - start));
+                        leaves.fill(lo, hi, |at| Leaf::Mapped(Mapping { phys: phys(at), prot, owned: false, cache: object.cache() }));
+                        phys(page)
+                    }
+                    Kind::Private { committed: true } | Kind::Copy { .. } => {
+                        let (mut filled, mut own) = (0, 0);
+                        leaves.fill(lo, hi, |at| {
+                            let phys = owned_frame(start, entry, at);
+                            filled += 1;
+                            if at == page {
+                                own = phys;
+                            }
+                            Leaf::Mapped(Mapping { phys, prot, owned: true, cache: CachePolicy::Normal })
+                        });
+                        assert_ne!(own, 0, "space: {page:#x} touched and not filled");
+                        self.usage.populated(filled);
+                        own
+                    }
                 }
-            },
+            }
         };
         Some(phys + va % PAGE_SIZE)
     }

kernel/src/object/paged.rs

@@ -472,8 +472,7 @@ impl PagerObject {
                 let size = state.entry(slot).object.size();
                 if state.gone || state.entry(slot).object.gone() {
                     Step::Gone
-                } else if done == want || (done == 0 && at >= size) {
-                    // Whole, each copy counted as it ended; or nothing to copy.
+                } else if done == 0 && (want == 0 || at >= size) {
                     Step::Done
                 } else if waits && !Self::unwritten(&state.entry(slot).object, &mut since) {
                     Step::Written
@@ -486,14 +485,7 @@ impl PagerObject {
                 }
             };
             match step {
-                Step::Copy(count) => {
-                    // A chunk of frames is copied a frame at a time: populated
-                    // first, fresh memory takes one hold rather than a fault
-                    // for each page. A one-frame read, the common small one,
-                    // would pay that hold on every call to save at most two faults.
-                    if count > 1 {
-                        out.fault_in(done, pieces[..count].iter().map(|piece| piece.len).sum());
-                    }
+                Step::Copy { count, last } => {
                     let mut faulted = false;
                     for piece in &pieces[..count] {
                         let copied = match piece.phys {
@@ -528,6 +520,9 @@ impl PagerObject {
                     if faulted {
                         return if done > 0 { done as u64 } else { SyscallError::BadAddress.to_u64() };
                     }
+                    if last {
+                        return done as u64;
+                    }
                 }
                 Step::Written => return SyscallError::WouldBlock.to_u64(),
                 Step::Done => return done as u64,
@@ -619,7 +614,7 @@ impl PagerObject {
                 Lookup::PastEnd => return Step::Done,
             }
         }
-        Step::Copy(count)
+        Step::Copy { count, last: pos == range.end }
     }
 
     /// A write call on object `id` begins: its slot is held by a reference
@@ -942,8 +937,8 @@ struct Piece {
 
 /// What one look at the next pages found.
 enum Step {
-    /// This many pieces were pinned.
-    Copy(usize),
+    /// This many pieces were pinned, `last` where they reach the read's end.
+    Copy { count: usize, last: bool },
     Absent(u64),
     /// A write met a holder's read.
     Written,

kernel/src/process.rs

@@ -1009,17 +1009,6 @@ pub fn handle_page_fault(fault_addr: u64, access: kernel::vm::Access) -> bool {
     served
 }
 
-/// Populate the running thread's pages over `[start, end)` for `access`, in
-/// one hold of its space's lock, stopping at the first its ranges refuse.
-pub fn fault_in(start: u64, end: u64, access: kernel::vm::Access) {
-    let Some(space) = scheduler::current_address_space() else { return };
-    let mut space = space.lock();
-    let mut page = start & !(crate::mm::PAGE_SIZE - 1);
-    while page < end && space.fault(page, access) {
-        page += crate::mm::PAGE_SIZE;
-    }
-}
-
 /// Dump memory around `fault_addr` and the instruction pointer for the
 /// current process; called from the exception handler on user-mode crashes.
 pub fn dump_crash_diagnostics(fault_addr: u64, rip: u64) {

kernel/src/user_ptr.rs

@@ -265,12 +265,4 @@ impl UserBytesMut<'_> {
     pub fn fill_zero(&mut self, off: usize, len: usize) -> Result<(), Faulted> {
         self.0.fill_zero(off, len)
     }
-
-    /// Populate the window's pages under `[off, off + len)` for writing, in
-    /// one hold of the space's lock, up to the first its process may not
-    /// write; the copy faults on any left out. `len` bounds the hold.
-    pub fn fault_in(&mut self, off: usize, len: usize) {
-        let start = self.0.at("UserBytesMut::fault_in", off, len);
-        crate::process::fault_in(start, start + len as u64, kernel::vm::Access::Write);
-    }
 }