Skip to content

Commit

Permalink
unset O_DIRECT flag on last chunk if irregular sized
Browse files Browse the repository at this point in the history
  • Loading branch information
cre4ture committed Mar 17, 2024
1 parent e450ce8 commit f6266b8
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 9 deletions.
4 changes: 1 addition & 3 deletions src/uu/dd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ gcd = { workspace = true }
libc = { workspace = true }
uucore = { workspace = true, features = ["format", "quoting-style"] }

[target.'cfg(any(target_os = "linux"))'.dependencies]
nix = { workspace = true, features = ["fs"] }

[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
nix = { workspace = true, features = ["fs"] }
signal-hook = { workspace = true }

[[bin]]
Expand Down
43 changes: 37 additions & 6 deletions src/uu/dd/src/dd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// file that was distributed with this source code.

// spell-checker:ignore fname, ftype, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, behaviour, bmax, bremain, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, iseek, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, oseek, outfile, parseargs, rlen, rmax, rremain, rsofar, rstat, sigusr, wlen, wstat seekable oconv canonicalized fadvise Fadvise FADV DONTNEED ESPIPE bufferedoutput
// spell-checker:ignore GETFL SETFL

mod blocks;
mod bufferedoutput;
Expand Down Expand Up @@ -41,6 +42,8 @@ use std::time::{Duration, Instant};

use clap::{crate_version, Arg, Command};
use gcd::Gcd;
#[cfg(any(target_os = "android", target_os = "linux"))]
use nix::fcntl::{fcntl, FcntlArg, OFlag};
#[cfg(target_os = "linux")]
use nix::{
errno::Errno,
Expand Down Expand Up @@ -509,6 +512,20 @@ enum Dest {
}

impl Dest {
fn unset_direct(&mut self) -> io::Result<()> {
match self {
#[cfg(any(target_os = "linux", target_os = "android"))]
Self::File(f, _d) => {
let mut mode = OFlag::from_bits_retain(fcntl(f.as_raw_fd(), FcntlArg::F_GETFL)?);
mode.remove(OFlag::O_DIRECT);
nix::fcntl::fcntl(f.as_raw_fd(), FcntlArg::F_SETFL(mode))?;
}
_ => {}
}

Ok(())
}

fn fsync(&mut self) -> io::Result<()> {
match self {
Self::Stdout(stdout) => stdout.flush(),
Expand Down Expand Up @@ -774,7 +791,14 @@ impl<'a> Output<'a> {
let mut writes_partial = 0;
let mut bytes_total = 0;

for chunk in buf.chunks(self.settings.obs) {
let chunk_size = self.settings.obs;
for chunk in buf.chunks(chunk_size) {
if (self.settings.oflags.direct) && (chunk.len() < chunk_size) {
// in case of direct io, only buffers with chunk_size are accepted.
// thus, for writing a (last) buffer with irregular length, we need to switch off the direct io.
self.dst.unset_direct()?;
}

let wlen = self.dst.write(chunk)?;
if wlen < self.settings.obs {
writes_partial += 1;
Expand Down Expand Up @@ -877,7 +901,7 @@ impl<'a> BlockWriter<'a> {
///
/// If there is a problem reading from the input or writing to
/// this output.
fn dd_copy(mut i: Input, o: Output) -> std::io::Result<()> {
fn dd_copy(mut i: Input, o: Output) -> UResult<()> {
// The read and write statistics.
//
// These objects are counters, initialized to zero. After each
Expand Down Expand Up @@ -992,11 +1016,18 @@ fn dd_copy(mut i: Input, o: Output) -> std::io::Result<()> {
// best buffer size for reading based on the number of
// blocks already read and the number of blocks remaining.
let loop_bsize = calc_loop_bsize(&i.settings.count, &rstat, &wstat, i.settings.ibs, bsize);
let rstat_update = read_helper(&mut i, &mut buf, loop_bsize)?;
let rstat_update = read_helper(&mut i, &mut buf, loop_bsize)
.map_err_context(|| format!("reading, ls: {loop_bsize}, rbt: {}", rstat.bytes_total))?;
if rstat_update.is_empty() {
break;
}
let wstat_update = o.write_blocks(&buf)?;
let wstat_update = o.write_blocks(&buf).map_err_context(|| {
format!(

Check warning on line 1025 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L1025

Added line #L1025 was not covered by tests
"writing, ls: {}/{loop_bsize}, wbt: {}",
buf.len(),

Check warning on line 1027 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L1027

Added line #L1027 was not covered by tests
wstat.bytes_total
)
})?;

Check warning on line 1030 in src/uu/dd/src/dd.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/dd/src/dd.rs#L1030

Added line #L1030 was not covered by tests

// Discard the system file cache for the read portion of
// the input file.
Expand Down Expand Up @@ -1047,7 +1078,7 @@ fn finalize<T>(
prog_tx: &mpsc::Sender<ProgUpdate>,
output_thread: thread::JoinHandle<T>,
truncate: bool,
) -> std::io::Result<()> {
) -> UResult<()> {
// Flush the output in case a partial write has been buffered but
// not yet written.
let wstat_update = output.flush()?;
Expand Down Expand Up @@ -1292,7 +1323,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
}
None => Output::new_stdout(&settings)?,
};
dd_copy(i, o).map_err_context(|| "IO error".to_string())
dd_copy(i, o)
}

pub fn uu_app() -> Command {
Expand Down
42 changes: 42 additions & 0 deletions tests/by-util/test_dd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1713,3 +1713,45 @@ fn test_reading_partial_blocks_from_fifo_unbuffered() {
let expected = b"0+2 records in\n0+2 records out\n4 bytes copied";
assert!(output.stderr.starts_with(expected));
}

#[cfg(any(target_os = "linux", target_os = "android"))]
#[test]
fn test_reading_and_writing_with_direct_flag_from_and_to_files_with_irregular_size() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;

let min_direct_block_size = if cfg!(target_os = "android") {
4096
} else {
512
};

let p = |m: i32, o: i32| (min_direct_block_size * m + o).to_string();

ts.ccmd("truncate")
.args(&["-s", p(1, -1).as_str(), "short"])
.succeeds();
ts.ccmd("truncate")
.args(&["-s", p(16, 0).as_str(), "in"])
.succeeds();
ts.ccmd("truncate")
.args(&["-s", p(16, -1).as_str(), "m1"])
.succeeds();
ts.ccmd("truncate")
.args(&["-s", p(16, 1).as_str(), "p1"])
.succeeds();

ts.ucmd()
.arg(format!("bs={}", min_direct_block_size))
.args(&["if=in", "oflag=direct", "of=out"])
.succeeds();

for testfile in ["short", "m1", "p1"] {
at.remove("out");
ts.ucmd()
.arg(format!("if={testfile}"))
.arg(format!("bs={}", min_direct_block_size))
.args(&["iflag=direct", "oflag=direct", "of=out"])
.succeeds();
}
}

0 comments on commit f6266b8

Please sign in to comment.