[PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[FUJITA Tomonori]

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

write_all() and wait_until_sent() take the timeout as a raw Jiffies,
which is a c_ulong alias. The type does not show the unit.

Take the timeout as Delta<Jiffy> instead. Clamp a negative timeout to
0, as the current code does for a value larger than isize::MAX.

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

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 17ca504b7f8d..094503f059ff 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -20,7 +20,10 @@
         aref::AlwaysRefCounted,
         Mutex, //
     },
-    time::Jiffies,
+    time::{
+        Delta,
+        Jiffy, //
+    },
     types::{
         Opaque,
         ScopeGuard, //
@@ -481,22 +484,19 @@ pub fn set_parity(&self, parity: Parity) -> Result {
     /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
     /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
     #[inline]
-    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
+    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
         if data.len() > i32::MAX as usize {
             return Err(EINVAL);
         }
 
+        let timeout = isize::max(timeout.as_jiffies(), 0);
+
         // SAFETY:
         // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
         // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
         //   `data.len()`.
         let ret = unsafe {
-            bindings::serdev_device_write(
-                self.as_raw(),
-                data.as_ptr(),
-                data.len(),
-                isize::try_from(timeout).unwrap_or_default(),
-            )
+            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
         };
         // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
         // which always fit into a `i32`.
@@ -547,14 +547,11 @@ pub fn write_flush(&self) {
     ///
     /// Use a timeout of 0 to wait indefinitely.
     #[inline]
-    pub fn wait_until_sent(&self, timeout: Jiffies) {
+    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
+        let timeout = isize::max(timeout.as_jiffies(), 0);
+
         // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
-        unsafe {
-            bindings::serdev_device_wait_until_sent(
-                self.as_raw(),
-                isize::try_from(timeout).unwrap_or_default(),
-            )
-        };
+        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
     }
 }
 

base-commit: f1850e443b0e4f2429ddf42a8d5033ea54ae8a90
-- 
2.43.0

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rust: serdev: negative timeout clamped to 0 causes infinite wait
--

commit 68348e8f0bd03a5788e4d72cf264605070a06317
Author: FUJITA Tomonori <fujita.tomonori@gmail.com>

rust: serdev: use Delta<Jiffy> for timeouts

write_all() and wait_until_sent() take the timeout as a raw Jiffies,
which is a c_ulong alias. The type does not show the unit.

> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d6..094503f059ff5 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs

[ ... ]

> @@ -481,22 +484,19 @@ pub fn set_parity(&self, parity: Parity) -> Result {
>      /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
>      /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
>      #[inline]
> -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
> +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
>          if data.len() > i32::MAX as usize {
>              return Err(EINVAL);
>          }
>  
> +        let timeout = isize::max(timeout.as_jiffies(), 0);

[Severity: High]
Does this cause an unintended infinite wait when a negative timeout is passed?

When a driver calculates a deadline that is already in the past, it will
produce a negative Delta<Jiffy>, which this clamps to 0.

However, the underlying C function serdev_device_write() treats a timeout of
0 as an infinite wait:

drivers/tty/serdev/core.c:serdev_device_write() {
    ...
	if (timeout == 0)
		timeout = MAX_SCHEDULE_TIMEOUT;
    ...
}

Should negative timeouts instead return immediately or fail with -ETIMEDOUT
rather than causing the thread to hang indefinitely?

> +
>          // SAFETY:
>          // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
>          // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
>          //   `data.len()`.
>          let ret = unsafe {
> -            bindings::serdev_device_write(
> -                self.as_raw(),
> -                data.as_ptr(),
> -                data.len(),
> -                isize::try_from(timeout).unwrap_or_default(),
> -            )
> +            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
>          };

[ ... ]

> @@ -547,14 +547,11 @@ pub fn write_flush(&self) {
>      ///
>      /// Use a timeout of 0 to wait indefinitely.
>      #[inline]
> -    pub fn wait_until_sent(&self, timeout: Jiffies) {
> +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
> +        let timeout = isize::max(timeout.as_jiffies(), 0);

[Severity: High]
Can this also cause an unintended infinite wait?

Similar to write_all(), if wait_until_sent() is called with a negative timeout,
it is clamped to 0.

The underlying C implementation tty_wait_until_sent() treats a timeout of 0
as an infinite wait:

drivers/tty/tty_ioctl.c:tty_wait_until_sent() {
    ...
	if (!timeout)
		timeout = MAX_SCHEDULE_TIMEOUT;
    ...
}

Would it be safer to handle expired deadlines without waiting indefinitely?

> +
>          // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> -        unsafe {
> -            bindings::serdev_device_wait_until_sent(
> -                self.as_raw(),
> -                isize::try_from(timeout).unwrap_or_default(),
> -            )
> -        };
> +        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
>      }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930041339.1551129-1-tomo@flapping.org?part=1

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 3601 bytes --]

On Wed, 2026-09-30 at 13:13 +0900, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> write_all() and wait_until_sent() take the timeout as a raw Jiffies,
> which is a c_ulong alias. The type does not show the unit.
> 
> Take the timeout as Delta<Jiffy> instead. Clamp a negative timeout to
> 0, as the current code does for a value larger than isize::MAX.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/serdev.rs | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d..094503f059ff 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
> @@ -20,7 +20,10 @@
>          aref::AlwaysRefCounted,
>          Mutex, //
>      },
> -    time::Jiffies,
> +    time::{
> +        Delta,
> +        Jiffy, //
> +    },
>      types::{
>          Opaque,
>          ScopeGuard, //
> @@ -481,22 +484,19 @@ pub fn set_parity(&self, parity: Parity) -> Result {
>      /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
>      /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
>      #[inline]
> -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
> +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
>          if data.len() > i32::MAX as usize {
>              return Err(EINVAL);
>          }
>  
> +        let timeout = isize::max(timeout.as_jiffies(), 0);
> +
>          // SAFETY:
>          // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
>          // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
>          //   `data.len()`.
>          let ret = unsafe {
> -            bindings::serdev_device_write(
> -                self.as_raw(),
> -                data.as_ptr(),
> -                data.len(),
> -                isize::try_from(timeout).unwrap_or_default(),
> -            )
> +            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
>          };
>          // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
>          // which always fit into a `i32`.
> @@ -547,14 +547,11 @@ pub fn write_flush(&self) {
>      ///
>      /// Use a timeout of 0 to wait indefinitely.
>      #[inline]
> -    pub fn wait_until_sent(&self, timeout: Jiffies) {
> +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
> +        let timeout = isize::max(timeout.as_jiffies(), 0);
> +
>          // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> -        unsafe {
> -            bindings::serdev_device_wait_until_sent(
> -                self.as_raw(),
> -                isize::try_from(timeout).unwrap_or_default(),
> -            )
> -        };
> +        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
>      }
>  }
>  
> 
> base-commit: f1850e443b0e4f2429ddf42a8d5033ea54ae8a90

Both functions have "Use a timeout of 0 to wait indefinitely." inside
the rustdoc.

Make sure `Delta::ZERO` is also usable for `Delta<Jiffy>` and replace
the 0 in the rustdoc with "[`Delta::<Jiffy>::ZERO`]". You might also
remove the "timeout of" part, but I don't mind if it stays.

With that fixed,

Reviewed-By: Markus Probst <markus.probst@posteo.de>

Thanks
- Markus Probst






[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[Gary Guo]

On Wed Sep 30, 2026 at 1:59 PM BST, Markus Probst wrote:
> On Wed, 2026-09-30 at 13:13 +0900, FUJITA Tomonori wrote:
>> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> 
>> write_all() and wait_until_sent() take the timeout as a raw Jiffies,
>> which is a c_ulong alias. The type does not show the unit.
>> 
>> Take the timeout as Delta<Jiffy> instead. Clamp a negative timeout to
>> 0, as the current code does for a value larger than isize::MAX.
>> 
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
>>  rust/kernel/serdev.rs | 27 ++++++++++++---------------
>>  1 file changed, 12 insertions(+), 15 deletions(-)
>> 
>> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
>> index 17ca504b7f8d..094503f059ff 100644
>> --- a/rust/kernel/serdev.rs
>> +++ b/rust/kernel/serdev.rs
>> @@ -20,7 +20,10 @@
>>          aref::AlwaysRefCounted,
>>          Mutex, //
>>      },
>> -    time::Jiffies,
>> +    time::{
>> +        Delta,
>> +        Jiffy, //
>> +    },
>>      types::{
>>          Opaque,
>>          ScopeGuard, //
>> @@ -481,22 +484,19 @@ pub fn set_parity(&self, parity: Parity) -> Result {
>>      /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
>>      /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
>>      #[inline]
>> -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
>> +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
>>          if data.len() > i32::MAX as usize {
>>              return Err(EINVAL);
>>          }
>>  
>> +        let timeout = isize::max(timeout.as_jiffies(), 0);
>> +
>>          // SAFETY:
>>          // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
>>          // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
>>          //   `data.len()`.
>>          let ret = unsafe {
>> -            bindings::serdev_device_write(
>> -                self.as_raw(),
>> -                data.as_ptr(),
>> -                data.len(),
>> -                isize::try_from(timeout).unwrap_or_default(),
>> -            )
>> +            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
>>          };
>>          // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
>>          // which always fit into a `i32`.
>> @@ -547,14 +547,11 @@ pub fn write_flush(&self) {
>>      ///
>>      /// Use a timeout of 0 to wait indefinitely.
>>      #[inline]
>> -    pub fn wait_until_sent(&self, timeout: Jiffies) {
>> +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
>> +        let timeout = isize::max(timeout.as_jiffies(), 0);
>> +
>>          // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
>> -        unsafe {
>> -            bindings::serdev_device_wait_until_sent(
>> -                self.as_raw(),
>> -                isize::try_from(timeout).unwrap_or_default(),
>> -            )
>> -        };
>> +        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
>>      }
>>  }
>>  
>> 
>> base-commit: f1850e443b0e4f2429ddf42a8d5033ea54ae8a90
>
> Both functions have "Use a timeout of 0 to wait indefinitely." inside
> the rustdoc.
>
> Make sure `Delta::ZERO` is also usable for `Delta<Jiffy>` and replace
> the 0 in the rustdoc with "[`Delta::<Jiffy>::ZERO`]". You might also
> remove the "timeout of" part, but I don't mind if it stays.

I think the API should ideally use `Option<Delta<Jiffies>>` for this case, and
use `None` to represent indefinite wait.

We might need to round 0 jiffies to 1 to avoid C API change. But ideally the C
API should be using MAX_JIFFY_OFFSET to mean indefinite..

Best,
Gary

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 4492 bytes --]

On Wed, 2026-09-30 at 14:52 +0100, Gary Guo wrote:
> On Wed Sep 30, 2026 at 1:59 PM BST, Markus Probst wrote:
> > On Wed, 2026-09-30 at 13:13 +0900, FUJITA Tomonori wrote:
> > > From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > > 
> > > write_all() and wait_until_sent() take the timeout as a raw Jiffies,
> > > which is a c_ulong alias. The type does not show the unit.
> > > 
> > > Take the timeout as Delta<Jiffy> instead. Clamp a negative timeout to
> > > 0, as the current code does for a value larger than isize::MAX.
> > > 
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > > ---
> > >  rust/kernel/serdev.rs | 27 ++++++++++++---------------
> > >  1 file changed, 12 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > > index 17ca504b7f8d..094503f059ff 100644
> > > --- a/rust/kernel/serdev.rs
> > > +++ b/rust/kernel/serdev.rs
> > > @@ -20,7 +20,10 @@
> > >          aref::AlwaysRefCounted,
> > >          Mutex, //
> > >      },
> > > -    time::Jiffies,
> > > +    time::{
> > > +        Delta,
> > > +        Jiffy, //
> > > +    },
> > >      types::{
> > >          Opaque,
> > >          ScopeGuard, //
> > > @@ -481,22 +484,19 @@ pub fn set_parity(&self, parity: Parity) -> Result {
> > >      /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> > >      /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
> > >      #[inline]
> > > -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
> > > +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
> > >          if data.len() > i32::MAX as usize {
> > >              return Err(EINVAL);
> > >          }
> > >  
> > > +        let timeout = isize::max(timeout.as_jiffies(), 0);
> > > +
> > >          // SAFETY:
> > >          // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > >          // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
> > >          //   `data.len()`.
> > >          let ret = unsafe {
> > > -            bindings::serdev_device_write(
> > > -                self.as_raw(),
> > > -                data.as_ptr(),
> > > -                data.len(),
> > > -                isize::try_from(timeout).unwrap_or_default(),
> > > -            )
> > > +            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
> > >          };
> > >          // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> > >          // which always fit into a `i32`.
> > > @@ -547,14 +547,11 @@ pub fn write_flush(&self) {
> > >      ///
> > >      /// Use a timeout of 0 to wait indefinitely.
> > >      #[inline]
> > > -    pub fn wait_until_sent(&self, timeout: Jiffies) {
> > > +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
> > > +        let timeout = isize::max(timeout.as_jiffies(), 0);
> > > +
> > >          // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > > -        unsafe {
> > > -            bindings::serdev_device_wait_until_sent(
> > > -                self.as_raw(),
> > > -                isize::try_from(timeout).unwrap_or_default(),
> > > -            )
> > > -        };
> > > +        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
> > >      }
> > >  }
> > >  
> > > 
> > > base-commit: f1850e443b0e4f2429ddf42a8d5033ea54ae8a90
> > 
> > Both functions have "Use a timeout of 0 to wait indefinitely." inside
> > the rustdoc.
> > 
> > Make sure `Delta::ZERO` is also usable for `Delta<Jiffy>` and replace
> > the 0 in the rustdoc with "[`Delta::<Jiffy>::ZERO`]". You might also
> > remove the "timeout of" part, but I don't mind if it stays.
> 
> I think the API should ideally use `Option<Delta<Jiffies>>` for this case, and
> use `None` to represent indefinite wait.
Yes, this would make it more explicit.
> 
> We might need to round 0 jiffies to 1 to avoid C API change. But ideally the C
> API should be using MAX_JIFFY_OFFSET to mean indefinite..
In both cases MAX_SCHEDULE_TIMEOUT is also accepted (even if not
documented).

At least both pass through such call:

if (timeout == 0)
        timeout = MAX_SCHEDULE_TIMEOUT;

Thanks
- Markus Probst

> 
> Best,
> Gary


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[FUJITA Tomonori]

On Wed, 30 Sep 2026 14:52:50 +0100
"Gary Guo" <gary@garyguo.net> wrote:

>>> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
>>> index 17ca504b7f8d..094503f059ff 100644
>>> --- a/rust/kernel/serdev.rs
>>> +++ b/rust/kernel/serdev.rs

(snip)

>>>      #[inline]
>>> -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
>>> +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
>>>          if data.len() > i32::MAX as usize {
>>>              return Err(EINVAL);
>>>          }
>>>  
>>> +        let timeout = isize::max(timeout.as_jiffies(), 0);
>>> +

(snip)

>>>      /// Use a timeout of 0 to wait indefinitely.
>>>      #[inline]
>>> -    pub fn wait_until_sent(&self, timeout: Jiffies) {
>>> +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
>>> +        let timeout = isize::max(timeout.as_jiffies(), 0);
>>> +
>>>          // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
>>> -        unsafe {
>>> -            bindings::serdev_device_wait_until_sent(
>>> -                self.as_raw(),
>>> -                isize::try_from(timeout).unwrap_or_default(),
>>> -            )
>>> -        };
>>> +        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
>>>      }
>>>  }
>>>  
>>> 
>>> base-commit: f1850e443b0e4f2429ddf42a8d5033ea54ae8a90
>>
>> Both functions have "Use a timeout of 0 to wait indefinitely." inside
>> the rustdoc.
>>
>> Make sure `Delta::ZERO` is also usable for `Delta<Jiffy>` and replace
>> the 0 in the rustdoc with "[`Delta::<Jiffy>::ZERO`]". You might also
>> remove the "timeout of" part, but I don't mind if it stays.
> 
> I think the API should ideally use `Option<Delta<Jiffies>>` for this case, and
> use `None` to represent indefinite wait.

For read_poll_timeout(), where a timeout of 0 means "never time out"
in C, we did not take Option for the timeout. Alice's comment [1]:

| Another thing is the `timeout_delta` option. I would just have
| written it as two methods, one that takes a timeout and one that
| doesn't. That way, callers that don't need a timeout do not need to
| handle timeout errors. (Do we have any users without a timeout? If
| not, maybe just remove the Option.)

serdev is in the same situation. Do we want an API that uses None for
"never" here?

[1] https://lore.kernel.org/rust-for-linux/aJm9A_D-zlJtbV6X@google.com/

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[Gary Guo]

On Thu Oct 1, 2026 at 1:29 AM BST, FUJITA Tomonori wrote:
> On Wed, 30 Sep 2026 14:52:50 +0100
> "Gary Guo" <gary@garyguo.net> wrote:
>
>> I think the API should ideally use `Option<Delta<Jiffies>>` for this case, and
>> use `None` to represent indefinite wait.
>
> For read_poll_timeout(), where a timeout of 0 means "never time out"
> in C, we did not take Option for the timeout. Alice's comment [1]:
>
> | Another thing is the `timeout_delta` option. I would just have
> | written it as two methods, one that takes a timeout and one that
> | doesn't. That way, callers that don't need a timeout do not need to
> | handle timeout errors. (Do we have any users without a timeout? If
> | not, maybe just remove the Option.)
>
> serdev is in the same situation. Do we want an API that uses None for
> "never" here?

I don't like the "using 0" to mean forever, because 0 has a more sensible
meaning -- which is just do a poll and don't wait.

This one doesn't even return a result at all, so `Option` sounds reasonable.
Alternatively, just use `MAX_SCHEDULE_TIMEOUT` (i.e.
`Delta::from_jiffies(isize::MAX)`). We can perhaps add a `Delta::MAX` for this.

Best,
Gary

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[FUJITA Tomonori]

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

> On Thu Oct 1, 2026 at 1:29 AM BST, FUJITA Tomonori wrote:
>> On Wed, 30 Sep 2026 14:52:50 +0100
>> "Gary Guo" <gary@garyguo.net> wrote:
>>
>>> I think the API should ideally use `Option<Delta<Jiffies>>` for this case, and
>>> use `None` to represent indefinite wait.
>>
>> For read_poll_timeout(), where a timeout of 0 means "never time out"
>> in C, we did not take Option for the timeout. Alice's comment [1]:
>>
>> | Another thing is the `timeout_delta` option. I would just have
>> | written it as two methods, one that takes a timeout and one that
>> | doesn't. That way, callers that don't need a timeout do not need to
>> | handle timeout errors. (Do we have any users without a timeout? If
>> | not, maybe just remove the Option.)
>>
>> serdev is in the same situation. Do we want an API that uses None for
>> "never" here?
> 
> I don't like the "using 0" to mean forever, because 0 has a more sensible
> meaning -- which is just do a poll and don't wait.

Fully agreed.

> This one doesn't even return a result at all, so `Option` sounds reasonable.

That's true for wait_until_sent(), but write_all() returns a result
and it can be ETIMEDOUT.

> Alternatively, just use `MAX_SCHEDULE_TIMEOUT` (i.e.
> `Delta::from_jiffies(isize::MAX)`). We can perhaps add a `Delta::MAX` for this.

I don't think Delta::<Jiffy>::MAX is a good idea. Whether the maximum
value means an indefinite wait depends on the function. For example,
it doesn't for Queue::enqueue_delayed().

Another option is to take Delta<Jiffy> and round 0 up to 1 jiffy. A
driver that needs an indefinite wait passes
Delta::from_jiffies(MAX_SCHEDULE_TIMEOUT) explicitly.

Markus, what do you think?

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[Gary Guo]

On Thu Oct 1, 2026 at 2:28 AM BST, FUJITA Tomonori wrote:
> On Thu, 01 Oct 2026 01:39:51 +0100
> "Gary Guo" <gary@garyguo.net> wrote:
>
>> On Thu Oct 1, 2026 at 1:29 AM BST, FUJITA Tomonori wrote:
>>> On Wed, 30 Sep 2026 14:52:50 +0100
>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>
>>>> I think the API should ideally use `Option<Delta<Jiffies>>` for this case, and
>>>> use `None` to represent indefinite wait.
>>>
>>> For read_poll_timeout(), where a timeout of 0 means "never time out"
>>> in C, we did not take Option for the timeout. Alice's comment [1]:
>>>
>>> | Another thing is the `timeout_delta` option. I would just have
>>> | written it as two methods, one that takes a timeout and one that
>>> | doesn't. That way, callers that don't need a timeout do not need to
>>> | handle timeout errors. (Do we have any users without a timeout? If
>>> | not, maybe just remove the Option.)
>>>
>>> serdev is in the same situation. Do we want an API that uses None for
>>> "never" here?
>> 
>> I don't like the "using 0" to mean forever, because 0 has a more sensible
>> meaning -- which is just do a poll and don't wait.
>
> Fully agreed.
>
>> This one doesn't even return a result at all, so `Option` sounds reasonable.
>
> That's true for wait_until_sent(), but write_all() returns a result
> and it can be ETIMEDOUT.
>
>> Alternatively, just use `MAX_SCHEDULE_TIMEOUT` (i.e.
>> `Delta::from_jiffies(isize::MAX)`). We can perhaps add a `Delta::MAX` for this.
>
> I don't think Delta::<Jiffy>::MAX is a good idea. Whether the maximum
> value means an indefinite wait depends on the function. For example,
> it doesn't for Queue::enqueue_delayed().

Whether it means infinite or not depends on the function, but that's still mean
"MAX", no?

Also, it's not like serdev_device_wait_until_sent use it to mean infinite
either. I randomly sampled two wait_until_sent impls and they just use the
timeout in arithmetic, so it's just a very long wait.

Best,
Gary

Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

[FUJITA Tomonori]

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

> On Thu Oct 1, 2026 at 2:28 AM BST, FUJITA Tomonori wrote:

[...]

>>> Alternatively, just use `MAX_SCHEDULE_TIMEOUT` (i.e.
>>> `Delta::from_jiffies(isize::MAX)`). We can perhaps add a `Delta::MAX` for this.
>>
>> I don't think Delta::<Jiffy>::MAX is a good idea. Whether the maximum
>> value means an indefinite wait depends on the function. For example,
>> it doesn't for Queue::enqueue_delayed().
> 
> Whether it means infinite or not depends on the function, but that's still mean
> "MAX", no?

If I understand you correctly, a Rust function that takes Delta<Jiffy>
turns Delta::<Jiffy>::MAX into the maximum value of the C function it
calls, and documents what MAX means.

isize::MAX is MAX_SCHEDULE_TIMEOUT, so schedule_timeout() and the
functions built on it need nothing. Other functions might need a
clamp, because C avoids LONG_MAX as a jiffies span:

| /*
|  * Change timeval to jiffies, trying to avoid the
|  * most obvious overflows..
|  *
|  * And some not so obvious.
|  *
|  * Note that we don't want to return LONG_MAX, because
|  * for various timeout reasons we often end up having
|  * to wait "jiffies+1" in order to guarantee that we wait
|  * at _least_ "jiffies" - so "jiffies+1" had better still
|  * be positive.
|  */
| #define MAX_JIFFY_OFFSET ((LONG_MAX >> 1)-1)

With that, I'm fine with Delta::<Jiffy>::MAX.