linux-kernel - Re: [PATCH 2/2] rust: sync: add `CondVar::wait

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 14 Dec 2023 11:58:38 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Tiago Lam <tiagolam@...il.com>, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...sung.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
	Waiman Long <longman@...hat.com>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] rust: sync: add `CondVar::wait_timeout`

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

[...]