rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Tiago Lam" <tiagolam@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Will Deacon" <will@kernel.org>,
	"Waiman Long" <longman@redhat.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] rust: sync: add `CondVar::wait_timeout`
Date: Thu, 14 Dec 2023 11:58:38 -0800	[thread overview]
Message-ID: <ZXtebif2S1CyJRik@boqun-archlinux> (raw)
In-Reply-To: <CAH5fLgijsRK3funsGuG6nbK26C+s6m0nO0i83RYD2cO3z7L22Q@mail.gmail.com>

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

[...]

  parent reply	other threads:[~2023-12-14 19:58 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 10:09 [PATCH 0/2] Additional CondVar methods needed by Rust Binder Alice Ryhl
2023-12-06 10:09 ` [PATCH 1/2] rust: sync: add `CondVar::notify_sync` Alice Ryhl
2023-12-06 15:49   ` Martin Rodriguez Reboredo
2023-12-07 20:21   ` Benno Lossin
2023-12-08  7:29     ` Alice Ryhl
2023-12-08  9:30       ` Benno Lossin
2023-12-06 10:09 ` [PATCH 2/2] rust: sync: add `CondVar::wait_timeout` Alice Ryhl
2023-12-06 15:53   ` Martin Rodriguez Reboredo
2023-12-06 16:38     ` Alice Ryhl
2023-12-06 16:30   ` Boqun Feng
2023-12-06 16:39     ` Peter Zijlstra
2023-12-06 16:42       ` Alice Ryhl
2023-12-06 16:53         ` Peter Zijlstra
2023-12-06 17:00           ` Alice Ryhl
2023-12-06 17:05   ` Tiago Lam
2023-12-08  7:37     ` Alice Ryhl
2023-12-08  9:27       ` Benno Lossin
2023-12-12  9:45         ` Alice Ryhl
2023-12-14 19:58       ` Boqun Feng [this message]
2023-12-14 20:04         ` [PATCH] rust: sync: Makes `CondVar::wait()` an uninterruptible wait Boqun Feng
2023-12-15 10:27           ` Alice Ryhl
2023-12-15 23:45             ` Boqun Feng
2023-12-18 17:39               ` Benno Lossin
2023-12-18 20:57                 ` Boqun Feng
2023-12-15 11:58           ` Tiago Lam
2023-12-20 11:11           ` Benno Lossin
2023-12-21 21:43           ` Miguel Ojeda
2023-12-08 19:04   ` [PATCH 2/2] rust: sync: add `CondVar::wait_timeout` Benno Lossin
2023-12-12  9:51     ` Alice Ryhl
2023-12-12 17:05       ` Benno Lossin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZXtebif2S1CyJRik@boqun-archlinux \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tiagolam@gmail.com \
    --cc=wedsonaf@gmail.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).