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

[Boqun Feng]

On Fri, Dec 08, 2023 at 08:37:27AM +0100, Alice Ryhl wrote:
[...]
> > > +    /// Releases the lock and waits for a notification in interruptible mode.
> > > +    ///
> > > +    /// 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.
> > > +    ///
> > > +    /// Returns whether there is a signal pending.
> > > +    #[must_use = "wait_timeout returns if a signal is pending, so the caller must check the return value"]
> > > +    pub fn wait_timeout<T: ?Sized, B: Backend>(
> > > +        &self,
> > > +        guard: &mut Guard<'_, T, B>,
> > > +        jiffies: u64,
> > > +    ) -> CondVarTimeoutResult {
> >
> > Should this be called `wait_timeout_interruptable` instead, so that if
> > we need to add one using the `TASK_INTERRUPTIBLE` state later we don't
> > need to modfy it again? It also matches the
> > `schedule_timeout_interruptible` one in the kernel (although that's not
> > a reason to change it just in itself).
> 
> I don't mind changing the names, but in this patch I was just
> consistent with what was already there.
> 

Hmm.. so Rust's wait() is actually interruptible wait and we have
wait_uninterruptible(), while C API is wait_event() is uninterruptible,
and we have a wait_event_interruptible(), I think it makes sense we
follow what C API has. Will send a patch soon.

Regards,
Boqun

[...]