Re: [PATCH v11 5/8] rust: time: Add wrapper for fsleep() function

Andreas Hindborg
2025-03-22T21:54:23Z

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

> On Fri, 21 Mar 2025 23:05:23 +0100
> Frederic Weisbecker <frederic@kernel.org> wrote:
>

[...]

>>> +/// `delta` must be within `[0, i32::MAX]` microseconds;
>>> +/// otherwise, it is erroneous behavior. That is, it is considered a bug
>>> +/// to call this function with an out-of-range value, in which case the function
>>> +/// will sleep for at least the maximum value in the range and may warn
>>> +/// in the future.
>>> +///
>>> +/// The behavior above differs from the C side [`fsleep()`] for which out-of-range
>>> +/// values mean "infinite timeout" instead.
>>
>> And very important: the behaviour also differ in that the C side takes
>> usecs while this takes nsecs. We should really disambiguate the situation
>> as that might create confusion or misusage.
>>
>> Either this should be renamed to fsleep_ns() or fsleep_nsecs(), or this should
>> take microseconds directly.
>
> You meant that `Delta` type internally tracks time in nanoseconds?
>
> It's true but Delta type is a unit-agnostic time abstraction, designed
> to represent durations across different granularities — seconds,
> milliseconds, microseconds, nanoseconds. The Rust abstraction always
> tries to us Delta type to represent durations.
>
> Rust's fsleep takes Delta, internally converts it in usecs, and calls
> C's fsleep.
>
> Usually, drivers convert from a certain time unit to Delta before
> calling fsleep like the following, so misuse or confusion is unlikely
> to occur, I think.
>
> fsleep(Delta::from_micros(50));
>
> However, as you pointed out, there is a difference; C's fsleep takes
> usecs while Rust's fsleep takes a unit-agnostic time type. Taking this
> difference into account, if we were to rename fsleep for Rust, I think
> that a name that is agnostic to the time unit would seem more
> appropriate. Simply sleep(), perhaps?

I would prefer to keep the same name. But if we must change it, how
about `flexible_sleep` to indicate the behavior?

And just to reiterate what Tomonori already said, it is not possible to
call this method with an integer,

fsleep(500)

will not compile. A unit based conversion into `Duration` is required.

Best regards,
Andreas Hindborg