Skip to content

Fix fetch_and_atomic and fetch_or_atomic #684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 18, 2022
Merged

Fix fetch_and_atomic and fetch_or_atomic #684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15588a5
Fix fetch_and_atomic and fetch_or_atomic
wks Oct 17, 2022
b9dfcf0
Fix bit ops and tests
wks Oct 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 82 additions & 66 deletions src/util/metadata/header_metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,8 @@ impl HeaderMetadataSpec {
#[cfg(debug_assertions)]
self.assert_spec::<T>();
if self.num_of_bits < 8 {
let (_, mask) = self.get_shift_and_mask_for_bits();
let new_val = val.to_u8().unwrap() | !mask;
let (lshift, mask) = self.get_shift_and_mask_for_bits();
let new_val = (val.to_u8().unwrap() << lshift) | !mask;
// We do not need to use fetch_ops_on_bits(), we can just set irrelavent bits to 1, and do fetch_and
let old_raw_byte =
unsafe { <u8 as MetadataValue>::fetch_and(self.meta_addr(object), new_val, order) };
Expand All @@ -425,9 +425,9 @@ impl HeaderMetadataSpec {
#[cfg(debug_assertions)]
self.assert_spec::<T>();
if self.num_of_bits < 8 {
let (_, mask) = self.get_shift_and_mask_for_bits();
let new_val = val.to_u8().unwrap() & mask;
// We do not need to use fetch_ops_on_bits(), we can just set irrelavent bits to 1, and do fetch_and
let (lshift, mask) = self.get_shift_and_mask_for_bits();
let new_val = (val.to_u8().unwrap() << lshift) & mask;
// We do not need to use fetch_ops_on_bits(), we can just set irrelavent bits to 0, and do fetch_or
let old_raw_byte =
unsafe { <u8 as MetadataValue>::fetch_or(self.meta_addr(object), new_val, order) };
let old_val = self.get_bits_from_u8(old_raw_byte);
Expand Down Expand Up @@ -882,123 +882,139 @@ mod tests {
#[test]
fn [<$tname _fetch_add>]() {
[<with_ $type _obj>](|obj, _| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;

let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);

let old_val_from_fetch = spec.fetch_add::<$type>(obj, max_value, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
let old_val_from_fetch = spec.fetch_add::<$type>(obj, max_value, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
}
})
}

#[test]
fn [<$tname _fetch_add_overflow>]() {
[<with_ $type _obj>](|obj, ptr| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;

unsafe { spec.store::<$type>(obj, max_value, None) };
let old_val = unsafe { spec.load::<$type>(obj, None) };

// add 1 will cause overflow
let old_val_from_fetch = spec.fetch_add::<$type>(obj, 1, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
assert_eq!(unsafe { *ptr }, 0); // we should not accidentally affect other bits
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;

unsafe { spec.store::<$type>(obj, max_value, None) };
let old_val = unsafe { spec.load::<$type>(obj, None) };

// add 1 will cause overflow
let old_val_from_fetch = spec.fetch_add::<$type>(obj, 1, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
assert_eq!(unsafe { *ptr }, 0); // we should not accidentally affect other bits
}
})
}

#[test]
fn [<$tname _fetch_sub>]() {
[<with_ $type _obj>](|obj, _| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };

unsafe { spec.store::<$type>(obj, 1, None) };
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 1);
unsafe { spec.store::<$type>(obj, 1, None) };
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 1);

let old_val_from_fetch = spec.fetch_sub::<$type>(obj, 1, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
let old_val_from_fetch = spec.fetch_sub::<$type>(obj, 1, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
}
})
}

#[test]
fn [<$tname _fetch_sub_overflow>]() {
[<with_ $type _obj>](|obj, _| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;

let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);

let old_val_from_fetch = spec.fetch_sub::<$type>(obj, 1, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
let old_val_from_fetch = spec.fetch_sub::<$type>(obj, 1, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
}
})
}

#[test]
fn [<$tname _fetch_and>]() {
[<with_ $type _obj>](|obj, _| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;

let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);

let old_val_from_fetch = spec.fetch_and::<$type>(obj, max_value, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
let old_val_from_fetch = spec.fetch_and::<$type>(obj, max_value, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
}
})
}

#[test]
fn [<$tname _fetch_or>]() {
[<with_ $type _obj>](|obj, _| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;

let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);

let old_val_from_fetch = spec.fetch_or::<$type>(obj, max_value, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
let old_val_from_fetch = spec.fetch_or::<$type>(obj, max_value, Ordering::SeqCst);
assert_eq!(old_val, old_val_from_fetch);
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
}
})
}

#[test]
fn [<$tname _fetch_update_success>]() {
[<with_ $type _obj>](|obj, _| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };
let max_value = max_value($num_of_bits) as $type;

let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);

let update_res = spec.fetch_update(obj, Ordering::SeqCst, Ordering::SeqCst, |_x: $type| Some(max_value));
assert!(update_res.is_ok());
assert_eq!(old_val, update_res.unwrap());
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
let update_res = spec.fetch_update(obj, Ordering::SeqCst, Ordering::SeqCst, |_x: $type| Some(max_value));
assert!(update_res.is_ok());
assert_eq!(old_val, update_res.unwrap());
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, max_value);
}
})
}

#[test]
fn [<$tname _fetch_update_fail>]() {
[<with_ $type _obj>](|obj, _| {
let spec = HeaderMetadataSpec { bit_offset: 0, num_of_bits: $num_of_bits };
for bit_offset in (0isize..($type::BITS as isize)).step_by($num_of_bits) {
let spec = HeaderMetadataSpec { bit_offset, num_of_bits: $num_of_bits };

let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);
let old_val = unsafe { spec.load::<$type>(obj, None) };
assert_eq!(old_val, 0);

let update_res = spec.fetch_update(obj, Ordering::SeqCst, Ordering::SeqCst, |_x: $type| None);
assert!(update_res.is_err());
assert_eq!(old_val, update_res.err().unwrap());
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
let update_res = spec.fetch_update(obj, Ordering::SeqCst, Ordering::SeqCst, |_x: $type| None);
assert!(update_res.is_err());
assert_eq!(old_val, update_res.err().unwrap());
assert_eq!(unsafe { spec.load::<$type>(obj, None) }, 0);
}
})
}
}
Expand Down
9 changes: 4 additions & 5 deletions src/util/metadata/side_metadata/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,9 +538,9 @@ impl SideMetadataSpec {
let lshift = meta_byte_lshift(self, data_addr);
let mask = meta_byte_mask(self) << lshift;
// We do not need to use fetch_ops_on_bits(), we can just set irrelavent bits to 1, and do fetch_and
let new_val = val.to_u8().unwrap() | !mask;
let rhs = (val.to_u8().unwrap() << lshift) | !mask;
let old_raw_byte =
unsafe { <u8 as MetadataValue>::fetch_and(meta_addr, new_val, order) };
unsafe { <u8 as MetadataValue>::fetch_and(meta_addr, rhs, order) };
let old_val = (old_raw_byte & mask) >> lshift;
FromPrimitive::from_u8(old_val).unwrap()
} else {
Expand Down Expand Up @@ -570,9 +570,9 @@ impl SideMetadataSpec {
let lshift = meta_byte_lshift(self, data_addr);
let mask = meta_byte_mask(self) << lshift;
// We do not need to use fetch_ops_on_bits(), we can just set irrelavent bits to 0, and do fetch_or
let new_val = val.to_u8().unwrap() & mask;
let rhs = (val.to_u8().unwrap() << lshift) & mask;
let old_raw_byte =
unsafe { <u8 as MetadataValue>::fetch_or(meta_addr, new_val, order) };
unsafe { <u8 as MetadataValue>::fetch_or(meta_addr, rhs, order) };
let old_val = (old_raw_byte & mask) >> lshift;
FromPrimitive::from_u8(old_val).unwrap()
} else {
Expand Down Expand Up @@ -1019,7 +1019,6 @@ mod tests {

let data_addr = vm_layout_constants::HEAP_START;
let meta_addr = address_to_meta_address(&spec, data_addr);

with_cleanup(
|| {
let mmap_result = context.try_map_metadata_space(data_addr, BYTES_IN_PAGE);
Expand Down
61 changes: 60 additions & 1 deletion src/util/metadata/side_metadata/side_metadata_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ mod tests {
// We need to do this because of the static NO_METADATA
// sanity::reset();
let data_addr = vm_layout_constants::HEAP_START
+ (vm_layout_constants::BYTES_IN_CHUNK << 1);
+ (vm_layout_constants::BYTES_IN_CHUNK << 1) * 2;

let metadata_1_spec = SideMetadataSpec {
name: "metadata_1_spec",
Expand Down Expand Up @@ -416,6 +416,65 @@ mod tests {
let one = metadata_1_spec.load_atomic::<u8>(data_addr, Ordering::SeqCst);
assert_eq!(one, 1);

metadata_1_spec.store_atomic::<u8>(data_addr, 0, Ordering::SeqCst);

metadata.ensure_unmap_metadata_space(data_addr, constants::BYTES_IN_PAGE);

metadata_sanity.reset();
},
|| {
sanity::reset();
},
);
});
}

#[test]
fn test_side_metadata_atomic_fetch_and_or_2bits() {
serial_test(|| {
with_cleanup(
|| {
// We need to do this because of the static NO_METADATA
// sanity::reset();
let data_addr = vm_layout_constants::HEAP_START
+ (vm_layout_constants::BYTES_IN_CHUNK << 1);

let metadata_1_spec = SideMetadataSpec {
name: "metadata_1_spec",
is_global: true,
offset: SideMetadataOffset::addr(GLOBAL_SIDE_METADATA_BASE_ADDRESS),
log_num_of_bits: 1,
log_bytes_in_region: constants::LOG_BYTES_IN_WORD as usize,
};

let metadata = SideMetadataContext {
global: vec![metadata_1_spec],
local: vec![],
};

let mut metadata_sanity = SideMetadataSanity::new();
metadata_sanity.verify_metadata_context("NoPolicy", &metadata);

assert!(metadata
.try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE,)
.is_ok());

let zero =
metadata_1_spec.fetch_or_atomic::<u8>(data_addr, 0b11, Ordering::SeqCst);
assert_eq!(zero, 0);

let value_11 = metadata_1_spec.load_atomic::<u8>(data_addr, Ordering::SeqCst);
assert_eq!(value_11, 0b11);

let another_value_11 =
metadata_1_spec.fetch_and_atomic::<u8>(data_addr, 0b01, Ordering::SeqCst);
assert_eq!(another_value_11, 0b11);

let value_01 = metadata_1_spec.load_atomic::<u8>(data_addr, Ordering::SeqCst);
assert_eq!(value_01, 0b01);

metadata_1_spec.store_atomic::<u8>(data_addr, 0, Ordering::SeqCst);

metadata.ensure_unmap_metadata_space(data_addr, constants::BYTES_IN_PAGE);

metadata_sanity.reset();
Expand Down