Skip to content

Commit

Permalink
Fix find_last_non_zero_bit, and align metadata address before convert…
Browse files Browse the repository at this point in the history
…ing to data address. (#1188)

This PR fixes some bugs in finding base references for internal
references. Prominently two bugs:
1. The way we find the last non zero bit was wrong. We used to to use
`trailing_zeroes` and compare the result with the given range of the
starting and ending bits. This was simply wrong. Now we do a mask first
to extract the value between the starting and the ending bits, and then
use `leading_zeroes`. Performance-wise, the new code has similar
performnace.
2. When we convert a metadata bit (address + bit offset) back to the
data address, the metadata bit needs to be the start of the metadata
value. If it is the middle of the metadata value, then the computed data
address will be incorrect. This PR adds `align_metadata_address`.

This PR skips some tests for LOS in plans that do not use LOS.
  • Loading branch information
qinsoon authored Aug 28, 2024
1 parent c320cf3 commit 963e9c2
Show file tree
Hide file tree
Showing 4 changed files with 289 additions and 28 deletions.
19 changes: 14 additions & 5 deletions src/util/metadata/side_metadata/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,7 @@ impl SideMetadataSpec {
///
/// This function uses non-atomic load for the side metadata. The user needs to make sure
/// that there is no other thread that is mutating the side metadata.
#[allow(clippy::let_and_return)]
pub unsafe fn find_prev_non_zero_value<T: MetadataValue>(
&self,
data_addr: Address,
Expand All @@ -1000,7 +1001,15 @@ impl SideMetadataSpec {

if self.uses_contiguous_side_metadata() {
// Contiguous side metadata
self.find_prev_non_zero_value_fast::<T>(data_addr, search_limit_bytes)
let result = self.find_prev_non_zero_value_fast::<T>(data_addr, search_limit_bytes);
#[cfg(debug_assertions)]
{
// Double check if the implementation is correct
let result2 =
self.find_prev_non_zero_value_simple::<T>(data_addr, search_limit_bytes);
assert_eq!(result, result2, "find_prev_non_zero_value_fast returned a diffrent result from the naive implementation.");
}
result
} else {
// TODO: We should be able to optimize further for this case. However, we need to be careful that the side metadata
// is not contiguous, and we need to skip to the next chunk's side metadata when we search to a different chunk.
Expand All @@ -1018,12 +1027,10 @@ impl SideMetadataSpec {
let region_bytes = 1 << self.log_bytes_in_region;
// Figure out the range that we need to search.
let start_addr = data_addr.align_down(region_bytes);
let end_addr = data_addr
.saturating_sub(search_limit_bytes)
.align_down(region_bytes);
let end_addr = data_addr.saturating_sub(search_limit_bytes) + 1usize;

let mut cursor = start_addr;
while cursor > end_addr {
while cursor >= end_addr {
// We encounter an unmapped address. Just return None.
if !cursor.is_mapped() {
return None;
Expand Down Expand Up @@ -1073,6 +1080,7 @@ impl SideMetadataSpec {
BitByteRange::Bytes { start, end } => {
match helpers::find_last_non_zero_bit_in_metadata_bytes(start, end) {
helpers::FindMetaBitResult::Found { addr, bit } => {
let (addr, bit) = align_metadata_address(self, addr, bit);
res = Some(contiguous_meta_address_to_address(self, addr, bit));
// Return true to abort the search. We found the bit.
true
Expand All @@ -1091,6 +1099,7 @@ impl SideMetadataSpec {
match helpers::find_last_non_zero_bit_in_metadata_bits(addr, bit_start, bit_end)
{
helpers::FindMetaBitResult::Found { addr, bit } => {
let (addr, bit) = align_metadata_address(self, addr, bit);
res = Some(contiguous_meta_address_to_address(self, addr, bit));
// Return true to abort the search. We found the bit.
true
Expand Down
Loading

0 comments on commit 963e9c2

Please sign in to comment.