diff options
| author | Frank Rehwinkel <frankrehwinkel@gmail.com> | 2024-10-02 08:04:25 -0400 |
|---|---|---|
| committer | Oli Scherer <github@oli-obk.de> | 2024-10-02 20:08:46 +0200 |
| commit | 86bb1373aa94aec8d9d4c8fb692cd352eca01d90 (patch) | |
| tree | 7b15ff0a0f144ac0605a835276c6ebe7520f5df6 /src | |
| parent | 1b622f46728a51b6582b02b84407e5f64262479b (diff) | |
| download | rust-86bb1373aa94aec8d9d4c8fb692cd352eca01d90.tar.gz rust-86bb1373aa94aec8d9d4c8fb692cd352eca01d90.zip | |
epoll: add vector clock to the epoll ready_list
This adds a VClock to the epoll implementation's ready_list and has this VClock synced from the thread that updates an event in the ready_list and then has the VClocks of any threads being made runnable again, out of the calls to epoll_wait, synced from it.
Diffstat (limited to 'src')
4 files changed, 67 insertions, 119 deletions
diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index 13c44334d58..88fab8aa010 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -4,6 +4,7 @@ use std::io; use std::rc::{Rc, Weak}; use std::time::Duration; +use crate::concurrency::VClock; use crate::shims::unix::fd::{FdId, FileDescriptionRef, WeakFileDescriptionRef}; use crate::shims::unix::*; use crate::*; @@ -19,7 +20,7 @@ struct Epoll { /// and file descriptor value. // This is an Rc because EpollInterest need to hold a reference to update // it. - ready_list: Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>, + ready_list: Rc<ReadyList>, /// A list of thread ids blocked on this epoll instance. thread_id: RefCell<Vec<ThreadId>>, } @@ -63,7 +64,7 @@ pub struct EpollEventInterest { /// <https://man7.org/linux/man-pages/man3/epoll_event.3type.html> data: u64, /// Ready list of the epoll instance under which this EpollEventInterest is registered. - ready_list: Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>, + ready_list: Rc<ReadyList>, /// The epoll file description that this EpollEventInterest is registered under. weak_epfd: WeakFileDescriptionRef, } @@ -88,6 +89,12 @@ pub struct EpollReadyEvents { pub epollerr: bool, } +#[derive(Debug, Default)] +struct ReadyList { + mapping: RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>, + clock: RefCell<VClock>, +} + impl EpollReadyEvents { pub fn new() -> Self { EpollReadyEvents { @@ -127,7 +134,7 @@ impl EpollReadyEvents { } impl Epoll { - fn get_ready_list(&self) -> Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>> { + fn get_ready_list(&self) -> Rc<ReadyList> { Rc::clone(&self.ready_list) } } @@ -374,7 +381,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { drop(epoll_interest); // Remove related epoll_interest from ready list. - ready_list.borrow_mut().remove(&epoll_key); + ready_list.mapping.borrow_mut().remove(&epoll_key); // Remove dangling EpollEventInterest from its global table. // .unwrap() below should succeed because the file description id must have registered @@ -469,7 +476,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { .downcast::<Epoll>() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; let binding = epoll_file_description.get_ready_list(); - ready_list_empty = binding.borrow_mut().is_empty(); + ready_list_empty = binding.mapping.borrow_mut().is_empty(); thread_ids = epoll_file_description.thread_id.borrow_mut(); } if timeout == 0 || !ready_list_empty { @@ -558,9 +565,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // holds a strong ref to epoll_interest. let epfd = epoll_interest.borrow().weak_epfd.upgrade().unwrap(); // FIXME: We can randomly pick a thread to unblock. - if let Some(thread_id) = - epfd.downcast::<Epoll>().unwrap().thread_id.borrow_mut().pop() - { + + let epoll = epfd.downcast::<Epoll>().unwrap(); + + // Synchronize running thread to the epoll ready list. + if let Some(clock) = &this.release_clock() { + epoll.ready_list.clock.borrow_mut().join(clock); + } + + if let Some(thread_id) = epoll.thread_id.borrow_mut().pop() { waiter.push(thread_id); }; } @@ -614,7 +627,7 @@ fn check_and_update_one_event_interest<'tcx>( // insert an epoll_return to the ready list. if flags != 0 { let epoll_key = (id, epoll_event_interest.fd_num); - let ready_list = &mut epoll_event_interest.ready_list.borrow_mut(); + let ready_list = &mut epoll_event_interest.ready_list.mapping.borrow_mut(); let event_instance = EpollEventInstance::new(flags, epoll_event_interest.data); // Triggers the notification by inserting it to the ready list. ready_list.insert(epoll_key, event_instance); @@ -641,7 +654,11 @@ fn blocking_epoll_callback<'tcx>( .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; let ready_list = epoll_file_description.get_ready_list(); - let mut ready_list = ready_list.borrow_mut(); + + // Synchronize waking thread from the epoll ready list. + ecx.acquire_clock(&ready_list.clock.borrow()); + + let mut ready_list = ready_list.mapping.borrow_mut(); let mut num_of_events: i32 = 0; let mut array_iter = ecx.project_array_fields(events)?; diff --git a/src/tools/miri/tests/fail-dep/libc/libc-epoll-blocking.rs b/src/tools/miri/tests/fail-dep/libc/libc-epoll-blocking.rs deleted file mode 100644 index fd9d1875daf..00000000000 --- a/src/tools/miri/tests/fail-dep/libc/libc-epoll-blocking.rs +++ /dev/null @@ -1,79 +0,0 @@ -//@only-target: linux -// test_epoll_race depends on a deterministic schedule. -//@compile-flags: -Zmiri-preemption-rate=0 - -use std::convert::TryInto; -use std::thread; - -fn main() { - test_epoll_race(); -} - -// Using `as` cast since `EPOLLET` wraps around -const EPOLL_IN_OUT_ET: u32 = (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET) as _; - -#[track_caller] -fn check_epoll_wait<const N: usize>( - epfd: i32, - expected_notifications: &[(u32, u64)], - timeout: i32, -) { - let epoll_event = libc::epoll_event { events: 0, u64: 0 }; - let mut array: [libc::epoll_event; N] = [epoll_event; N]; - let maxsize = N; - let array_ptr = array.as_mut_ptr(); - let res = unsafe { libc::epoll_wait(epfd, array_ptr, maxsize.try_into().unwrap(), timeout) }; - if res < 0 { - panic!("epoll_wait failed: {}", std::io::Error::last_os_error()); - } - assert_eq!( - res, - expected_notifications.len().try_into().unwrap(), - "got wrong number of notifications" - ); - let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; - for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) { - let event = return_event.events; - let data = return_event.u64; - assert_eq!(event, expected_event.0, "got wrong events"); - assert_eq!(data, expected_event.1, "got wrong data"); - } -} - -// This test shows a data_race before epoll had vector clocks added. -fn test_epoll_race() { - // Create an epoll instance. - let epfd = unsafe { libc::epoll_create1(0) }; - assert_ne!(epfd, -1); - - // Create an eventfd instance. - let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; - let fd = unsafe { libc::eventfd(0, flags) }; - - // Register eventfd with the epoll instance. - let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd as u64 }; - let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) }; - assert_eq!(res, 0); - - static mut VAL: u8 = 0; - let thread1 = thread::spawn(move || { - // Write to the static mut variable. - unsafe { VAL = 1 }; - // Write to the eventfd instance. - let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes(); - let res = unsafe { libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; - // read returns number of bytes that have been read, which is always 8. - assert_eq!(res, 8); - }); - thread::yield_now(); - // epoll_wait for the event to happen. - let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); - let expected_value = u64::try_from(fd).unwrap(); - check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)], -1); - // Read from the static mut variable. - #[allow(static_mut_refs)] - unsafe { - assert_eq!(VAL, 1) //~ ERROR: Data race detected - }; - thread1.join().unwrap(); -} diff --git a/src/tools/miri/tests/fail-dep/libc/libc-epoll-blocking.stderr b/src/tools/miri/tests/fail-dep/libc/libc-epoll-blocking.stderr deleted file mode 100644 index 27974981cea..00000000000 --- a/src/tools/miri/tests/fail-dep/libc/libc-epoll-blocking.stderr +++ /dev/null @@ -1,29 +0,0 @@ -error: Undefined Behavior: Data race detected between (1) non-atomic write on thread `unnamed-ID` and (2) retag read of type `u8` on thread `main` at ALLOC. (2) just happened here - --> tests/fail-dep/libc/libc-epoll-blocking.rs:LL:CC - | -LL | assert_eq!(VAL, 1) - | ^^^^^^^^^^^^^^^^^^ Data race detected between (1) non-atomic write on thread `unnamed-ID` and (2) retag read of type `u8` on thread `main` at ALLOC. (2) just happened here - | -help: and (1) occurred earlier here - --> tests/fail-dep/libc/libc-epoll-blocking.rs:LL:CC - | -LL | unsafe { VAL = 1 }; - | ^^^^^^^ - = help: retags occur on all (re)borrows and as well as when references are copied or moved - = help: retags permit optimizations that insert speculative reads or writes - = help: therefore from the perspective of data races, a retag has the same implications as a read or write - = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior - = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information - = note: BACKTRACE (of the first span): - = note: inside `test_epoll_race` at RUSTLIB/core/src/macros/mod.rs:LL:CC -note: inside `main` - --> tests/fail-dep/libc/libc-epoll-blocking.rs:LL:CC - | -LL | test_epoll_race(); - | ^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to 1 previous error - diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs index eb38529ae57..d7675a40163 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs @@ -1,5 +1,5 @@ //@only-target: linux -// test_epoll_block_then_unblock depends on a deterministic schedule. +// test_epoll_block_then_unblock and test_epoll_race depend on a deterministic schedule. //@compile-flags: -Zmiri-preemption-rate=0 use std::convert::TryInto; @@ -12,6 +12,7 @@ fn main() { test_epoll_block_without_notification(); test_epoll_block_then_unblock(); test_notification_after_timeout(); + test_epoll_race(); } // Using `as` cast since `EPOLLET` wraps around @@ -137,3 +138,41 @@ fn test_notification_after_timeout() { let expected_value = fds[0] as u64; check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], 10); } + +// This test shows a data_race before epoll had vector clocks added. +fn test_epoll_race() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create an eventfd instance. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd = unsafe { libc::eventfd(0, flags) }; + + // Register eventfd with the epoll instance. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) }; + assert_eq!(res, 0); + + static mut VAL: u8 = 0; + let thread1 = thread::spawn(move || { + // Write to the static mut variable. + unsafe { VAL = 1 }; + // Write to the eventfd instance. + let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes(); + let res = unsafe { libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; + // read returns number of bytes that have been read, which is always 8. + assert_eq!(res, 8); + }); + thread::yield_now(); + // epoll_wait for the event to happen. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fd).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)], -1); + // Read from the static mut variable. + #[allow(static_mut_refs)] + unsafe { + assert_eq!(VAL, 1) + }; + thread1.join().unwrap(); +} |
