From 361d19adaad82e1c366e3b16e2bda0f8f7e32881 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Tue, 22 Jun 2021 18:39:40 +0800 Subject: [PATCH 1/3] [space_map (rust)] Fix cache hit with async-io --- src/pdata/space_map_metadata.rs | 2 +- src/write_batcher.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/pdata/space_map_metadata.rs b/src/pdata/space_map_metadata.rs index be55fc3..9002ca0 100644 --- a/src/pdata/space_map_metadata.rs +++ b/src/pdata/space_map_metadata.rs @@ -77,7 +77,7 @@ fn adjust_counts( let nr_free = ie.nr_free - (end - begin) as u32; // Read the bitmap - let bitmap_block = w.engine.read(ie.blocknr)?; + let bitmap_block = w.read(ie.blocknr)?; let (_, mut bitmap) = Bitmap::unpack(bitmap_block.get_data())?; // Update all the entries diff --git a/src/write_batcher.rs b/src/write_batcher.rs index b6e7a34..2300c87 100644 --- a/src/write_batcher.rs +++ b/src/write_batcher.rs @@ -109,6 +109,14 @@ impl WriteBatcher { pub fn write(&mut self, b: Block, kind: checksum::BT) -> Result<()> { checksum::write_checksum(&mut b.get_data(), kind)?; + for blk in self.queue.iter().rev() { + if blk.loc == b.loc { + // write hit + blk.get_data().copy_from_slice(b.get_data()); + return Ok(()); + } + } + if self.queue.len() == self.batch_size { let mut tmp = Vec::new(); std::mem::swap(&mut tmp, &mut self.queue); From cd48f00191cc2a6d168c1255037b598499f73a3a Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Wed, 23 Jun 2021 11:40:12 +0800 Subject: [PATCH 2/3] [all (rust)] Make sync-io the default Multithreaded sync-io has performance similar to async-io. Also, sync-io saves the hassle of setting ulimits to get io_uring working on some systems (commit ba7fd7b). Now we default to sync-io, and leave async-io as a hidden option for testing and benchmarking. --- src/bin/cache_check.rs | 8 +++++++- src/bin/cache_dump.rs | 8 +++++++- src/bin/cache_restore.rs | 13 +++++++------ src/bin/thin_check.rs | 21 ++++++--------------- src/bin/thin_dump.rs | 13 +++++++------ src/bin/thin_restore.rs | 13 +++++++------ 6 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/bin/cache_check.rs b/src/bin/cache_check.rs index 4db7e9b..f266161 100644 --- a/src/bin/cache_check.rs +++ b/src/bin/cache_check.rs @@ -14,6 +14,12 @@ use thinp::report::*; fn main() { let parser = App::new("cache_check") .version(thinp::version::tools_version()) + .arg( + Arg::with_name("ASYNC_IO") + .help("Force use of io_uring for synchronous io") + .long("async-io") + .hidden(true), + ) .arg( Arg::with_name("INPUT") .help("Specify the input device to check") @@ -75,7 +81,7 @@ fn main() { let opts = CacheCheckOptions { dev: &input_file, - async_io: false, + async_io: matches.is_present("ASYNC_IO"), sb_only: matches.is_present("SB_ONLY"), skip_mappings: matches.is_present("SKIP_MAPPINGS"), skip_hints: matches.is_present("SKIP_HINTS"), diff --git a/src/bin/cache_dump.rs b/src/bin/cache_dump.rs index e706f0b..be6ceca 100644 --- a/src/bin/cache_dump.rs +++ b/src/bin/cache_dump.rs @@ -11,6 +11,12 @@ fn main() { let parser = App::new("cache_dump") .version(thinp::version::tools_version()) .about("Dump the cache metadata to stdout in XML format") + .arg( + Arg::with_name("ASYNC_IO") + .help("Force use of io_uring for synchronous io") + .long("async-io") + .hidden(true), + ) .arg( Arg::with_name("REPAIR") .help("Repair the metadata whilst dumping it") @@ -42,7 +48,7 @@ fn main() { let opts = CacheDumpOptions { input: input_file, output: output_file, - async_io: false, + async_io: matches.is_present("ASYNC_IO"), repair: matches.is_present("REPAIR"), }; diff --git a/src/bin/cache_restore.rs b/src/bin/cache_restore.rs index 9375c91..d5eda5e 100644 --- a/src/bin/cache_restore.rs +++ b/src/bin/cache_restore.rs @@ -15,6 +15,12 @@ fn main() { let parser = App::new("cache_restore") .version(thinp::version::tools_version()) .about("Convert XML format metadata to binary.") + .arg( + Arg::with_name("ASYNC_IO") + .help("Force use of io_uring for synchronous io") + .long("async-io") + .hidden(true), + ) .arg( Arg::with_name("OVERRIDE_MAPPING_ROOT") .help("Specify a mapping root to use") @@ -37,11 +43,6 @@ fn main() { .long("output") .value_name("OUTPUT") .required(true), - ) - .arg( - Arg::with_name("SYNC_IO") - .help("Force use of synchronous io") - .long("sync-io"), ); let matches = parser.get_matches(); @@ -66,7 +67,7 @@ fn main() { let opts = CacheRestoreOptions { input: &input_file, output: &output_file, - async_io: !matches.is_present("SYNC_IO"), + async_io: matches.is_present("ASYNC_IO"), report, }; diff --git a/src/bin/thin_check.rs b/src/bin/thin_check.rs index 2298ce9..c3ab573 100644 --- a/src/bin/thin_check.rs +++ b/src/bin/thin_check.rs @@ -16,6 +16,12 @@ fn main() { let parser = App::new("thin_check") .version(thinp::version::tools_version()) .about("Validates thin provisioning metadata on a device or file.") + .arg( + Arg::with_name("ASYNC_IO") + .help("Force use of io_uring for synchronous io") + .long("async-io") + .hidden(true), + ) .arg( Arg::with_name("QUIET") .help("Suppress output messages, return only exit code.") @@ -66,16 +72,6 @@ fn main() { .help("Specify the input device to check") .required(true) .index(1), - ) - .arg( - Arg::with_name("ASYNC_IO") - .help("Force use of io_uring for asynchronous IO") - .long("async-io"), - ) - .arg( - Arg::with_name("SYNC_IO") - .help("Force use of synchronous IO (currently the default)") - .long("sync-io"), ); let matches = parser.get_matches(); @@ -96,11 +92,6 @@ fn main() { report = Arc::new(mk_simple_report()); } - if matches.is_present("SYNC_IO") && matches.is_present("ASYNC_IO") { - eprintln!("--sync-io and --async-io may not be used at the same time."); - process::exit(1); - } - let engine: Arc; if matches.is_present("ASYNC_IO") { diff --git a/src/bin/thin_dump.rs b/src/bin/thin_dump.rs index b9f08d0..af238dd 100644 --- a/src/bin/thin_dump.rs +++ b/src/bin/thin_dump.rs @@ -15,6 +15,12 @@ fn main() { let parser = App::new("thin_dump") .version(thinp::version::tools_version()) .about("Dump thin-provisioning metadata to stdout in XML format") + .arg( + Arg::with_name("ASYNC_IO") + .help("Force use of io_uring for synchronous io") + .long("async-io") + .hidden(true), + ) .arg( Arg::with_name("QUIET") .help("Suppress output messages, return only exit code.") @@ -32,11 +38,6 @@ fn main() { .help("Do not dump the mappings") .long("skip-mappings"), ) - .arg( - Arg::with_name("SYNC_IO") - .help("Force use of synchronous io") - .long("sync-io"), - ) .arg( Arg::with_name("METADATA_SNAPSHOT") .help("Access the metadata snapshot on a live pool") @@ -84,7 +85,7 @@ fn main() { let opts = ThinDumpOptions { input: input_file, output: output_file, - async_io: !matches.is_present("SYNC_IO"), + async_io: matches.is_present("ASYNC_IO"), report, }; diff --git a/src/bin/thin_restore.rs b/src/bin/thin_restore.rs index 243f3a4..8654613 100644 --- a/src/bin/thin_restore.rs +++ b/src/bin/thin_restore.rs @@ -15,6 +15,12 @@ fn main() { let parser = App::new("thin_restore") .version(thinp::version::tools_version()) .about("Convert XML format metadata to binary.") + .arg( + Arg::with_name("ASYNC_IO") + .help("Force use of io_uring for synchronous io") + .long("async-io") + .hidden(true), + ) .arg( Arg::with_name("OVERRIDE_MAPPING_ROOT") .help("Specify a mapping root to use") @@ -37,11 +43,6 @@ fn main() { .long("output") .value_name("OUTPUT") .required(true), - ) - .arg( - Arg::with_name("SYNC_IO") - .help("Force use of synchronous io") - .long("sync-io"), ); let matches = parser.get_matches(); @@ -66,7 +67,7 @@ fn main() { let opts = ThinRestoreOptions { input: &input_file, output: &output_file, - async_io: !matches.is_present("SYNC_IO"), + async_io: matches.is_present("ASYNC_IO"), report, }; From 2cb84236d4ba916dd68a23286b582b733c3c79d4 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Wed, 23 Jun 2021 12:44:38 +0800 Subject: [PATCH 3/3] [all (rust)] Tidy command line options --- src/bin/cache_check.rs | 64 +++++++++++++++++++--------------------- src/bin/cache_dump.rs | 3 ++ src/bin/cache_restore.rs | 2 ++ src/bin/thin_check.rs | 43 ++++++++++++++------------- src/bin/thin_dump.rs | 3 ++ src/bin/thin_restore.rs | 16 +++++----- 6 files changed, 71 insertions(+), 60 deletions(-) diff --git a/src/bin/cache_check.rs b/src/bin/cache_check.rs index f266161..34fe874 100644 --- a/src/bin/cache_check.rs +++ b/src/bin/cache_check.rs @@ -14,6 +14,7 @@ use thinp::report::*; fn main() { let parser = App::new("cache_check") .version(thinp::version::tools_version()) + // flags .arg( Arg::with_name("ASYNC_IO") .help("Force use of io_uring for synchronous io") @@ -21,50 +22,47 @@ fn main() { .hidden(true), ) .arg( - Arg::with_name("INPUT") - .help("Specify the input device to check") - .required(true) - .index(1), - ) - .arg( - Arg::with_name("SB_ONLY") - .help("Only check the superblock.") - .long("super-block-only") - .value_name("SB_ONLY"), - ) - .arg( - Arg::with_name("SKIP_MAPPINGS") - .help("Don't check the mapping array") - .long("skip-mappings") - .value_name("SKIP_MAPPINGS"), - ) - .arg( - Arg::with_name("SKIP_HINTS") - .help("Don't check the hint array") - .long("skip-hints") - .value_name("SKIP_HINTS"), - ) - .arg( - Arg::with_name("SKIP_DISCARDS") - .help("Don't check the discard bitset") - .long("skip-discards") - .value_name("SKIP_DISCARDS"), + Arg::with_name("AUTO_REPAIR") + .help("Auto repair trivial issues.") + .long("auto-repair"), ) .arg( Arg::with_name("IGNORE_NON_FATAL") .help("Only return a non-zero exit code if a fatal error is found.") .long("ignore-non-fatal-errors"), ) - .arg( - Arg::with_name("AUTO_REPAIR") - .help("Auto repair trivial issues.") - .long("auto-repair"), - ) .arg( Arg::with_name("QUIET") .help("Suppress output messages, return only exit code.") .short("q") .long("quiet"), + ) + .arg( + Arg::with_name("SB_ONLY") + .help("Only check the superblock.") + .long("super-block-only"), + ) + .arg( + Arg::with_name("SKIP_MAPPINGS") + .help("Don't check the mapping array") + .long("skip-mappings"), + ) + .arg( + Arg::with_name("SKIP_HINTS") + .help("Don't check the hint array") + .long("skip-hints"), + ) + .arg( + Arg::with_name("SKIP_DISCARDS") + .help("Don't check the discard bitset") + .long("skip-discards"), + ) + // arguments + .arg( + Arg::with_name("INPUT") + .help("Specify the input device to check") + .required(true) + .index(1), ); let matches = parser.get_matches(); diff --git a/src/bin/cache_dump.rs b/src/bin/cache_dump.rs index be6ceca..93644ec 100644 --- a/src/bin/cache_dump.rs +++ b/src/bin/cache_dump.rs @@ -11,6 +11,7 @@ fn main() { let parser = App::new("cache_dump") .version(thinp::version::tools_version()) .about("Dump the cache metadata to stdout in XML format") + // flags .arg( Arg::with_name("ASYNC_IO") .help("Force use of io_uring for synchronous io") @@ -23,6 +24,7 @@ fn main() { .short("r") .long("repair"), ) + // options .arg( Arg::with_name("OUTPUT") .help("Specify the output file rather than stdout") @@ -30,6 +32,7 @@ fn main() { .long("output") .value_name("OUTPUT"), ) + // arguments .arg( Arg::with_name("INPUT") .help("Specify the input device to dump") diff --git a/src/bin/cache_restore.rs b/src/bin/cache_restore.rs index d5eda5e..e1f4209 100644 --- a/src/bin/cache_restore.rs +++ b/src/bin/cache_restore.rs @@ -15,6 +15,7 @@ fn main() { let parser = App::new("cache_restore") .version(thinp::version::tools_version()) .about("Convert XML format metadata to binary.") + // flags .arg( Arg::with_name("ASYNC_IO") .help("Force use of io_uring for synchronous io") @@ -28,6 +29,7 @@ fn main() { .value_name("OVERRIDE_MAPPING_ROOT") .takes_value(true), ) + // options .arg( Arg::with_name("INPUT") .help("Specify the input xml") diff --git a/src/bin/thin_check.rs b/src/bin/thin_check.rs index c3ab573..7887be3 100644 --- a/src/bin/thin_check.rs +++ b/src/bin/thin_check.rs @@ -16,12 +16,28 @@ fn main() { let parser = App::new("thin_check") .version(thinp::version::tools_version()) .about("Validates thin provisioning metadata on a device or file.") + // flags .arg( Arg::with_name("ASYNC_IO") .help("Force use of io_uring for synchronous io") .long("async-io") .hidden(true), ) + .arg( + Arg::with_name("AUTO_REPAIR") + .help("Auto repair trivial issues.") + .long("auto-repair"), + ) + .arg( + Arg::with_name("CLEAR_NEEDS_CHECK") + .help("Clears the 'needs_check' flag in the superblock") + .long("clear-needs-check-flag"), + ) + .arg( + Arg::with_name("IGNORE_NON_FATAL") + .help("Only return a non-zero exit code if a fatal error is found.") + .long("ignore-non-fatal-errors"), + ) .arg( Arg::with_name("QUIET") .help("Suppress output messages, return only exit code.") @@ -38,20 +54,13 @@ fn main() { .help("Don't check the mapping tree") .long("skip-mappings"), ) + // options .arg( - Arg::with_name("AUTO_REPAIR") - .help("Auto repair trivial issues.") - .long("auto-repair"), - ) - .arg( - Arg::with_name("IGNORE_NON_FATAL") - .help("Only return a non-zero exit code if a fatal error is found.") - .long("ignore-non-fatal-errors"), - ) - .arg( - Arg::with_name("CLEAR_NEEDS_CHECK") - .help("Clears the 'needs_check' flag in the superblock") - .long("clear-needs-check-flag"), + Arg::with_name("METADATA_SNAPSHOT") + .help("Check the metadata snapshot on a live pool") + .short("m") + .long("metadata-snapshot") + .value_name("METADATA_SNAPSHOT"), ) .arg( Arg::with_name("OVERRIDE_MAPPING_ROOT") @@ -60,13 +69,7 @@ fn main() { .value_name("OVERRIDE_MAPPING_ROOT") .takes_value(true), ) - .arg( - Arg::with_name("METADATA_SNAPSHOT") - .help("Check the metadata snapshot on a live pool") - .short("m") - .long("metadata-snapshot") - .value_name("METADATA_SNAPSHOT"), - ) + // arguments .arg( Arg::with_name("INPUT") .help("Specify the input device to check") diff --git a/src/bin/thin_dump.rs b/src/bin/thin_dump.rs index af238dd..606ea42 100644 --- a/src/bin/thin_dump.rs +++ b/src/bin/thin_dump.rs @@ -15,6 +15,7 @@ fn main() { let parser = App::new("thin_dump") .version(thinp::version::tools_version()) .about("Dump thin-provisioning metadata to stdout in XML format") + // flags .arg( Arg::with_name("ASYNC_IO") .help("Force use of io_uring for synchronous io") @@ -38,6 +39,7 @@ fn main() { .help("Do not dump the mappings") .long("skip-mappings"), ) + // options .arg( Arg::with_name("METADATA_SNAPSHOT") .help("Access the metadata snapshot on a live pool") @@ -52,6 +54,7 @@ fn main() { .long("output") .value_name("OUTPUT"), ) + // arguments .arg( Arg::with_name("INPUT") .help("Specify the input device to dump") diff --git a/src/bin/thin_restore.rs b/src/bin/thin_restore.rs index 8654613..093f376 100644 --- a/src/bin/thin_restore.rs +++ b/src/bin/thin_restore.rs @@ -15,19 +15,14 @@ fn main() { let parser = App::new("thin_restore") .version(thinp::version::tools_version()) .about("Convert XML format metadata to binary.") + // flags .arg( Arg::with_name("ASYNC_IO") .help("Force use of io_uring for synchronous io") .long("async-io") .hidden(true), ) - .arg( - Arg::with_name("OVERRIDE_MAPPING_ROOT") - .help("Specify a mapping root to use") - .long("override-mapping-root") - .value_name("OVERRIDE_MAPPING_ROOT") - .takes_value(true), - ) + // options .arg( Arg::with_name("INPUT") .help("Specify the input xml") @@ -43,6 +38,13 @@ fn main() { .long("output") .value_name("OUTPUT") .required(true), + ) + .arg( + Arg::with_name("OVERRIDE_MAPPING_ROOT") + .help("Specify a mapping root to use") + .long("override-mapping-root") + .value_name("OVERRIDE_MAPPING_ROOT") + .takes_value(true), ); let matches = parser.get_matches();