[Japabu]

Problem

A DATA listing entry carries its file's size and kind, so DataVolume::list read each entry's inode with Mount::inode: one search of the inodes btree from its root per name, after the dirents in batches of 256, each batch reading the directory's own inode again. On the host model of the T14's DATA those lookups were 416 us of a 1 000-entry listing's 534 us (#1152's evidence); fs.disk.readdir.1000 stood at about 3x main's on both benches.

Resulting behaviour

The engine answers a listing itself. Source::listing reads a directory's names in one walk of the dirents and their inodes with Source::inodes, which takes the numbers in ascending order and keeps one walk of the inodes btree going while the next number is within NEAR (32) of the last. The keys between are stepped over where they lie, far fewer than a leaf holds; a number further on is found from the root as before, so a directory whose inodes are scattered costs what it did. Two names of one inode are answered from one read, and a name whose inode is absent refuses the listing NotFound, as the lookup alone did. DataVolume::list maps and sorts what the engine answers.

The record's other half stands: every READDIR request still lists the directory whole. The record is renamed to say that (issues/a-data-directory-listing-is-read-whole-for-every-readdir-request.md) and keeps its exit on both benches' readdir.1000.

Checks

Host, macOS arm64, at 7d00a1f (this change on 422915e; review r1's 506f6f8 changes two comments only, and cargo clippy -p bcachefs --all-targets is clean there, exit 0; it merges with main b566931 without conflict, and nothing since touches lib/bcachefs or system/fileserver):

• cargo test -p bcachefs -p fileserver: exit 0 (bcachefs 201, fileserver 122 and osbench_replay 3).
  • a_listing_reads_its_inodes_in_a_pass_over_each_run_of_them wraps the mount in a Source that counts walks, each of which starts from a root. A directory of 1 000 files made in a row, a second name for one and a file made after 40 elsewhere lists in 4 walks: the directory's inode, its names and the two runs of inodes. Every answer is compared with Mount::inode's. Negative control: with NEAR 0 the count is 1 003 and the test is red.
  • inodes_read_together_are_those_read_alone: any order, a number twice, none, and an absent number first, in the middle and last.
  • system/fileserver's a_listing_says_of_each_entry_what_its_path_does holds each listed entry to lstat and identity of its own path after a remount.
• cargo clippy -p bcachefs --all-targets with the gate's adopted lints and -D warnings: exit 0. The file server is a program, which the gate does not lint; run anyway, it finds four things in files this PR does not touch (listing.rs, pages/tests.rs, writeback.rs) and none in data.rs.

The full --ci host was not run: the change is two packages' code, and their tests and lint are above.

Metal is no longer a landing condition: both benches are down. The speed-up is measured on the T14 alone; on the AMD bench it is not yet measured. Owed on metal (recorded in issues/a-data-directory-listing-is-read-whole-for-every-readdir-request.md): both benches, osbench_disk's fs.disk.* rows in one A/B of main before and after this PR; on the AMD bench main after must show readdir.1000 below main before on every boot, and on both no other row may resolve against main after, which is where open_close and stat (below) recur or not.

The T14 A/B, done before the benches went down: a one-bench A/B of osbench_disk (the fs.disk.* rows) on the T14, H = 7d00a1f against its base B = 422915e, four boots a side in the counterbalanced order B, H, H, B, H, B, B, H set before any boot. r1's comments leave the measured code as it is.

• Valid boots: 8 of 8. Each judged offline with the osbench_disk row: PASS, which holds data_on_the_nvme (DATA ours, mounted, not formatted by the boot); no boot formatted DATA; readdir.1000 reported on every boot. This was also the listing's first run on a real drive.
• readdir.1000 resolves, H faster: B 2.869 to 2.914 ms, H 2.384 to 2.400 ms, every H below every B; H/B 0.826 (exact two-sided Mann-Whitney, p = 2/70). Far less than the host model promised; the record says so and stays open.
• Two other rows resolve against H: open_close (B 21.73 to 21.89 us, H 22.05 to 22.29 us; H/B 1.018) and stat (B 12.21 to 12.36 us, H 12.50, 14.84, 14.13, 12.52 us; H/B 1.081, carried by two boots). Neither path lists anything. Disassembling both builds' fileserver, function by function with addresses removed: DataVolume::lstat, stat and open are the same code in H as in B, and so are 3 589 other functions, every one at another address in H. The one change on their path is Mount::inode's future, which in H calls the out-of-line decode_inode (with time inlined into it) where B decoded inline (calling time four times): the same calls into InodeValue, one call and return more per inode read. That is nanoseconds against the 0.3 to 1 us the rows moved, so the change adds no work there that would explain them. Code placement and boot-to-boot variance would; that is not shown by a control.
• Every other row is unresolved. Each row's H/B, B median to H median: reopen_read_4k 0.972, fstat 1.010, create_write_unlink 1.003, read_4k_at 0.971, write_4k_at 0.978, seek 0.790, read_4k 1.035, pread_4k 1.002, pread_8 1.058, direct.seek 0.991, direct.read_4k_at 1.004, direct.pread_4k 1.090, direct.fstat 1.061, direct.stat 1.023, rename 0.990, mkdir_rmdir 0.951, tree.100 1.027, seq_write.4096 1.031 (MB/s), seq_read.4096 0.988 (MB/s), seq_write.1048576 0.983 (MB/s), seq_read.1048576 1.007 (MB/s). It is also the run in which the listing reads a real DATA volume on a real drive. No guest row is dispatched: the boot's DATA rows are where this path runs, and they are the metal row's.

Evidence: https://github.com/ToyOSOrg/evidence/tree/9124a882ece9c3441d9894a5101d78221401134b (host logs, the control patch, and under metal/ the A/B: rules, every boot's fs.disk rows and verdict, the offline judgments, the table, and the code comparison).

Risks

• NEAR is a reasoned constant (keys stepped over against a root-to-leaf search), not a measured one, as its comment now says.
• The single pass does not bring readdir.1000 within the record's 1.5x of main's (2.39 ms against about 1.37 ms); the record stays open with the measurement, and where the remaining time goes is not measured.
• open_close and stat resolve against H by 1.8 % and 8 %; the compiled code says the change does no more work on those paths, but no layout control has shown the cause.

🤖 Generated with Claude Code

[Japabu]

Review of #1236 at 7d00a1f

Architectural assessment

• Ownership is right. The engine answers a listing with names and inodes together. bcachefs decides how its btrees are walked, and the file server only maps and sorts. Source::inodes with a NEAR bound is small and general, and a directory with scattered inodes costs what it did. Nothing changes the on-disk format, so the A/B's premise that DATA carries over between boots holds.
• Concurrent change. In the file server nothing can change during a pass. A listing is one request run to completion by Cache::block_on, whose executor holds no other task, and no background task changes the namespace.
• The engine's own terms, if tasks did interleave on one Mount:
  • The names are one snapshot from one walk of the dirents.
  • Mount::walk finds each leaf from the root by position, so a key present throughout is seen exactly once.
  • An inode created inside a run is stepped over.
  • An inode released before its turn refuses the whole listing NotFound. The old per-entry Mount::inode did the same, and the old mapped("directory entry") also failed the whole listing.
• POSIX. POSIX leaves it unspecified whether readdir returns an entry added or removed after opendir/rewinddir. Refusing the whole listing is stricter than POSIX needs, but unchanged and unreachable today. Changes between READDIR batches remain the listing cursor's job, held by listing::tests::removing_what_was_listed_skips_nothing_after_it and a_name_added_mid_listing_costs_no_other_entry.
• The walk-count test is a cheap, valid host guard for "not one search per entry":
  • The wrapper's inherited default get also goes through walk, so a return to per-entry lookups would show.
  • The control NEAR = 0 gives 1 003 walks and turns the test red.
  • The == 4 assertion also goes red if the run break is lost: ignoring NEAR gives 3 walks.
  • Every answer is compared with Mount::inode's.
• The other tests. inodes_read_together_are_those_read_alone covers unsorted input, a duplicate, an empty request, and an absent number first, in the middle and last. The file server test holds each listed entry to lstat and identity after a remount.
• The record rename. The new slug states what remains. The citation in smaller-data-rows-trail-main-by-up-to-half.md was moved, and main has no other reference to the old slug. The new opened date follows issues/README.md for a new heading.
• Evidence.
  • Host tests exit 0 at 7d00a1f.
  • The control log shows 1 003 walks against 4.
  • The branch merges cleanly with main b566931, which changes nothing under lib/bcachefs or system/fileserver, so the host evidence carries over to the merge.
  • The file server's four clippy findings are in untouched files and covered by issues/userland-programs-are-never-linted.md.
• Size. About 45 production lines added in read.rs and ops.rs, about 20 removed from data.rs; about 120 lines of tests.

Required

None in the code.

The metal A/B's rules

• The statistics are stated correctly. With 4 boots a side, an exact two-sided Mann-Whitney test resolves only on complete separation, p = 2/70 ≈ 0.029. A watched row resolves by chance about 2.9% of the time (about 1.4% against H).
• The boot order needs changing before any boot runs. B,H,B,H,B,H,B,H puts every H boot right after a B boot.
  • DATA carries over: each boot's osbench_disk leaves journal and btree history behind, and makes its listed directory at ever-higher inode numbers.
  • Any steady drift is therefore confounded with the change. Complete separation in a low-noise watched row is exactly what a small steady drift can produce.
  • No boot has run yet, so a counterbalanced order costs nothing, e.g. B,H,H,B,H,B,B,H.
  • This is a correction to the rules, not to the code.
• The no-regression watch is weak at 4 a side. It flags only complete separation. That is acceptable here because inspection shows only DataVolume::list changed: no other timed fs.disk row lists a directory (the listed directory's removal lists it, untimed). It would still help to report each watched row's ratio of medians and to name any row whose H range lies wholly above its B range.

What the A/B must show to land

1. Valid boots. Report the count if fewer than eight. In each:

   • the data_on_the_nvme judge passes;
   • DATA is not reformatted;
   • on every H boot, readdir.1000 reports (its own check that 1 000 names were listed).

   This is also the first run of the new listing path on a real drive.

2. The primary row. For readdir.1000, every H boot's p50 is below every B boot's: the result resolves. The host model (inode lookups 416 of 534 µs) predicts this. If it does not resolve, that contradicts the PR's premise and is investigated before landing, not landed on direction alone. The record's exit (about 1.37 ms) is reported, not required: the record stays open either way.

3. No regression elsewhere. No other fs.disk.* row resolves against H. One that does is explained before landing.

Suggestions

• S1. lib/bcachefs/src/engine/tests/ops.rs:300-301: "each is one search of a btree from its root" is not exact. Mount::walk finds every leaf from the root (lib/bcachefs/src/engine/mount/mod.rs:1018-1036), so a walk that crosses a leaf boundary makes several searches. The test counts walks; say that each starts from the root. The PR description repeats the claim.
• S2. lib/bcachefs/src/engine/read.rs:87-89: "so the steps cost less than finding the second from the root" is an inference. The PR's Risks section says NEAR is "reasoned, not measured", and the comment should say so too.

LAND, pending the metal A/B above, run in a counterbalanced order.

[Japabu]

Orchestrator decision on the A/B (8 of 8 boots valid, B-1 H-1 H-2 B-2 H-3 B-3 B-4 H-4):

• readdir.1000 resolves faster under H, 2.87–2.91 ms to 2.38–2.40 ms (H/B 0.826). The record's 1.37 ms exit is not met, so it stays open with the measurement.
• Two non-listing rows resolve against H. open_close is +1.8%, below the 3% this project counts as a change elsewhere. stat's +8.1% median rests on two H boots (14.84 and 14.13 µs), while the other two H boots sit 1.1–1.3% above B's slowest. Both paths run code that is function-for-function identical apart from one out-of-line decode_inode call per inode read, which costs nanoseconds against moves of 0.3–1 µs. Two of 23 rows resolving by chance has a probability of about 4.5%.
• I accept this as code placement and boot variance, not a regression caused by the change. Criterion 3's explanation is recorded here and in the body's table.
• The first review's LAND stands. Queueing.