Re: [PATCH 2/2] rust: sync: add `CondVar::wait_timeout`

[Peter Zijlstra]

On Wed, Dec 06, 2023 at 05:42:29PM +0100, Alice Ryhl wrote:
> On Wed, Dec 6, 2023 at 5:39 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > On Wed, Dec 06, 2023 at 08:30:06AM -0800, Boqun Feng wrote:
> > > On Wed, Dec 06, 2023 at 10:09:24AM +0000, Alice Ryhl wrote:
> > > [...]
> > > > +
> > > > +/// The return type of `wait_timeout`.
> > > > +pub enum CondVarTimeoutResult {
> > > > +    /// The timeout was reached.
> > > > +    Timeout,
> > > > +    /// Somebody woke us up.
> > > > +    Woken {
> > > > +        /// Remaining sleep duration.
> > > > +        jiffies: u64,
> > >
> > > I have a Jiffies definition in the my upcoming timer patchset:
> > >
> > > /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
> > > pub type Jiffies = core::ffi::c_ulong;
> > >
> > > Maybe you can add that (in a separate patch) in kernel::time?
> >
> > Urgh, why are we using jiffies in 2023?
> 
> I assumed that the correct thing here would be to accept the same unit
> as what schedule_timeout takes. Should I be doing something else?

Bah, so we have schedule_hrtimeout() that takes ktime/u64 nsec. But the
'problem' is that hrtimers are written with the expectation to fire,
while the old timers are written with the expectation to not fire.

Timeouts are typically best done with the latter, so in that regard
using schedule_timeout() is right. But it is sad to inflict the
brain-damage that is jiffies onto new code.

Perhaps add schedule_timeout_*msec() wrappers around schedule_timeout*()
and use a consistent sane time unit?

Thomas?