qiurui
/
asterinas
mirror of https://github.com/asterinas/asterinas.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make `wake_robust_futex` atomic 

Replace `VmWriter::atomic_update` with `VmWriter::atomic_compare_exchange`,
which takes the old value for comparison and new value instead of a
closure to compute it. This version has one less unsafe call.

Then use `atomic_compare_exchange` to reimplement the looping logic
of `wake_robust_futex` and make it atomic.
This commit is contained in:
Arthur Paulino 2025-08-18 20:34:01 -03:00 committed by Tate, Hongliang Tian
parent fc5a12356a
commit a73f210c7a
4 changed files with 89 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
kernel/src/context.rs
View File

@ -174,7 +174,7 @@ impl<'a> CurrentUserSpace<'a> {
/// Atomically updates a `PodAtomic` value with [`Ordering::Relaxed`] semantics.
///
/// This method internally uses an atomic compare-and-exchange operation.If the value changes
/// This method internally uses [`atomic_compare_exchange`]. If the value changes
/// concurrently, this method will retry so the operation may be performed multiple times.
///
/// # Panics
@ -183,18 +183,20 @@ impl<'a> CurrentUserSpace<'a> {
/// boundary.
///
/// [`Ordering::Relaxed`]: core::sync::atomic::Ordering::Relaxed
/// [`atomic_compare_exchange`]: VmWriter::atomic_compare_exchange
pub fn atomic_update<T>(&self, vaddr: Vaddr, op: impl Fn(T) -> T) -> Result<T>
where
T: PodAtomic + Eq,
{
check_vaddr(vaddr)?;
let writer = self.writer(vaddr, core::mem::size_of::<T>())?;
let reader = self.reader(vaddr, core::mem::size_of::<T>())?;
let user_reader = self.reader(vaddr, core::mem::size_of::<T>())?;
let mut user_writer = self.writer(vaddr, core::mem::size_of::<T>())?;
let mut old_val = reader.atomic_load()?;
loop {
match user_writer.atomic_update(&user_reader, &op)? {
(old_val, true) => return Ok(old_val),
(_, false) => continue,
match writer.atomic_compare_exchange(&reader, old_val, op(old_val))? {
(_, true) => return Ok(old_val),
(cur_val, false) => old_val = cur_val,
}
}
}

50
kernel/src/process/posix_thread/robust_list.rs
View File

@ -124,38 +124,44 @@ const FUTEX_WAITERS: u32 = 0x8000_0000;
const FUTEX_OWNER_DIED: u32 = 0x4000_0000;
const FUTEX_TID_MASK: u32 = 0x3FFF_FFFF;
/// Wakeup one robust futex owned by the thread
/// FIXME: requires atomic operations here
/// Attempts to wake a robust futex owned by the given thread.
///
/// If the futex at `futex_addr` is still owned by `tid`, it is marked with
/// `FUTEX_OWNER_DIED` and one waiter (if any) is woken.
/// If the futex is owned by another thread, the operation is canceled.
pub fn wake_robust_futex(futex_addr: Vaddr, tid: Tid) -> Result<()> {
if futex_addr == 0 {
return_errno_with_message!(Errno::EINVAL, "invalid futext addr");
}
let task = Task::current().unwrap();
let user_space = CurrentUserSpace::new(task.as_thread_local().unwrap());
let futex_val = {
if futex_addr == 0 {
return_errno_with_message!(Errno::EINVAL, "invalid futext addr");
}
user_space.read_val::<u32>(futex_addr)?
};
let mut old_val = futex_val;
// Instantiate reader and writer pointing at the same `futex_addr`, set up
// for the same length: the length of an `u32`.
const U32_LEN: usize = size_of::<u32>();
let writer = user_space.writer(futex_addr, U32_LEN)?;
let reader = user_space.reader(futex_addr, U32_LEN)?;
let mut old_val: u32 = reader.atomic_load()?;
loop {
// This futex may held by another thread, do nothing
// This futex may be held by another thread. If so, do nothing.
if old_val & FUTEX_TID_MASK != tid {
break;
}
let new_val = (old_val & FUTEX_WAITERS) | FUTEX_OWNER_DIED;
let cur_val = user_space.read_val(futex_addr)?;
if cur_val != new_val {
// The futex value has changed, let's retry with current value
old_val = cur_val;
user_space.write_val(futex_addr, &new_val)?;
continue;
match writer.atomic_compare_exchange(&reader, old_val, new_val)? {
(cur_val, false) => old_val = cur_val, // Try again with `cur_val`.
(_, true) => {
// Wake up one waiter and break out from the loop.
if new_val & FUTEX_WAITERS != 0 {
debug!("wake robust futex addr: {:?}", futex_addr);
futex_wake(futex_addr, 1, None)?;
}
break;
}
}
// Wakeup one waiter
if cur_val & FUTEX_WAITERS != 0 {
debug!("wake robust futex addr: {:?}", futex_addr);
futex_wake(futex_addr, 1, None)?;
}
break;
}
Ok(())
}

41
ostd/src/mm/io.rs
View File

@ -781,7 +781,7 @@ impl<'a> VmWriter<'a, Infallible> {
}
// Currently, there are no volatile atomic operations in `core::intrinsics`. Therefore, we do
// not provide an infallible implementation of `VmWriter::atomic_update`.
// not provide an infallible implementation of `VmWriter::atomic_compare_exchange`.
/// Writes `len` zeros to the target memory.
///
@ -859,27 +859,30 @@ impl VmWriter<'_, Fallible> {
Ok(())
}
/// Atomically updates a `PodAtomic` value.
/// Atomically compares and exchanges a `PodAtomic` value.
///
/// This is implemented by performing an atomic load, applying the operation, and performing an
/// atomic compare-and-exchange. So this cannot prevent the [ABA
/// problem](https://en.wikipedia.org/wiki/ABA_problem).
/// This method compares `old_val` with the value pointed by `self` and, if they are equal,
/// updates it with `new_val`.
///
/// The caller is required to provide a reader which points to the exactly same memory location
/// The value that was previously in memory will be returned, along with a boolean denoting
/// whether the compare-and-exchange succeeds. The caller usually wants to retry if this
/// flag is false, passing the most recent value that was returned by this method.
///
/// The caller is required to provide a reader which points to the exact same memory location
/// to ensure that reading from the memory is allowed.
///
/// On success, the previous value will be returned with a boolean value denoting whether the
/// compare-and-exchange succeeds. The caller usually wants to retry if the flag is false.
///
/// Regardless of whether it is successful, the cursor of the reader and writer will not move.
/// Regardless of whether it is successful, the cursors of the reader and writer will not move.
///
/// This method only guarantees the atomicity of the specific operation. There are no
/// synchronization constraints on other memory accesses. This aligns with the [Relaxed
/// ordering](https://en.cppreference.com/w/cpp/atomic/memory_order.html#Relaxed_ordering)
/// specified in the C++11 memory model.
///
/// Since the operation does not involve memory locks, it can't prevent the [ABA
/// problem](https://en.wikipedia.org/wiki/ABA_problem).
///
/// This method will fail with errors if:
/// 1. the remaining (avail) space of the reader (writer) is less than
/// 1. the remaining space of the reader or the available space of the writer are less than
/// `core::mem::size_of::<T>()` bytes, or
/// 2. the memory operation fails due to an unresolvable page fault.
///
@ -888,10 +891,11 @@ impl VmWriter<'_, Fallible> {
/// This method will panic if:
/// 1. the reader and the writer does not point to the same memory location, or
/// 2. the memory location is not aligned on a `core::mem::align_of::<T>()`-byte boundary.
pub fn atomic_update<T>(
&mut self,
pub fn atomic_compare_exchange<T>(
&self,
reader: &VmReader,
op: impl FnOnce(T) -> T,
old_val: T,
new_val: T,
) -> Result<(T, bool)>
where
T: PodAtomic + Eq,
@ -905,19 +909,12 @@ impl VmWriter<'_, Fallible> {
let cursor = self.cursor.cast::<T>();
assert!(cursor.is_aligned());
// SAFETY:
// 1. The cursor is either valid for reading or in user space for `size_of::<T>()` bytes.
// 2. The cursor is aligned on a `align_of::<T>()`-byte boundary.
let old_val = unsafe { T::atomic_load_fallible(cursor)? };
let new_val = op(old_val);
// SAFETY:
// 1. The cursor is either valid for reading and writing or in user space for 4 bytes.
// 2. The cursor is aligned on a 4-byte boundary.
let cur_val = unsafe { T::atomic_cmpxchg_fallible(cursor, old_val, new_val)? };
Ok((old_val, old_val == cur_val))
Ok((cur_val, old_val == cur_val))
}
/// Writes `len` zeros to the target memory.

60
ostd/src/mm/test.rs
View File

@ -345,41 +345,49 @@ mod io {
assert_eq!(reader_fallible.atomic_load::<u32>().unwrap(), 0x02020202);
}
/// Tests the `atomic_update` method in Fallible mode.
/// Tests the `atomic_compare_exchange` method in Fallible mode.
#[ktest]
fn atomic_update_fallible() {
type Segment = crate::mm::Segment<()>;
fn update(segment: &Segment, old_val: u32, new_val: u32, f: fn(segment: &Segment)) -> bool {
let (val, is_succ) = segment
.writer()
.to_fallible()
.skip(4)
.atomic_update(segment.reader().to_fallible().skip(4), |val| {
assert_eq!(val, old_val);
f(segment);
new_val
})
.unwrap();
assert_eq!(val, old_val);
is_succ
}
fn atomic_compare_exchange_fallible() {
let segment = FrameAllocOptions::new()
.zeroed(true)
.alloc_segment(1)
.unwrap();
assert!(update(&segment, 0, 100, |_| ()));
assert!(update(&segment, 100, 200, |_| ()));
let cmpxchg = |expected, new| {
segment
.writer()
.to_fallible()
.atomic_compare_exchange(&segment.reader().to_fallible(), expected, new)
.unwrap()
};
let is_succ = update(&segment, 200, 400, |segment| {
assert!(update(segment, 200, 300, |_| ()))
});
assert!(!is_succ);
// Initially 0, expect 0 -> succeed and set to 100
let (val, ok) = cmpxchg(0, 100);
assert_eq!(val, 0);
assert!(ok);
// Now 100, expect 100 -> succeed and set to 200
let (val, ok) = cmpxchg(100, 200);
assert_eq!(val, 100);
assert!(ok);
// Now 200, but we expect 300 -> fail, memory stays 200
let (val, ok) = cmpxchg(300, 400);
assert_eq!(val, 200);
assert!(!ok);
// Still 200, expect 200 -> succeed and set to 300
let (val, ok) = cmpxchg(200, 300);
assert_eq!(val, 200);
assert!(ok);
// Now 300, expect 200 -> fail, memory stays 300
let (val, ok) = cmpxchg(200, 400);
assert_eq!(val, 300);
assert!(!ok);
// Final check with raw reader
let mut reader = segment.reader().to_fallible();
reader.skip(4);
assert_eq!(reader.atomic_load::<u32>().unwrap(), 300);
assert_eq!(reader.read_val::<u32>().unwrap(), 300);
}