From 46f70e8c1679e0758683112db45fec590beb5b5b Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 27 Sep 2018 09:34:59 -0700 Subject: [PATCH] qcow: optimize sync_caches to avoid extra writes Track the clean/dirty state of the L1 table and the refcount table to avoid writing them out and doing an extra fsyncdata() if nothing has changed. BUG=None TEST=Manually verify strace output contains only the expected fsyncs Change-Id: I20bdd250024039a5b4142605462a8977ced1efcc Signed-off-by: Daniel Verkamp Reviewed-on: https://chromium-review.googlesource.com/1247442 Reviewed-by: Dylan Reid --- qcow/src/qcow.rs | 31 +++++++++++++++++++------------ qcow/src/refcount.rs | 26 ++++++++++++++++++++------ qcow/src/vec_cache.rs | 8 ++++++++ 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/qcow/src/qcow.rs b/qcow/src/qcow.rs index 312920365a..c44cc4add8 100644 --- a/qcow/src/qcow.rs +++ b/qcow/src/qcow.rs @@ -257,7 +257,7 @@ fn max_refcount_clusters(refcount_order: u32, cluster_size: u32, num_clusters: u pub struct QcowFile { raw_file: QcowRawFile, header: QcowHeader, - l1_table: Vec, + l1_table: VecCache, l2_entries: u64, l2_cache: CacheMap>, refcounts: RefCount, @@ -316,13 +316,13 @@ impl QcowFile { let l2_size = cluster_size / size_of::() as u64; let num_clusters = div_round_up_u64(header.size, u64::from(cluster_size)); let num_l2_clusters = div_round_up_u64(num_clusters, l2_size); - let l1_table = raw_file + let l1_table = VecCache::from_vec(raw_file .read_pointer_table( header.l1_table_offset, num_l2_clusters, Some(L1_TABLE_OFFSET_MASK), ) - .map_err(Error::ReadingHeader)?; + .map_err(Error::ReadingHeader)?); let num_clusters = div_round_up_u64(header.size, u64::from(cluster_size)) as u32; let refcount_clusters = @@ -396,7 +396,7 @@ impl QcowFile { /// Returns the L1 lookup table for this file. This is only useful for debugging. pub fn l1_table(&self) -> &[u64] { - &self.l1_table + &self.l1_table.get_values() } /// Returns an L2_table of cluster addresses, only used for debugging. @@ -414,7 +414,7 @@ impl QcowFile { Self::read_l2_cluster(&mut self.raw_file, l2_addr_disk) .map_err(Error::ReadingPointers)?, ); - let l1_table = &mut self.l1_table; + let l1_table = &self.l1_table; let raw_file = &mut self.raw_file; self.l2_cache .insert(l1_index, table, |index, evicted| { @@ -529,7 +529,7 @@ impl QcowFile { let table = VecCache::from_vec(Self::read_l2_cluster(&mut self.raw_file, l2_addr_disk)?); - let l1_table = &mut self.l1_table; + let l1_table = &self.l1_table; let raw_file = &mut self.raw_file; self.l2_cache.insert(l1_index, table, |index, evicted| { raw_file.write_pointer_table( @@ -577,7 +577,7 @@ impl QcowFile { } else { VecCache::from_vec(Self::read_l2_cluster(&mut self.raw_file, l2_addr_disk)?) }; - let l1_table = &mut self.l1_table; + let l1_table = &self.l1_table; let raw_file = &mut self.raw_file; self.l2_cache.insert(l1_index, l2_table, |index, evicted| { raw_file.write_pointer_table( @@ -690,7 +690,7 @@ impl QcowFile { // Not in the cache. let table = VecCache::from_vec(Self::read_l2_cluster(&mut self.raw_file, l2_addr_disk)?); - let l1_table = &mut self.l1_table; + let l1_table = &self.l1_table; let raw_file = &mut self.raw_file; self.l2_cache.insert(l1_index, table, |index, evicted| { raw_file.write_pointer_table( @@ -826,10 +826,17 @@ impl QcowFile { // Push L1 table and refcount table last as all the clusters they point to are now // guaranteed to be valid. - self.raw_file - .write_pointer_table(self.header.l1_table_offset, &self.l1_table, 0)?; - self.refcounts.flush_table(&mut self.raw_file)?; - self.raw_file.file_mut().sync_data()?; + let mut sync_required = false; + if self.l1_table.dirty() { + self.raw_file + .write_pointer_table(self.header.l1_table_offset, &self.l1_table.get_values(), 0)?; + self.l1_table.mark_clean(); + sync_required = true; + } + sync_required |= self.refcounts.flush_table(&mut self.raw_file)?; + if sync_required { + self.raw_file.file_mut().sync_data()?; + } Ok(()) } } diff --git a/qcow/src/refcount.rs b/qcow/src/refcount.rs index 96b5e0cae3..13906a19f6 100644 --- a/qcow/src/refcount.rs +++ b/qcow/src/refcount.rs @@ -29,7 +29,7 @@ pub type Result = std::result::Result; /// Represents the refcount entries for an open qcow file. #[derive(Debug)] pub struct RefCount { - ref_table: Vec, + ref_table: VecCache, refcount_table_offset: u64, refblock_cache: CacheMap>, refcount_block_entries: u64, // number of refcounts in a cluster. @@ -48,8 +48,11 @@ impl RefCount { refcount_block_entries: u64, cluster_size: u64, ) -> io::Result { - let ref_table = - raw_file.read_pointer_table(refcount_table_offset, refcount_table_entries, None)?; + let ref_table = VecCache::from_vec(raw_file.read_pointer_table( + refcount_table_offset, + refcount_table_entries, + None, + )?); Ok(RefCount { ref_table, refcount_table_offset, @@ -134,8 +137,19 @@ impl RefCount { } /// Flush the refcount table that keeps the address of the refcounts blocks. - pub fn flush_table(&mut self, raw_file: &mut QcowRawFile) -> io::Result<()> { - raw_file.write_pointer_table(self.refcount_table_offset, &self.ref_table, 0) + /// Returns true if the table changed since the previous `flush_table()` call. + pub fn flush_table(&mut self, raw_file: &mut QcowRawFile) -> io::Result { + if self.ref_table.dirty() { + raw_file.write_pointer_table( + self.refcount_table_offset, + &self.ref_table.get_values(), + 0, + )?; + self.ref_table.mark_clean(); + Ok(true) + } else { + Ok(false) + } } /// Gets the refcount for a cluster with the given address. @@ -167,7 +181,7 @@ impl RefCount { /// Returns the refcount table for this file. This is only useful for debugging. pub fn ref_table(&self) -> &[u64] { - &self.ref_table + &self.ref_table.get_values() } /// Returns the refcounts stored in the given block. diff --git a/qcow/src/vec_cache.rs b/qcow/src/vec_cache.rs index 4144eb4f6e..667f45b6d5 100644 --- a/qcow/src/vec_cache.rs +++ b/qcow/src/vec_cache.rs @@ -6,6 +6,7 @@ use std::collections::hash_map::IterMut; use std::collections::HashMap; use std::io; use std::ops::{Index, IndexMut}; +use std::slice::SliceIndex; /// Trait that allows for checking if an implementor is dirty. Useful for types that are cached so /// it can be checked if they need to be committed to disk. @@ -38,6 +39,13 @@ impl VecCache { } } + pub fn get(&self, index: I) -> Option<&>::Output> + where + I: SliceIndex<[T]>, + { + self.vec.get(index) + } + /// Gets a reference to the underlying vector. pub fn get_values(&self) -> &[T] { &self.vec