swap: fix for VMs with highmem

Handle the case where VM memory is split up into multiple regions by
taking into account the region end when swapping out memory. Also rename
SwapFile.len to SwapFile.num_pages to make the units clear.

BUG=b:215093219
TEST=cargo test --package base
TEST=enable/disable swap on VM with >4GB of memory

Change-Id: I8dcdadd39c3f7cbd22c1c042498153966a1a0431
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4040653
Commit-Queue: David Stevens <stevensd@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Shin Kawamura <kawasin@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
This commit is contained in:
David Stevens 2022-11-21 14:49:19 +09:00 committed by crosvm LUCI
parent 1fe0c44816
commit 34d7f69f27
3 changed files with 41 additions and 12 deletions

View file

@ -35,6 +35,7 @@ fn lseek(fd: &dyn AsRawDescriptor, offset: u64, option: LseekOption) -> Result<u
pub struct FileDataIterator<'a> {
fd: &'a dyn AsRawDescriptor,
offset: u64,
end: u64,
}
impl<'a> FileDataIterator<'a> {
@ -44,13 +45,24 @@ impl<'a> FileDataIterator<'a> {
///
/// * `fd` - the [trait@AsRawDescriptor] of the file
/// * `offset` - the offset to start traversing from.
pub fn new(fd: &'a dyn AsRawDescriptor, offset: u64) -> Self {
Self { fd, offset }
/// * `len` - the len of the region over which to iterate
pub fn new(fd: &'a dyn AsRawDescriptor, offset: u64, len: u64) -> Self {
Self {
fd,
offset,
end: offset + len,
}
}
fn find_next_data(&self) -> Result<Option<Range<u64>>> {
let offset_data = match lseek(self.fd, self.offset, LseekOption::Data) {
Ok(offset) => offset,
Ok(offset) => {
if offset >= self.end {
return Ok(None);
} else {
offset
}
}
Err(e) => {
return match e.errno() {
libc::ENXIO => Ok(None),
@ -60,7 +72,7 @@ impl<'a> FileDataIterator<'a> {
};
let offset_hole = lseek(self.fd, offset_data, LseekOption::Hole)?;
Ok(Some(offset_data..offset_hole))
Ok(Some(offset_data..offset_hole.min(self.end)))
}
}
@ -99,11 +111,27 @@ mod tests {
file.write_at(&[1_u8], 2 * pagesize() as u64).unwrap();
file.write_at(&[1_u8], (4 * pagesize() - 1) as u64).unwrap();
let iter = FileDataIterator::new(&file, 0);
let iter = FileDataIterator::new(&file, 0, 4 * pagesize() as u64);
let result: Vec<Range<u64>> = iter.collect();
assert_eq!(result.len(), 2);
assert_eq!(result[0], 0..(pagesize() as u64));
assert_eq!(result[1], (2 * pagesize() as u64)..(4 * pagesize() as u64));
}
#[test]
fn file_data_iterator_subrange() {
let file = tempfile::tempfile().unwrap();
file.write_at(&[1_u8], 0).unwrap();
file.write_at(&[1_u8], pagesize() as u64).unwrap();
file.write_at(&[1_u8], 2 * pagesize() as u64).unwrap();
file.write_at(&[1_u8], 4 * pagesize() as u64).unwrap();
let iter = FileDataIterator::new(&file, pagesize() as u64, pagesize() as u64);
let result: Vec<Range<u64>> = iter.collect();
assert_eq!(result.len(), 1);
assert_eq!(result[0], (pagesize() as u64)..(2 * pagesize() as u64));
}
}

View file

@ -104,7 +104,7 @@ impl SwapFile {
}
/// Returns the total count of managed pages.
pub fn len(&self) -> usize {
pub fn num_pages(&self) -> usize {
self.state_list.len()
}
@ -255,7 +255,7 @@ mod tests {
let dir_path = tempfile::tempdir().unwrap();
let swap_file = SwapFile::new(dir_path.path(), 200).unwrap();
assert_eq!(swap_file.len(), 200);
assert_eq!(swap_file.num_pages(), 200);
}
#[test]

View file

@ -121,7 +121,8 @@ impl PageHandler {
// sequential search the corresponding page map from the list. It should be fast enough
// because there are a few regions (usually only 1).
self.regions.iter().position(|region| {
region.head_page_idx <= page_idx && page_idx < region.head_page_idx + region.file.len()
region.head_page_idx <= page_idx
&& page_idx < region.head_page_idx + region.file.num_pages()
})
}
@ -147,7 +148,7 @@ impl PageHandler {
// find an overlaping region
match self.regions.iter().position(|region| {
if region.head_page_idx < head_page_idx {
region.head_page_idx + region.file.len() > head_page_idx
region.head_page_idx + region.file.num_pages() > head_page_idx
} else {
region.head_page_idx < head_page_idx + num_of_pages
}
@ -158,7 +159,7 @@ impl PageHandler {
Err(Error::RegionOverlap(
address_range.clone(),
self.page_idx_to_addr(region.head_page_idx)
..(self.page_idx_to_addr(region.head_page_idx + region.file.len())),
..(self.page_idx_to_addr(region.head_page_idx + region.file.num_pages())),
))
}
None => {
@ -312,8 +313,8 @@ impl PageHandler {
if self.regions[region_position].head_page_idx != head_page_idx {
return Err(Error::InvalidAddress(base_addr));
}
let region_size = self.regions[region_position].file.len() << self.pagesize_shift;
let file_data = FileDataIterator::new(memfd, base_offset);
let region_size = self.regions[region_position].file.num_pages() << self.pagesize_shift;
let file_data = FileDataIterator::new(memfd, base_offset, region_size as u64);
for data_range in file_data {
// assert offset is page aligned