Refactor futex wait with timeout

src/libos/src/process/do_exec.rs

@@ -109,6 +109,7 @@ fn wait_for_other_threads_to_exit(current_ref: &ThreadRef) {
     let timeout = FutexTimeout::new(
         ClockID::CLOCK_MONOTONIC,
         timespec_t::from(Duration::from_millis(50)),
+        false,
     );
     // Use calling process's pointer as futex value
     let futex_val = Arc::as_ptr(current_ref.process()) as *const i32;

src/libos/src/process/do_futex.rs

@@ -9,7 +9,7 @@ use crate::time::{timespec_t, ClockID};
 /// `FutexOp`, `FutexFlags`, and `futex_op_and_flags_from_u32` are helper types and
 /// functions for handling the versatile commands and arguments of futex system
 /// call in a memory-safe way.
-
+#[derive(PartialEq)]
 #[allow(non_camel_case_types)]
 pub enum FutexOp {
     FUTEX_WAIT = 0,
@@ -77,11 +77,16 @@ const FUTEX_BITSET_MATCH_ANY: u32 = 0xFFFF_FFFF;
 pub struct FutexTimeout {
     clock_id: ClockID,
     ts: timespec_t,
+    absolute_time: bool,
 }
 
 impl FutexTimeout {
-    pub fn new(clock_id: ClockID, ts: timespec_t) -> Self {
-        Self { clock_id, ts }
+    pub fn new(clock_id: ClockID, ts: timespec_t, absolute_time: bool) -> Self {
+        Self {
+            clock_id,
+            ts,
+            absolute_time,
+        }
     }
 
     pub fn clock_id(&self) -> &ClockID {
@@ -91,6 +96,10 @@ impl FutexTimeout {
     pub fn ts(&self) -> &timespec_t {
         &self.ts
     }
+
+    pub fn absolute_time(&self) -> bool {
+        self.absolute_time
+    }
 }
 
 /// Do futex wait
@@ -485,16 +494,20 @@ unsafe impl Sync for Waiter {}
 fn wait_event_timeout(thread: *const c_void, timeout: &Option<FutexTimeout>) -> Result<()> {
     let mut ret: c_int = 0;
     let mut sgx_ret: c_int = 0;
-    let (clockbit, ts_ptr) = timeout
+    let (clockbit, ts_ptr, absolute_time) = timeout
         .as_ref()
         .map(|timeout| {
             let clockbit = match timeout.clock_id() {
                 ClockID::CLOCK_REALTIME => FutexFlags::FUTEX_CLOCK_REALTIME.bits() as i32,
                 _ => 0,
             };
-            (clockbit, timeout.ts() as *const timespec_t)
+            (
+                clockbit,
+                timeout.ts() as *const timespec_t,
+                timeout.absolute_time() as i32,
+            )
         })
-        .unwrap_or((0, 0 as *const _));
+        .unwrap_or((0, 0 as *const _, 0));
     let mut errno: c_int = 0;
     unsafe {
         sgx_ret = sgx_thread_wait_untrusted_event_timeout_ocall(
@@ -502,6 +515,7 @@ fn wait_event_timeout(thread: *const c_void, timeout: &Option<FutexTimeout>) ->
             thread,
             clockbit,
             ts_ptr,
+            absolute_time,
             &mut errno as *mut c_int,
         );
         assert!(sgx_ret == 0);
@@ -550,6 +564,7 @@ extern "C" {
         self_thread: *const c_void,
         clockbit: i32,
         ts: *const timespec_t,
+        absolute_time: i32,
         errno: *mut c_int,
     ) -> c_int;
 

src/libos/src/process/syscalls.rs

@@ -282,6 +282,14 @@ pub fn do_futex(
         ts.validate()?;
         // TODO: use a secure clock to transfer the real time to monotonic time
         let clock_id = if futex_flags.contains(FutexFlags::FUTEX_CLOCK_REALTIME) {
+            // FUTEX_WAIT can't work with FUTEX_CLOCK_REALTIME. This rule is applied by Linux. And we shall just obey it.
+            // Reference: https://github.com/torvalds/linux/commit/4fbf5d6837bf81fd7a27d771358f4ee6c4f243f8
+            if futex_op == FutexOp::FUTEX_WAIT {
+                return_errno!(
+                    ENOSYS,
+                    "FUTEX_CLOCK_REALTIME with this futex operation is not supported"
+                );
+            }
             ClockID::CLOCK_REALTIME
         } else {
             ClockID::CLOCK_MONOTONIC
@@ -291,24 +299,14 @@ pub fn do_futex(
         // for FUTEX_WAIT, timeout is interpreted as a relative value. This differs from other futex operations, where
         // timeout is interpreted as an absolute value. To obtain the equivalent of FUTEX_WAIT with an absolute timeout,
         // employ FUTEX_WAIT_BITSET with val3 specified as FUTEX_BITSET_MATCH_ANY.
-        //
-        // However, in Occlum, we prefer to use relative value for timeout for performance consideration because getting current time
-        // requires an extra ocall.
-        let ts = match futex_op {
-            FutexOp::FUTEX_WAIT => ts,
-            _ => {
-                let relative_ts = {
-                    let absolute_ts = ts.as_duration();
-                    let current_ts = crate::time::do_clock_gettime(clock_id)?.as_duration();
-                    debug_assert!(current_ts <= absolute_ts);
-
-                    timespec_t::from(absolute_ts - current_ts)
-                };
-                relative_ts
+        let absolute_time = {
+            match futex_op {
+                FutexOp::FUTEX_WAIT => false,
+                _ => true,
             }
         };
 
-        Ok(Some(FutexTimeout::new(clock_id, ts)))
+        Ok(Some(FutexTimeout::new(clock_id, ts, absolute_time)))
     };
 
     match futex_op {