dd: stop faulting in the whole copy buffer before reading - #14044
dd: stop faulting in the whole copy buffer before reading#14044luantaraschi wants to merge 1 commit into
Conversation
The copy buffer was reserved and then filled with BUF_INIT_BYTE, so every page of bs= was written before the first read. A bs= far larger than the data being copied therefore cost its full size in resident memory and in the time to write it, even when the input was empty. Read::read does need an initialised slice, but zeroed pages are free: vec![0; n] allocates through alloc_zeroed, so the pages come from the kernel already zero and are never touched until something is read into them. The reservation is kept ahead of it so that an unobtainable bs= still reports an error instead of aborting.
| // try_with_capacity is unstable https://github.com/rust-lang/rust/issues/91913 | ||
| probe.try_reserve(bsize)?; | ||
| drop(probe); | ||
| Ok(vec![0u8; bsize]) |
There was a problem hiding this comment.
Why reallocating at here? It causes OOM race.
There was a problem hiding this comment.
You are right that it allocates twice. The first one is there for the failure path: vec![0; n] goes through handle_alloc_error, so with nothing fallible in front of it dd bs=1PB aborts instead of reporting an error.
I built it both ways to be sure. With the reservation: dd: IO error: out of memory, exit 1. Without it: memory allocation of 1000000000000000 bytes failed, SIGABRT, exit 134. That second one is what test_huge_obs_reports_memory_error_instead_of_aborting was added for, in #12847.
The window you are pointing at is real, though. Between the drop and the second allocation another allocation can take the space, and then this one aborts anyway. I could not find a fallible alloc_zeroed in safe stable Rust, and Vec::try_with_capacity is still open as rust-lang/rust#91913, so the way out I can see is a single alloc_zeroed behind a small unsafe block, which drops the second allocation and the window at once. dd.rs already has one unsafe block, at File::from_raw_fd. Would you rather see it that way?
There was a problem hiding this comment.
This project does not accept unsafe just for performance. So we should improvement for std.
|
I don't think this change avoids filling by 0 and different with previous code. |
|
Please don't link #11544 as closed by this PR which is coming from different fn. |
|
On the zeros: they are still there, that part does not change. What changes is who writes them. The command from #11544, rebuilt today, debug build, peak RSS from On the link, you are right and I have dropped it. The write is the |
ddreserves the copy buffer and then fills it withBUF_INIT_BYTE, so everypage of
bs=is written before the first read, whether or not there is anythingto copy. GNU dd leaves the buffer alone.
Measured against GNU coreutils 9.7 in a container with 15 GiB of RAM, debug build:
bs=1G count=1 iflag=nonblock if=<empty fifo> of=/dev/nullobs=11777777 ibs=1111 bs=7177118117 cbs=8818181111 if=/dev/null of=aRead::readdoes need an initialised slice, but zeroed pages are free.vec![0; n]allocates throughalloc_zeroed, so the pages come from the kernelalready zero and are never touched until something is read into them. The
reservation still runs first, because
vec![0; n]aborts when the allocationfails and
ddhas to report an error instead. The existingbs=1PBtests coverthat path.
The attempts in #11555 and #11577 went after reading into uninitialised memory,
which needs
unsafeor an unstableRead::read_buf. This does not. The bufferstays zeroed, it just never gets written to.
One case is left over. After a short read the buffer is truncated and grown
again on the next iteration, and growing it writes over the new region, so
bs=2Gon a ten byte file still peaks at 2 GiB. Neither reported case reachesit, since both read nothing, but removing the truncate and regrow cycle is a
separate change.
#12143 and #13373 rework the same allocation for O_DIRECT alignment. Both keep a
resize, so the fault-in would survive either of them.Fixes #13869
Refs #11544. The write there is the
resizeinread_helper, and this onlymakes that call a no-op while the buffer still has its full length, so that one
gets better here without being closed.