mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: boqun@kernel.org, gary@garyguo.net, aliceryhl@google.com,
	lyude@redhat.com, daniel.almeida@collabora.com,
	work@onurozkan.dev, ojeda@kernel.org, bjorn3_gh@protonmail.com,
	lossin@kernel.org, a.hindborg@kernel.org, tmgross@umich.edu,
	dakr@kernel.org, tamird@kernel.org, acourbot@nvidia.com,
	viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz
Cc: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: [PATCH v2 3/5] rust: sync: convert CondVar and PollCondVar to use WaitQueue
Date: Thu,  5 Nov 2026 01:59:40 +0100	[thread overview]
Message-ID: <20261105005951.756157-4-dakr@kernel.org> (raw)
In-Reply-To: <20261105005951.756157-1-dakr@kernel.org>

CondVar currently wraps a raw wait_queue_head directly. Now that
WaitQueue provides the same primitive, convert CondVar to use it
instead. This removes duplicate init, pinning, Send/Sync, and wake-up
code from CondVar.

Keep the wait logic in CondVar since it needs to release and reacquire a
lock guard around the sleep, which WaitQueue's condition-based
wait_event() methods do not handle. Use wait_once_exclusive() to perform
a single prepare_to_wait_exclusive / finish_wait cycle with a
caller-provided schedule function to implement the lock-aware sleep.

Adapt PollCondVar accordingly.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/sync/condvar.rs | 89 +++++++++++--------------------------
 rust/kernel/sync/poll.rs    | 19 ++++----
 rust/kernel/sync/wait.rs    |  2 -
 3 files changed, 33 insertions(+), 77 deletions(-)

diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index 69d58dfbad7b..78a5f10d68fa 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -5,18 +5,26 @@
 //! This module allows Rust code to use the kernel's [`struct wait_queue_head`] as a condition
 //! variable.
 
-use super::{lock::Backend, lock::Guard, LockClassKey};
+use super::{
+    lock::Backend,
+    lock::Guard,
+    LockClassKey,
+    WaitQueue, //
+};
+
 use crate::{
-    ffi::{c_int, c_long},
-    str::{CStr, CStrExt as _},
+    prelude::*,
+    str::CStr,
     task::{
-        MAX_SCHEDULE_TIMEOUT, TASK_FREEZABLE, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE,
+        MAX_SCHEDULE_TIMEOUT,
+        TASK_FREEZABLE,
+        TASK_INTERRUPTIBLE,
+        TASK_UNINTERRUPTIBLE, //
     },
     time::Jiffies,
-    types::Opaque,
 };
-use core::{marker::PhantomPinned, pin::Pin, ptr};
-use pin_init::{pin_data, pin_init, PinInit};
+
+use core::pin::Pin;
 
 /// Creates a [`CondVar`] initialiser with the given name and a newly-created lock class.
 #[macro_export]
@@ -81,33 +89,14 @@ macro_rules! new_condvar {
 #[pin_data]
 pub struct CondVar {
     #[pin]
-    pub(crate) wait_queue_head: Opaque<bindings::wait_queue_head>,
-
-    /// A condvar needs to be pinned because it contains a [`struct list_head`] that is
-    /// self-referential, so it cannot be safely moved once it is initialised.
-    ///
-    /// [`struct list_head`]: srctree/include/linux/types.h
-    #[pin]
-    _pin: PhantomPinned,
+    pub(crate) wq: WaitQueue,
 }
 
-// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on any thread.
-unsafe impl Send for CondVar {}
-
-// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on multiple threads
-// concurrently.
-unsafe impl Sync for CondVar {}
-
 impl CondVar {
     /// Constructs a new condvar initialiser.
     pub fn new(name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self> {
         pin_init!(Self {
-            _pin: PhantomPinned,
-            // SAFETY: `slot` is valid while the closure is called and both `name` and `key` have
-            // static lifetimes so they live indefinitely.
-            wait_queue_head <- Opaque::ffi_init(|slot| unsafe {
-                bindings::__init_waitqueue_head(slot, name.as_char_ptr(), key.as_ptr())
-            }),
+            wq <- WaitQueue::new(name, key),
         })
     }
 
@@ -117,23 +106,10 @@ fn wait_internal<T: ?Sized, B: Backend>(
         guard: &mut Guard<'_, T, B>,
         timeout_in_jiffies: c_long,
     ) -> c_long {
-        let wait = Opaque::<bindings::wait_queue_entry>::uninit();
-
-        // SAFETY: `wait` points to valid memory.
-        unsafe { bindings::init_wait(wait.get()) };
-
-        // SAFETY: Both `wait` and `wait_queue_head` point to valid memory.
-        unsafe {
-            bindings::prepare_to_wait_exclusive(self.wait_queue_head.get(), wait.get(), wait_state)
-        };
-
-        // SAFETY: Switches to another thread. The timeout can be any number.
-        let ret = guard.do_unlocked(|| unsafe { bindings::schedule_timeout(timeout_in_jiffies) });
-
-        // SAFETY: Both `wait` and `wait_queue_head` point to valid memory.
-        unsafe { bindings::finish_wait(self.wait_queue_head.get(), wait.get()) };
-
-        ret
+        self.wq.wait_once_exclusive(wait_state, || {
+            // SAFETY: Switches to another thread. The timeout can be any number.
+            guard.do_unlocked(|| unsafe { bindings::schedule_timeout(timeout_in_jiffies) })
+        })
     }
 
     /// Releases the lock and waits for a notification in uninterruptible mode.
@@ -198,28 +174,13 @@ pub fn wait_interruptible_timeout<T: ?Sized, B: Backend>(
         }
     }
 
-    /// Calls the kernel function to notify the appropriate number of threads.
-    fn notify(&self, count: c_int) {
-        // SAFETY: `wait_queue_head` points to valid memory.
-        unsafe {
-            bindings::__wake_up(
-                self.wait_queue_head.get(),
-                TASK_NORMAL,
-                count,
-                ptr::null_mut(),
-            )
-        };
-    }
-
     /// Calls the kernel function to notify one thread synchronously.
     ///
     /// This method behaves like `notify_one`, except that it hints to the scheduler that the
-    /// current thread is about to go to sleep, so it should schedule the target thread on the same
-    /// CPU.
+    /// current thread expects to sleep soon.
     #[inline]
     pub fn notify_sync(&self) {
-        // SAFETY: `wait_queue_head` points to valid memory.
-        unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), TASK_NORMAL) };
+        self.wq.wake_up_sync();
     }
 
     /// Wakes a single waiter up, if any.
@@ -228,7 +189,7 @@ pub fn notify_sync(&self) {
     /// completely (as opposed to automatically waking up the next waiter).
     #[inline]
     pub fn notify_one(&self) {
-        self.notify(1);
+        self.wq.wake_up();
     }
 
     /// Wakes all waiters up, if any.
@@ -237,7 +198,7 @@ pub fn notify_one(&self) {
     /// completely (as opposed to automatically waking up the next waiter).
     #[inline]
     pub fn notify_all(&self) {
-        self.notify(0);
+        self.wq.wake_up_all();
     }
 }
 
diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index dc40bfaa57e6..fbd754a0bc54 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -64,11 +64,11 @@ pub fn register_wait(&self, file: &File, cv: &PollCondVar) {
         // * `file.as_ptr()` references a valid file for the duration of this call.
         // * `self.table` is null or references a valid poll_table for the duration of this call.
         // * Since `PollCondVar` is pinned, its destructor is guaranteed to run before the memory
-        //   containing `cv.wait_queue_head` is invalidated. Since the destructor clears all
-        //   waiters and then waits for an rcu grace period, it's guaranteed that
-        //   `cv.wait_queue_head` remains valid for at least an rcu grace period after the removal
-        //   of the last waiter.
-        unsafe { bindings::poll_wait(file.as_ptr(), cv.wait_queue_head.get(), self.table) }
+        //   containing the wait queue head is invalidated. Since the destructor clears all
+        //   waiters and then waits for an rcu grace period, it's guaranteed that the wait queue
+        //   head remains valid for at least an rcu grace period after the removal of the last
+        //   waiter.
+        unsafe { bindings::poll_wait(file.as_ptr(), cv.wq.as_raw(), self.table) }
     }
 }
 
@@ -105,9 +105,7 @@ impl PinnedDrop for PollCondVar {
     #[inline]
     fn drop(self: Pin<&mut Self>) {
         // Clear anything registered using `register_wait`.
-        //
-        // SAFETY: The pointer points at a valid `wait_queue_head`.
-        unsafe { bindings::__wake_up_pollfree(self.inner.wait_queue_head.get()) };
+        self.inner.wq.wake_up_pollfree();
 
         // Wait for epoll items to be properly removed.
         synchronize_rcu();
@@ -165,14 +163,13 @@ fn drop(&mut self) {
         // SAFETY: ManuallyDrop::take ok because not already taken.
         let boxed = unsafe { ManuallyDrop::take(&mut self.inner) };
 
+        boxed.inner.inner.wq.wake_up_pollfree();
+
         // SAFETY: The code below frees the box without calling the actual destructor of the type,
         // but it's okay because it re-implements the destructor using `kfree_rcu()` in place of
         // `synchronize_rcu()`.
         let ptr = KBox::into_raw(unsafe { Pin::into_inner_unchecked(boxed) });
 
-        // SAFETY: The pointer points at a valid `wait_queue_head`.
-        unsafe { bindings::__wake_up_pollfree((*ptr).inner.inner.wait_queue_head.get()) };
-
         // SAFETY: This was allocated using `KBox::pin_init`, so it can be freed with `kvfree`.
         unsafe { bindings::kvfree_call_rcu((*ptr).rcu.get(), ptr.cast::<ffi::c_void>()) };
     }
diff --git a/rust/kernel/sync/wait.rs b/rust/kernel/sync/wait.rs
index 00120dbb4fca..92c1ae93705e 100644
--- a/rust/kernel/sync/wait.rs
+++ b/rust/kernel/sync/wait.rs
@@ -219,7 +219,6 @@ pub(super) fn wait_once<F, R>(&self, wait_state: c_int, schedule_fn: F) -> R
     /// Performs a single exclusive prepare-to-wait / finish-wait cycle, calling `schedule_fn`
     /// in between.
     #[inline]
-    #[expect(unused)]
     pub(super) fn wait_once_exclusive<F, R>(&self, wait_state: c_int, schedule_fn: F) -> R
     where
         F: FnOnce() -> R,
@@ -303,7 +302,6 @@ pub fn wake_up_sync(&self) {
     /// Used when a wait queue is about to be freed, to ensure epoll items are properly removed.
     /// Matches C's `wake_up_pollfree()`.
     #[inline]
-    #[expect(unused)]
     pub(super) fn wake_up_pollfree(&self) {
         // SAFETY: `wait_queue_head` points to valid memory.
         unsafe { bindings::__wake_up_pollfree(self.wait_queue_head.get()) };
-- 
2.56.0


  parent reply	other threads:[~2026-11-05  1:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05  0:59 [PATCH v2 0/5] rust: sync: add WaitQueue infrastructure Danilo Krummrich
2026-11-05  0:59 ` [PATCH v2 1/5] rust: task: add safe schedule_timeout() wrapper Danilo Krummrich
2026-11-05  4:20   ` FUJITA Tomonori
2026-11-05  0:59 ` [PATCH v2 2/5] rust: sync: add WaitQueue infrastructure Danilo Krummrich
2026-11-05  0:59 ` Danilo Krummrich [this message]
2026-11-05  0:59 ` [PATCH v2 4/5] rust: sync: condvar: use task::schedule_timeout() Danilo Krummrich
2026-11-05  0:59 ` [PATCH v2 5/5] rust: sync: condvar: add non-exclusive freezable waiting Danilo Krummrich

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=20261105005951.756157-4-dakr@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=brauner@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=viro@zeniv.linux.org.uk \
    --cc=work@onurozkan.dev \
    /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

all inboxes | Powered by JetHome®