[PATCH v1 0/2] rust: switch CondVar's timeout API to Delta

[PATCH v1 0/2] rust: switch CondVar's timeout API to Delta

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

CondVar::wait_interruptible_timeout() currently takes and returns a
raw jiffies count (a plain c_ulong alias with no type safety). Callers
have to know on their own that the value meant jiffies and convert
to/from it themselves, which is easy to get wrong (e.g. passing a
millisecond value where a jiffies value is expected).

This series adds jiffies conversion helpers to Delta and then switches
CondVar's timeout API to use Delta instead of raw jiffies, updating
its only caller, binder's ioctl_freeze().

FUJITA Tomonori (2):
  rust: time: add jiffies conversion helpers to Delta
  rust: sync: use Delta for CondVar timeout API

 drivers/android/binder/process.rs | 12 +++++------
 rust/kernel/sync/condvar.rs       | 20 ++++++++++--------
 rust/kernel/time.rs               | 34 +++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 14 deletions(-)


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.43.0

[PATCH v1 1/2] rust: time: add jiffies conversion helpers to Delta

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

Callers that hand a timeout to some C APIs need conversion between
Delta and jiffies.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 363e93cbb139..be31f0486445 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -377,6 +377,15 @@ impl Delta {
     /// A span of time equal to zero.
     pub const ZERO: Self = Self { nanos: 0 };
 
+    /// Create a new [`Delta`] from a number of jiffies.
+    #[inline]
+    pub fn from_jiffies(jiffies: u64) -> Self {
+        // SAFETY: `jiffies64_to_nsecs` is always safe to call no matter what the argument is.
+        let nanos = unsafe { bindings::jiffies64_to_nsecs(jiffies) } as i64;
+
+        Self { nanos }
+    }
+
     /// Create a new [`Delta`] from a number of nanoseconds.
     #[inline]
     pub const fn from_nanos(nanos: i64) -> Self {
@@ -431,6 +440,15 @@ pub fn is_negative(self) -> bool {
         self.as_nanos() < 0
     }
 
+    /// Return the smallest number of jiffies greater than or equal
+    /// to the value in the [`Delta`].
+    ///
+    /// If the value is negative, `0` is used.
+    #[inline]
+    pub fn as_jiffies_ceil(self) -> crate::ffi::c_ulong {
+        msecs_to_jiffies(u32::try_from(self.as_millis_ceil().max(0)).unwrap_or(u32::MAX))
+    }
+
     /// Return the number of nanoseconds in the [`Delta`].
     #[inline]
     pub const fn as_nanos(self) -> i64 {
@@ -468,6 +486,22 @@ pub fn as_millis(self) -> i64 {
         }
     }
 
+    /// Return the smallest number of milliseconds greater than or equal
+    /// to the value in the [`Delta`].
+    #[inline]
+    pub fn as_millis_ceil(self) -> i64 {
+        #[cfg(CONFIG_64BIT)]
+        {
+            self.as_nanos().saturating_add(NSEC_PER_MSEC - 1) / NSEC_PER_MSEC
+        }
+
+        #[cfg(not(CONFIG_64BIT))]
+        // SAFETY: It is always safe to call `ktime_to_ms()` with any value.
+        unsafe {
+            bindings::ktime_to_ms(self.as_nanos().saturating_add(NSEC_PER_MSEC - 1))
+        }
+    }
+
     /// Return `self % dividend` where `dividend` is in nanoseconds.
     ///
     /// The kernel doesn't have any emulation for `s64 % s64` on 32 bit platforms, so this is
-- 
2.43.0

[PATCH v1 2/2] rust: sync: use Delta for CondVar timeout API

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

wait_interruptible_timeout() and CondVarTimeoutResult take/return a
raw jiffies count (a plain c_ulong alias with no type safety). Callers
have to know on their own that the value meant jiffies and convert
to/from it themselves, which is easy to get wrong (e.g. passing a
millisecond value where a jiffies value is expected).

Switch to Delta, the duration type already used elsewhere, so the unit
is part of the type instead of a caller convention.

Use as_jiffies_ceil() rather than a plain truncating conversion: a
caller that passes a nonzero Delta expects the call to actually sleep
for roughly that long (or return earlier via a wakeup or signal).

Update binder's ioctl_freeze(), the only caller, accordingly.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 drivers/android/binder/process.rs | 12 ++++++------
 rust/kernel/sync/condvar.rs       | 20 ++++++++++++--------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 96b8440ceac6..cc693484489a 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -1468,25 +1468,25 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
         inner.is_frozen = IsFrozen::InProgress;
 
         if info.timeout_ms > 0 {
-            let mut jiffies = kernel::time::msecs_to_jiffies(info.timeout_ms);
-            while jiffies > 0 {
+            let mut delta = kernel::time::Delta::from_millis(info.timeout_ms.into());
+            while !delta.is_zero() {
                 if inner.outstanding_txns == 0 {
                     break;
                 }
 
                 match self
                     .freeze_wait
-                    .wait_interruptible_timeout(&mut inner, jiffies)
+                    .wait_interruptible_timeout(&mut inner, delta)
                 {
                     CondVarTimeoutResult::Signal { .. } => {
                         inner.is_frozen = IsFrozen::No;
                         return Err(ERESTARTSYS);
                     }
-                    CondVarTimeoutResult::Woken { jiffies: remaining } => {
-                        jiffies = remaining;
+                    CondVarTimeoutResult::Woken { delta: remaining } => {
+                        delta = remaining;
                     }
                     CondVarTimeoutResult::Timeout => {
-                        jiffies = 0;
+                        delta = kernel::time::Delta::ZERO;
                     }
                 }
             }
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index 69d58dfbad7b..c94ae83a583f 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -12,7 +12,7 @@
     task::{
         MAX_SCHEDULE_TIMEOUT, TASK_FREEZABLE, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE,
     },
-    time::Jiffies,
+    time::Delta,
     types::Opaque,
 };
 use core::{marker::PhantomPinned, pin::Pin, ptr};
@@ -186,15 +186,19 @@ pub fn wait_interruptible_freezable<T: ?Sized, B: Backend>(
     pub fn wait_interruptible_timeout<T: ?Sized, B: Backend>(
         &self,
         guard: &mut Guard<'_, T, B>,
-        jiffies: Jiffies,
+        delta: Delta,
     ) -> CondVarTimeoutResult {
-        let jiffies = jiffies.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT);
+        let jiffies = c_long::try_from(delta.as_jiffies_ceil()).unwrap_or(MAX_SCHEDULE_TIMEOUT);
         let res = self.wait_internal(TASK_INTERRUPTIBLE, guard, jiffies);
 
-        match (res as Jiffies, crate::current!().signal_pending()) {
-            (jiffies, true) => CondVarTimeoutResult::Signal { jiffies },
+        match (res, crate::current!().signal_pending()) {
+            (jiffies, true) => CondVarTimeoutResult::Signal {
+                delta: Delta::from_jiffies(jiffies as u64),
+            },
             (0, false) => CondVarTimeoutResult::Timeout,
-            (jiffies, false) => CondVarTimeoutResult::Woken { jiffies },
+            (jiffies, false) => CondVarTimeoutResult::Woken {
+                delta: Delta::from_jiffies(jiffies as u64),
+            },
         }
     }
 
@@ -248,11 +252,11 @@ pub enum CondVarTimeoutResult {
     /// Somebody woke us up.
     Woken {
         /// Remaining sleep duration.
-        jiffies: Jiffies,
+        delta: Delta,
     },
     /// A signal occurred.
     Signal {
         /// Remaining sleep duration.
-        jiffies: Jiffies,
+        delta: Delta,
     },
 }
-- 
2.43.0