Skip to content

Commit e6cd0bc

Browse files
Aaron DrewCQ Bot
authored andcommitted
[fxfs] Move bitmap-based zeroing out of read_and_decrypt.
Externalized to be more in-keeping with "one function, one purpose". Change-Id: I8467be7f20aa14aa13fe7b0317bfa29973b65869 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1276525 Reviewed-by: Stephen Demos <[email protected]> Commit-Queue: Auto-Submit <auto-submit-builder@fuchsia-internal-service-accts.iam.gserviceaccount.com> Fuchsia-Auto-Submit: Aaron Drew <[email protected]>
1 parent df47ba7 commit e6cd0bc

File tree

2 files changed

+37
-35
lines changed

2 files changed

+37
-35
lines changed

src/storage/fxfs/src/object_store/data_object_handle.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,14 +1175,8 @@ impl<S: HandleOwner> DataObjectHandle<S> {
11751175
// which case we don't actually need to bother zeroing out the tail. However,
11761176
// it's not strictly incorrect to change uninitialized data, so we skip the
11771177
// check and blindly do it to keep it simpler here.
1178-
self.read_and_decrypt(
1179-
device_offset,
1180-
aligned_old_size,
1181-
buf.as_mut(),
1182-
*key_id,
1183-
None,
1184-
)
1185-
.await?;
1178+
self.read_and_decrypt(device_offset, aligned_old_size, buf.as_mut(), *key_id)
1179+
.await?;
11861180
buf.as_mut_slice()[(old_size % block_size) as usize..].fill(0);
11871181
self.multi_write(
11881182
transaction,
@@ -1417,9 +1411,8 @@ impl<S: HandleOwner> DataObjectHandle<S> {
14171411
file_offset: u64,
14181412
buffer: MutableBufferRef<'_>,
14191413
key_id: u64,
1420-
block_bitmap: Option<bit_vec::BitVec>,
14211414
) -> Result<(), Error> {
1422-
self.handle.read_and_decrypt(device_offset, file_offset, buffer, key_id, block_bitmap).await
1415+
self.handle.read_and_decrypt(device_offset, file_offset, buffer, key_id).await
14231416
}
14241417

14251418
/// Truncates a file to a given size (growing/shrinking as required).

src/storage/fxfs/src/object_store/store_object_handle.rs

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,22 @@ pub const MAX_XATTR_VALUE_SIZE: usize = 64000;
5757
pub const EXTENDED_ATTRIBUTE_RANGE_START: u64 = 64;
5858
pub const EXTENDED_ATTRIBUTE_RANGE_END: u64 = 512;
5959

60+
/// Zeroes blocks in 'buffer' based on `bitmap`, one bit per block from start of buffer.
61+
fn apply_bitmap_zeroing(
62+
block_size: usize,
63+
bitmap: &bit_vec::BitVec,
64+
mut buffer: MutableBufferRef<'_>,
65+
) {
66+
let buf = buffer.as_mut_slice();
67+
debug_assert_eq!(bitmap.len() * block_size, buf.len());
68+
for (i, block) in bitmap.iter().enumerate() {
69+
if !block {
70+
let start = i * block_size;
71+
buf[start..start + block_size].fill(0);
72+
}
73+
}
74+
}
75+
6076
/// When writing, often the logic should be generic over whether or not checksums are generated.
6177
/// This provides that and a handy way to convert to the more general ExtentMode that eventually
6278
/// stores it on disk.
@@ -697,10 +713,6 @@ impl<S: HandleOwner> StoreObjectHandle<S> {
697713
file_offset: u64,
698714
mut buffer: MutableBufferRef<'_>,
699715
key_id: u64,
700-
// If provided, blocks in the bitmap that are zero will have their contents zeroed out. The
701-
// bitmap should be exactly the size of the buffer and aligned to the offset in the extent
702-
// the read is starting at.
703-
block_bitmap: Option<bit_vec::BitVec>,
704716
) -> Result<(), Error> {
705717
let store = self.store();
706718
store.device_read_ops.fetch_add(1, Ordering::Relaxed);
@@ -717,17 +729,6 @@ impl<S: HandleOwner> StoreObjectHandle<S> {
717729
if let Some(key) = key {
718730
key.decrypt(file_offset, buffer.as_mut_slice())?;
719731
}
720-
if let Some(bitmap) = block_bitmap {
721-
let block_size = self.block_size() as usize;
722-
let buf = buffer.as_mut_slice();
723-
debug_assert_eq!(bitmap.len() * block_size, buf.len());
724-
for (i, block) in bitmap.iter().enumerate() {
725-
if !block {
726-
let start = i * block_size;
727-
buf[start..start + block_size].fill(0);
728-
}
729-
}
730-
}
731732
Ok(())
732733
}
733734

@@ -985,7 +986,7 @@ impl<S: HandleOwner> StoreObjectHandle<S> {
985986
"R",
986987
);
987988
}
988-
let (head, tail) = buf.split_at_mut(to_copy);
989+
let (mut head, tail) = buf.split_at_mut(to_copy);
989990
let maybe_bitmap = match mode {
990991
ExtentMode::OverwritePartial(bitmap) => {
991992
let mut read_bitmap = bitmap.clone().split_off(
@@ -996,13 +997,15 @@ impl<S: HandleOwner> StoreObjectHandle<S> {
996997
}
997998
_ => None,
998999
};
999-
reads.push(self.read_and_decrypt(
1000-
device_offset,
1001-
offset,
1002-
head,
1003-
key_id,
1004-
maybe_bitmap,
1005-
));
1000+
reads.push(async move {
1001+
self
1002+
.read_and_decrypt(device_offset, offset, head.reborrow(), key_id)
1003+
.await?;
1004+
if let Some(bitmap) = maybe_bitmap {
1005+
apply_bitmap_zeroing(self.block_size() as usize, &bitmap, head);
1006+
}
1007+
Ok::<(), Error>(())
1008+
});
10061009
buf = tail;
10071010
if buf.is_empty() {
10081011
break;
@@ -1031,7 +1034,7 @@ impl<S: HandleOwner> StoreObjectHandle<S> {
10311034
"RT",
10321035
);
10331036
}
1034-
self.read_and_decrypt(device_offset, offset, align_buf.as_mut(), key_id, None)
1037+
self.read_and_decrypt(device_offset, offset, align_buf.as_mut(), key_id)
10351038
.await?;
10361039
buf.as_mut_slice().copy_from_slice(&align_buf.as_slice()[..end_align]);
10371040
buf = buf.subslice_mut(0..0);
@@ -1112,9 +1115,15 @@ impl<S: HandleOwner> StoreObjectHandle<S> {
11121115
extent_key.range.start,
11131116
buffer.subslice_mut(offset..end as usize),
11141117
*key_id,
1115-
maybe_bitmap,
11161118
)
11171119
.await?;
1120+
if let Some(bitmap) = maybe_bitmap {
1121+
apply_bitmap_zeroing(
1122+
self.block_size() as usize,
1123+
&bitmap,
1124+
buffer.subslice_mut(offset..end as usize),
1125+
);
1126+
}
11181127
last_offset = end;
11191128
if last_offset >= size {
11201129
break;

0 commit comments

Comments
 (0)