mirror of
https://chromium.googlesource.com/crosvm/crosvm
synced 2025-02-06 02:25:23 +00:00
disk: only fsync composite when needed
This CL avoids fsyncing when unncessary (optimization originally created for Windows). TEST=builds BUG=b:213150316 Change-Id: I9dc7d2d9c394ac83e4220e2a7ae99d670f4a5630 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3654618 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Noah Gold <nkgold@google.com>
This commit is contained in:
Noah Gold
Chromeos LUCI
parent
ce57578ce8
commit
67b8f92b85
1 changed files with 91 additions and 25 deletions
|
|
@ -93,6 +93,7 @@ struct ComponentDiskPart {
|
|||
file: Box<dyn DiskFile>,
|
||||
offset: u64,
|
||||
length: u64,
|
||||
needs_fsync: bool,
|
||||
}
|
||||
|
||||
impl ComponentDiskPart {
|
||||
|
|
@ -198,6 +199,7 @@ impl CompositeDiskFile {
|
|||
.map_err(|e| Error::DiskError(Box::new(e)))?,
|
||||
offset: disk.get_offset(),
|
||||
length: 0, // Assigned later
|
||||
needs_fsync: false,
|
||||
})
|
||||
})
|
||||
.collect::<Result<Vec<ComponentDiskPart>>>()?;
|
||||
|
|
@ -278,7 +280,10 @@ impl FileSetLen for CompositeDiskFile {
|
|||
impl FileSync for CompositeDiskFile {
|
||||
fn fsync(&mut self) -> io::Result<()> {
|
||||
for disk in self.component_disks.iter_mut() {
|
||||
disk.file.fsync()?;
|
||||
if disk.needs_fsync {
|
||||
disk.file.fsync()?;
|
||||
disk.needs_fsync = false;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -319,8 +324,12 @@ impl FileReadWriteAtVolatile for CompositeDiskFile {
|
|||
} else {
|
||||
slice
|
||||
};
|
||||
disk.file
|
||||
.write_at_volatile(subslice, cursor_location - disk.offset)
|
||||
|
||||
let bytes = disk
|
||||
.file
|
||||
.write_at_volatile(subslice, cursor_location - disk.offset)?;
|
||||
disk.needs_fsync = true;
|
||||
Ok(bytes)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -333,11 +342,11 @@ impl PunchHole for CompositeDiskFile {
|
|||
if intersection.start >= intersection.end {
|
||||
continue;
|
||||
}
|
||||
let result = disk.file.punch_hole(
|
||||
disk.file.punch_hole(
|
||||
intersection.start - disk.offset,
|
||||
intersection.end - intersection.start,
|
||||
);
|
||||
result?;
|
||||
)?;
|
||||
disk.needs_fsync = true;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -352,11 +361,11 @@ impl FileAllocate for CompositeDiskFile {
|
|||
if intersection.start >= intersection.end {
|
||||
continue;
|
||||
}
|
||||
let result = disk.file.allocate(
|
||||
disk.file.allocate(
|
||||
intersection.start - disk.offset,
|
||||
intersection.end - intersection.start,
|
||||
);
|
||||
result?;
|
||||
)?;
|
||||
disk.needs_fsync = true;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -372,7 +381,9 @@ impl WriteZeroesAt for CompositeDiskFile {
|
|||
} else {
|
||||
length
|
||||
};
|
||||
disk.file.write_zeroes_at(offset_within_disk, new_length)
|
||||
let bytes = disk.file.write_zeroes_at(offset_within_disk, new_length)?;
|
||||
disk.needs_fsync = true;
|
||||
Ok(bytes)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -664,7 +675,8 @@ mod tests {
|
|||
use std::matches;
|
||||
|
||||
use base::AsRawDescriptor;
|
||||
use data_model::VolatileMemory;
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Write;
|
||||
use tempfile::tempfile;
|
||||
|
||||
#[test]
|
||||
|
|
@ -675,11 +687,13 @@ mod tests {
|
|||
file: Box::new(file1),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part2 = ComponentDiskPart {
|
||||
file: Box::new(file2),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
assert!(CompositeDiskFile::new(vec![disk_part1, disk_part2]).is_err());
|
||||
}
|
||||
|
|
@ -692,11 +706,13 @@ mod tests {
|
|||
file: Box::new(file1),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part2 = ComponentDiskPart {
|
||||
file: Box::new(file2),
|
||||
offset: 100,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let composite = CompositeDiskFile::new(vec![disk_part1, disk_part2]).unwrap();
|
||||
let len = composite.get_len().unwrap();
|
||||
|
|
@ -710,51 +726,55 @@ mod tests {
|
|||
file: Box::new(file),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let mut composite = CompositeDiskFile::new(vec![disk_part]).unwrap();
|
||||
let mut input_memory = [55u8; 5];
|
||||
let input_volatile_memory = VolatileSlice::new(&mut input_memory[..]);
|
||||
composite
|
||||
.write_all_at_volatile(input_volatile_memory.get_slice(0, 5).unwrap(), 0)
|
||||
.write_all_at_volatile(input_volatile_memory, 0)
|
||||
.unwrap();
|
||||
let mut output_memory = [0u8; 5];
|
||||
let output_volatile_memory = VolatileSlice::new(&mut output_memory[..]);
|
||||
composite
|
||||
.read_exact_at_volatile(output_volatile_memory.get_slice(0, 5).unwrap(), 0)
|
||||
.read_exact_at_volatile(output_volatile_memory, 0)
|
||||
.unwrap();
|
||||
assert_eq!(input_memory, output_memory);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn triple_file_fds() {
|
||||
fn triple_file_descriptors() {
|
||||
let file1 = tempfile().unwrap();
|
||||
let file2 = tempfile().unwrap();
|
||||
let file3 = tempfile().unwrap();
|
||||
let mut in_fds = vec![
|
||||
let mut in_descriptors = vec![
|
||||
file1.as_raw_descriptor(),
|
||||
file2.as_raw_descriptor(),
|
||||
file3.as_raw_descriptor(),
|
||||
];
|
||||
in_fds.sort_unstable();
|
||||
in_descriptors.sort_unstable();
|
||||
let disk_part1 = ComponentDiskPart {
|
||||
file: Box::new(file1),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part2 = ComponentDiskPart {
|
||||
file: Box::new(file2),
|
||||
offset: 100,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part3 = ComponentDiskPart {
|
||||
file: Box::new(file3),
|
||||
offset: 200,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let composite = CompositeDiskFile::new(vec![disk_part1, disk_part2, disk_part3]).unwrap();
|
||||
let mut out_fds = composite.as_raw_descriptors();
|
||||
out_fds.sort_unstable();
|
||||
assert_eq!(in_fds, out_fds);
|
||||
let mut out_descriptors = composite.as_raw_descriptors();
|
||||
out_descriptors.sort_unstable();
|
||||
assert_eq!(in_descriptors, out_descriptors);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -766,28 +786,31 @@ mod tests {
|
|||
file: Box::new(file1),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part2 = ComponentDiskPart {
|
||||
file: Box::new(file2),
|
||||
offset: 100,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part3 = ComponentDiskPart {
|
||||
file: Box::new(file3),
|
||||
offset: 200,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let mut composite =
|
||||
CompositeDiskFile::new(vec![disk_part1, disk_part2, disk_part3]).unwrap();
|
||||
let mut input_memory = [55u8; 200];
|
||||
let input_volatile_memory = VolatileSlice::new(&mut input_memory[..]);
|
||||
composite
|
||||
.write_all_at_volatile(input_volatile_memory.get_slice(0, 200).unwrap(), 50)
|
||||
.write_all_at_volatile(input_volatile_memory, 50)
|
||||
.unwrap();
|
||||
let mut output_memory = [0u8; 200];
|
||||
let output_volatile_memory = VolatileSlice::new(&mut output_memory[..]);
|
||||
composite
|
||||
.read_exact_at_volatile(output_volatile_memory.get_slice(0, 200).unwrap(), 50)
|
||||
.read_exact_at_volatile(output_volatile_memory, 50)
|
||||
.unwrap();
|
||||
assert!(input_memory.iter().eq(output_memory.iter()));
|
||||
}
|
||||
|
|
@ -801,29 +824,32 @@ mod tests {
|
|||
file: Box::new(file1),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part2 = ComponentDiskPart {
|
||||
file: Box::new(file2),
|
||||
offset: 100,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part3 = ComponentDiskPart {
|
||||
file: Box::new(file3),
|
||||
offset: 200,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let mut composite =
|
||||
CompositeDiskFile::new(vec![disk_part1, disk_part2, disk_part3]).unwrap();
|
||||
let mut input_memory = [55u8; 300];
|
||||
let input_volatile_memory = VolatileSlice::new(&mut input_memory[..]);
|
||||
composite
|
||||
.write_all_at_volatile(input_volatile_memory.get_slice(0, 300).unwrap(), 0)
|
||||
.write_all_at_volatile(input_volatile_memory, 0)
|
||||
.unwrap();
|
||||
composite.punch_hole(50, 200).unwrap();
|
||||
let mut output_memory = [0u8; 300];
|
||||
let output_volatile_memory = VolatileSlice::new(&mut output_memory[..]);
|
||||
composite
|
||||
.read_exact_at_volatile(output_volatile_memory.get_slice(0, 300).unwrap(), 0)
|
||||
.read_exact_at_volatile(output_volatile_memory, 0)
|
||||
.unwrap();
|
||||
|
||||
input_memory[50..250].iter_mut().for_each(|x| *x = 0);
|
||||
|
|
@ -839,23 +865,26 @@ mod tests {
|
|||
file: Box::new(file1),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part2 = ComponentDiskPart {
|
||||
file: Box::new(file2),
|
||||
offset: 100,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let disk_part3 = ComponentDiskPart {
|
||||
file: Box::new(file3),
|
||||
offset: 200,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let mut composite =
|
||||
CompositeDiskFile::new(vec![disk_part1, disk_part2, disk_part3]).unwrap();
|
||||
let mut input_memory = [55u8; 300];
|
||||
let input_volatile_memory = VolatileSlice::new(&mut input_memory[..]);
|
||||
composite
|
||||
.write_all_at_volatile(input_volatile_memory.get_slice(0, 300).unwrap(), 0)
|
||||
.write_all_at_volatile(input_volatile_memory, 0)
|
||||
.unwrap();
|
||||
let mut zeroes_written = 0;
|
||||
while zeroes_written < 200 {
|
||||
|
|
@ -866,7 +895,7 @@ mod tests {
|
|||
let mut output_memory = [0u8; 300];
|
||||
let output_volatile_memory = VolatileSlice::new(&mut output_memory[..]);
|
||||
composite
|
||||
.read_exact_at_volatile(output_volatile_memory.get_slice(0, 300).unwrap(), 0)
|
||||
.read_exact_at_volatile(output_volatile_memory, 0)
|
||||
.unwrap();
|
||||
|
||||
input_memory[50..250].iter_mut().for_each(|x| *x = 0);
|
||||
|
|
@ -879,6 +908,43 @@ mod tests {
|
|||
assert!(input_memory.iter().eq(output_memory.iter()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fsync_skips_unchanged_parts() {
|
||||
let mut rw_file = tempfile().unwrap();
|
||||
rw_file.write_all(&[0u8; 100]).unwrap();
|
||||
rw_file.seek(SeekFrom::Start(0)).unwrap();
|
||||
let mut ro_disk_image = tempfile::NamedTempFile::new().unwrap();
|
||||
ro_disk_image.write_all(&[0u8; 100]).unwrap();
|
||||
let ro_file = OpenOptions::new()
|
||||
.read(true)
|
||||
.open(ro_disk_image.path())
|
||||
.unwrap();
|
||||
|
||||
let rw_part = ComponentDiskPart {
|
||||
file: Box::new(rw_file),
|
||||
offset: 0,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let ro_part = ComponentDiskPart {
|
||||
file: Box::new(ro_file),
|
||||
offset: 100,
|
||||
length: 100,
|
||||
needs_fsync: false,
|
||||
};
|
||||
let mut composite = CompositeDiskFile::new(vec![rw_part, ro_part]).unwrap();
|
||||
|
||||
// Write to the RW part so that some fsync operation will occur.
|
||||
composite.write_zeroes_at(0, 20).unwrap();
|
||||
|
||||
// This is the test's assert. fsyncing should NOT touch a read-only disk part. On Windows,
|
||||
// this would be an error.
|
||||
composite.fsync().expect(
|
||||
"Failed to fsync composite disk. \
|
||||
This can happen if the disk writable state is wrong.",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn beginning_size() {
|
||||
let mut buffer = vec![];
|
||||
|
|
|
|||
Loading…
Reference in a new issue