Skip to content

Commit 2a36076

Browse files
Aaron DrewRebase bot
authored andcommitted
[fxfs] Fix bug where pages may not be supplied.
It's possible that VMO size and 'byte_size' are different. This fixes a bug where, if byte_size is smaller, the page handler may fail to supply required pages. It also ensures that read ahead will not supply zero pages beyond the end of the VMO (uses max of byte_size and requested range) as this will cause supply_pages to fail a range check and lead to a similar result. Bug: b/318773519 Change-Id: I87dd37a7065a6e454668b7e72ad1f7a98992b1e4 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/971633 Commit-Queue: Aaron Drew <ripper@google.com> Reviewed-by: Chris Suter <csuter@google.com>
1 parent cfdb983 commit 2a36076

File tree

1 file changed

+26
-10
lines changed
  • src/storage/fxfs/platform/src/fuchsia

1 file changed

+26
-10
lines changed

src/storage/fxfs/platform/src/fuchsia/pager.rs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ pub trait PagerBacked: Sync + Send + Sized + 'static {
402402
}
403403

404404
/// A generic page_in implementation that supplies pages using block-aligned reads.
405-
pub fn default_page_in<P: PagerBacked>(this: Arc<P>, mut range: Range<u64>) {
405+
pub fn default_page_in<P: PagerBacked>(this: Arc<P>, range: Range<u64>) {
406406
fxfs_trace::duration!(
407407
"start-page-in",
408408
"offset" => range.start,
@@ -425,27 +425,43 @@ pub fn default_page_in<P: PagerBacked>(this: Arc<P>, mut range: Range<u64>) {
425425
} else {
426426
round_down(READ_AHEAD_SIZE, read_alignment)
427427
};
428+
429+
// Two important subtleties to consider in this space:
430+
//
431+
// `byte_size` is the official size of the object. VMOs are page-aligned so `aligned_size` is
432+
// the "offical" page length of the object. This may be smaller than Vmo::get_size because
433+
// these two things are not updated atomically. The reverse is not true -- We do not currently
434+
// ever shrink a VMO's size. We also do not update byte_size (self.handle.get_size()) if
435+
// an independent handle is used to grow a file. This means the VMO's size should always be
436+
// strictly equal or bigger than `byte_size`.
437+
//
438+
// It is valid to supply more pages than asked, but supplying pages outside of the VMO range
439+
// will trigger OUT_OF_RANGE errors and the call will fail without supplying anything.
440+
// We must supply the range requested under all circumstances to unblock any page misses but
441+
// we should take care to never supply additional pages beyond the object length (`byte_size`)
442+
// as there is a chance that we might serve a range outside of the VMO and fail to supply
443+
// anything at all.
444+
428445
let aligned_size = round_up(this.byte_size(), read_alignment).unwrap();
429-
range = round_down(range.start, readahead_alignment)
430-
..round_up(range.end, readahead_alignment).unwrap();
431-
if range.end > aligned_size {
432-
range.end = aligned_size;
433-
}
434446

435447
// Zero-pad the tail if requested range exceeds the size of the thing we're reading.
448+
// This can happen when we truncate and there are outstanding pager requests that the kernel
449+
// was not able to cancel in time.
436450
let mut offset = std::cmp::max(range.start, aligned_size);
437451
while offset < range.end {
438452
let end = std::cmp::min(range.end, offset + ZERO_VMO_SIZE);
439453
pager.supply_pages(this.vmo(), offset..end, &ZERO_VMO, 0);
440454
offset = end;
441455
}
442456

457+
let mut readahead_range = round_down(range.start, readahead_alignment)
458+
..std::cmp::min(round_up(range.end, readahead_alignment).unwrap(), aligned_size);
443459
// Read in chunks of 128 KiB.
444460
let read_size = round_up(128 * 1024, read_alignment).unwrap();
445-
446-
while range.start < range.end {
447-
let read_range = range.start..std::cmp::min(range.end, range.start + read_size);
448-
range.start += read_size;
461+
while readahead_range.start < readahead_range.end {
462+
let read_range = readahead_range.start
463+
..std::cmp::min(readahead_range.end, readahead_range.start + read_size);
464+
readahead_range.start += read_size;
449465

450466
let this = this.clone();
451467
this.clone().pager().spawn(page_in_chunk(this, read_range, ref_guard.clone()));

0 commit comments

Comments
 (0)