Re: [PATCH v7 2/2] rust: sync: condvar: use Delta<Jiffy> for timeout and result

[FUJITA Tomonori <tomo@flapping.org>]

On Thu, 01 Oct 2026 01:50:41 +0100
"Gary Guo" <gary@garyguo.net> wrote:

>> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
>> index 5372bfbd93b3..ca944a50c6eb 100644
>> --- a/drivers/android/binder/process.rs
>> +++ b/drivers/android/binder/process.rs
>> @@ -35,6 +35,7 @@
>>          Arc, ArcBorrow, CondVar, CondVarTimeoutResult, SetOnce, SpinLock, UniqueArc,
>>      },
>>      task::{Pid, Task},
>> +    time::Delta,
>>      uaccess::{UserSlice, UserSliceReader},
>>      uapi,
>>      workqueue::{self, Work},
>> @@ -1549,8 +1550,8 @@ 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 jiffies = Delta::from_millis(info.timeout_ms.into()).to_jiffies_timeout();
>> +            while jiffies.as_jiffies() > 0 {
>>                  if inner.outstanding_txns == 0 {
>>                      break;
>>                  }
>> @@ -1567,7 +1568,7 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
>>                          jiffies = remaining;
>>                      }
>>                      CondVarTimeoutResult::Timeout => {
>> -                        jiffies = 0;
>> +                        jiffies = Delta::from_jiffies(0);
> 
> Hmm, we should make `Delta::ZERO` work for jiffies too.

Agreed. I'll add a patch for it in v8.

>> @@ -182,19 +185,29 @@ pub fn wait_interruptible_freezable<T: ?Sized, B: Backend>(
>>      /// Atomically releases the given lock (whose ownership is proven by the guard) and puts the
>>      /// thread to sleep. It wakes up when notified by [`CondVar::notify_one`] or
>>      /// [`CondVar::notify_all`], or when a timeout occurs, or when the thread receives a signal.
>> +    ///
>> +    /// A negative timeout is treated as zero.
>>      #[must_use = "wait_interruptible_timeout returns if a signal is pending, so the caller must check the return value"]
>>      pub fn wait_interruptible_timeout<T: ?Sized, B: Backend>(
>>          &self,
>>          guard: &mut Guard<'_, T, B>,
>> -        jiffies: Jiffies,
>> +        delta: Delta<Jiffy>,
>>      ) -> CondVarTimeoutResult {
>> -        let jiffies = jiffies.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT);
>> -        let res = self.wait_internal(TASK_INTERRUPTIBLE, guard, jiffies);
>> +        let jiffies = delta.as_jiffies();
>> +        let res = self.wait_internal(
>> +            TASK_INTERRUPTIBLE,
>> +            guard,
>> +            jiffies.clamp(0, MAX_SCHEDULE_TIMEOUT),
> 
> This pattern shows up many times..
> 
> Makes me wonder if we want a `Duration` type that is `Delta` but unsigned (value
> range restricted between 0..isize::MAX (or i64::MAX for Nsec).
> 
> Or just have a `as_jiffies_unsigned()` which does the clamp.

I think Delta should stay signed. Once we have Instant for jiffies,
the difference of two instants can be negative, for example a deadline
minus the current jiffies. This is a common pattern in C.

I'll add as_jiffies_unsigned() in v8. It clamps a negative value to 0
and returns unsigned long.