dd: page-align the read buffer for iflag=direct

[relative23]

Fixes #12085.

Problem

dd iflag=direct can fail with EINVAL because Vec<u8> does not guarantee the alignment required by O_DIRECT.

Fix

Use a safe Vec-backed buffer that exposes a page-aligned fixed-size slice. Read helpers return the valid prefix instead of resizing the buffer, and the initial buffer size respects count when less data is requested. This adds no dependency or unsafe code.

Validation

• cargo test -p uu_dd
• dd integration suite, including FIFO partial-read coverage and the bounded-allocation regression
• CodSpeed reports no performance change

[relative23]

Updated the description to properly credit #12143, which I had missed when opening this (my issue search didn't match its title). See the "Relation to existing PRs" section.

[codspeed-hq]

Merging this PR will not alter performance

✅ 339 untouched benchmarks
⏩ 46 skipped benchmarks¹

---

_{Comparing relative23:dd-odirect-buffer-alignment (8d2d6ca) with main (4daa891)²}

Footnotes

1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

2. No successful run was found on main (e5fcabf) during the generation of this report, so 4daa891 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/seq/seq-epipe is now passing!

[relative23]

Thanks for the report — breakdown of the two kinds of deltas:

Memory (all six: exactly +4.0 KB): this is the intentional over-allocation by one page that provides the alignment guarantee (capacity + page_size, the safe-code equivalent of GNU dd's ptr_align(ibuf, page_size)), so it's a constant +4 KiB regardless of block size, not a leak or scaling issue.

Simulation (−4…−5%): per-iteration bookkeeping of the new buffer wrapper (capacity check + slice re-borrow), which adds up at bs=512/bs=4K where the copy loop runs tens of thousands of iterations. Addressed in 9eecf33 with a steady-state fast path in resize plus inlining. Local divan wall-time medians (same machine, 100 samples), main vs. this branch after the fix:

bench	main	this PR
dd_copy_default	23.8 ms	23.0 ms
dd_copy_8k_blocks	6.68 ms	6.39 ms
dd_copy_64k_blocks	12.4 ms	12.0 ms
dd_copy_partial	749 µs	730 µs

Worth keeping in mind when weighing the remaining instruction-count delta: these benchmarks don't use direct I/O, and CodSpeed doesn't measure syscall cost — while the bug this PR fixes currently makes every full-block oflag=direct write go through a failed write + two fcntl calls per block (and makes iflag=direct reads fail outright).

(The Android job failure is unrelated — it died in "Create and cache emulator image".)

[relative23]

Verified the fast path against the gate's own metric (instruction counts, single-pass analysis-mode bench binaries under valgrind --tool=cachegrind, whole-process I refs, main vs. this branch):

bench	main	this PR	delta
dd_copy_default	37,668,379	38,195,426	+1.40%
dd_copy_4k_blocks	4,741,075	4,809,777	+1.45%
dd_copy_partial	3,229,459	3,244,042	+0.45%

Down from the reported 4–5% to well under 1.5% (≈8 instructions per copied block, from the remaining slice re-borrows). The constant +4 KiB in the memory-mode numbers is the page over-allocation that provides the alignment guarantee itself.

[relative23]

Round 3, following up on the flame graphs from the last run: Input::read and Output::write_blocks were instruction-identical to main, so the residual simulation delta sat in the copy loop itself — every Deref of AlignedBuffer re-materialized the data slice (bounds check + RawVec::ptr reconstruction, ~4% combined on dd_copy_4k_blocks), plus the leftover per-iteration resize/truncate bookkeeping.

Rather than keep shaving at wrapper overhead, b859333 removes it structurally by consolidating with the read path from #12143 (credit to @chrboe — this is essentially his slice-based design on top of the safe runtime-page-size allocation):

• the copy loop takes the byte slice once, outside the loop; AlignedBuffer shrinks to an allocation helper and its Deref never appears in the hot loop
• fill_consecutive/fill_blocks operate on &mut [u8] and return the number of valid bytes instead of truncating
• conv=block/unblock output moves to a separate scratch Vec (those conversions can change the byte count); everything else stays in place
• the per-iteration resize is gone entirely, which also retires the dd takes a really long time to allocate memory with large buffers (tested with 1G) #11544 concern for this loop

The diff is net −75 lines. This run's CodSpeed result reflects it: no simulation benchmark is flagged anymore (previously default/4k_blocks/with_skip/with_seek at −3.2 to −3.8%). Local cachegrind on the codspeed analysis builds, same machine and toolchain for both sides (I refs, main 3aa5f8a vs this branch):

benchmark	main	this PR	Δ
dd_copy_default	37,668,434	37,081,830	−1.56%
dd_copy_4k_blocks	4,741,093	4,691,701	−1.04%
dd_copy_with_skip	8,405,560	8,310,586	−1.13%
dd_copy_with_seek	8,505,965	8,408,019	−1.15%
dd_copy_partial	3,229,514	3,220,910	−0.27%
dd_copy_8k_blocks	4,409,293	4,375,979	−0.76%
dd_copy_64k_blocks	5,110,850	5,103,359	−0.15%
dd_copy_1m_blocks	9,415,242	8,422,033	−10.55%
dd_copy_separate_blocks	55,875,370	55,938,721	+0.11%

That leaves the headline number (−16.32%), which is now purely the seven Memory entries averaged: peak memory is up by exactly one page (+4,096 B, constant, not proportional to the buffer size) because the buffer over-allocates one alignment unit to place the data page-aligned. That is the fix itself — the same trade GNU dd makes via ptr_align (…, page_size) — so I'd suggest acknowledging those seven entries in CodSpeed rather than gating on them; happy to adjust if there's a preferred way to handle intentional memory deltas.

Re-verified after the refactor: cargo test -p uu_dd (90) and the dd integration suite (123) pass; outputs are byte-identical to main across an 18-configuration sweep (odd ibs/obs, swab, sync, block/unblock, count_bytes, pipes); on a dma_alignment=511 loop device, iflag=direct reads succeed and a 16-block oflag=direct copy performs 3 fcntl calls in total instead of 2 per block.

The other two failing checks look unrelated to this diff: the macOS job failed on a test_timeout signal-timing flake (all 112 dd tests pass there), and cargo-deny hit a broken action invocation this morning (unrecognized subcommand 'warn') that other PRs stopped seeing within the hour — both should clear on a re-run.

[relative23]

Round 4: the seven Memory flags are gone as well — 5868242 removes the one-page over-allocation instead of asking for it to be acknowledged.

AlignedBuffer now allocates exactly bsize bytes at the runtime page size via std::alloc::alloc_zeroed(Layout) rather than over-allocating a Vec by one alignment unit and offsetting into it. That was the source of the constant +4,096 B peak-memory delta on every dd benchmark; with the exact allocation the peak matches the plain Vec<u8> baseline byte for byte. DHAT (allocator-level peak heap, dd bs=4K over a 3 MB file):

revision	peak heap	vs main
main	118,055 B	—
previous (over-allocating)	122,117 B	+4,062 B
this revision	118,021 B	−34 B

Being explicit about the trade-off, since the PR previously advertised "no unsafe": this adds three small unsafe blocks (alloc_zeroed, dealloc, Deref via slice::from_raw_parts), each with a SAFETY comment. The pointer is owned by AlignedBuffer alone and freed with the same Layout it was allocated with — unlike #9104's memalign + Vec::from_raw_parts, it never crosses into an allocator-aware container, so the allocator contract holds. The unit tests pass under Miri; note that they need to be run against the module standalone (e.g. copied into a scratch crate), because the uu_dd test binary as a whole cannot run under Miri — uucore's startup code calls sigaction, which Miri does not support. I think exact allocation plus a small, auditable unsafe surface is the better end state than a permanent by-design CodSpeed exception; happy to hear if you see it differently.

Side effect on the hot loop: Deref via from_raw_parts is cheaper than the previous bounds-checked storage[offset..] reconstruction, so the simulation numbers improved again (cachegrind I refs on the codspeed analysis builds, same machine/toolchain both sides, main 058a2a6):

benchmark	main	this PR	Δ
dd_copy_default	37,670,482	36,753,717	−2.43%
dd_copy_4k_blocks	4,743,133	4,662,232	−1.71%
dd_copy_with_skip	8,407,642	8,252,223	−1.85%
dd_copy_with_seek	8,508,047	8,348,418	−1.88%
dd_copy_8k_blocks	4,411,333	4,356,792	−1.24%
dd_copy_partial	3,231,604	3,219,789	−0.37%
dd_copy_64k_blocks	5,112,862	5,099,334	−0.26%
dd_copy_1m_blocks	9,417,324	8,424,089	−10.55%
dd_copy_separate_blocks	55,877,451	55,928,424	+0.09%

Re-verified after the change: 91 unit + 123 integration tests pass, outputs byte-identical to main across the 18-configuration sweep, and on a dma_alignment=511 loop device iflag=direct reads succeed while a 16-block oflag=direct copy performs 3 fcntl calls in total instead of 2 per block. The branch is also rebased onto current main (058a2a6).

[relative23]

Hardened the buffer internals a bit further ahead of review: AlignedBuffer now stores the Layout it validated at construction, so the deallocation invariant is simply "freed with exactly the stored layout" (and the unwrap is gone from the Drop path). The alignment contract is now enforced for zero-length buffers as well — previously try_new(0, 3) succeeded in release builds — and empty buffers get an aligned dangling pointer, so the alignment promise holds for them too. New edge-case tests cover rejected alignments and an unrepresentable size; the module's six unit tests still pass under Miri (standalone, as noted above).

One correction to my earlier phrasing for precision: the unsafe surface is four unsafe expressions in three categories — allocation, deallocation, and the two Deref slice constructions. No behavioral change to the copy loop; the DHAT peak is unchanged.

[sylvestre]

i stopped reviewing it because i became tired ;)
I do appreciate the improvements that you are making but you need to use your AI better

A few comments:

• it is clearly written by an AI (i don't mind) but I mind that you aren't using it to make my life easier and these PR are impossible to review
• all your comments in this PR are too long
• This PR is WAY too big
• Too many unsafe
• please leverage other crates that we already use
• Don't follow our practices/recommendations
• If you improved the performances, write divan's benchmark to demonstrate this, i don't care about comments as they don't scale

I will stop there too but maybe i should start providing an AGENTS.md and skills to help contributors

[relative23]

Thanks, understood. Before reworking this: should I keep #13373 to the minimal alignment fix and move the probe/strace tests to a follow-up? rustix::mm::mmap is also unsafe; I can use safe memmap2::MmapMut::map_anon (already in the workspace) or the safe over-allocated Vec. Which do you prefer? I will drop the performance prose unless it is backed by divan.

[relative23]

Reworked this around a safe, Vec-backed buffer; the PR now adds no unsafe code. I removed the probe/strace scaffolding, leaving the read-buffer fix and focused regression tests. CodSpeed reports no performance change.