rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lyude Paul <lyude@redhat.com>
To: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	Benno Lossin <lossin@kernel.org>
Cc: "Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Will Deacon" <will@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Waiman Long" <longman@redhat.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>
Subject: [PATCH v4] rust: lock: Export Guard::do_unlocked()
Date: Wed, 29 Oct 2025 14:35:38 -0400	[thread overview]
Message-ID: <20251029183538.226257-1-lyude@redhat.com> (raw)

In RVKMS, I discovered a silly issue where as a result of our HrTimer for
vblank emulation and our vblank enable/disable callbacks sharing a
spinlock, it was possible to deadlock while trying to disable the vblank
timer.

The solution for this ended up being simple: keep track of when the HrTimer
could potentially acquire the shared spinlock, and simply drop the spinlock
temporarily from our vblank enable/disable callbacks when stopping the
timer. And do_unlocked() ended up being perfect for this.

Since this seems like it's useful, let's export this for use by the rest of
the world and write short documentation for it.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Fix documentation for do_unlocked
* Add an example
V3:
* Documentation changes from Miguel
V4:
* Improve the example to actually demonstrate a situation where
  do_unlocked() would be useful.
* Remove unneeded sentence above example in do_unlocked()

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/sync/lock.rs | 71 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 5d7991e6d3736..c5f049a115d09 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -230,7 +230,76 @@ pub fn lock_ref(&self) -> &'a Lock<T, B> {
         self.lock
     }
 
-    pub(crate) fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
+    /// Releases this [`Guard`]'s lock temporarily, executes `cb` and then re-acquires it.
+    ///
+    /// This can be useful for situations where you may need to do a temporary unlock dance to avoid
+    /// issues like circular locking dependencies.
+    ///
+    /// It returns the value returned by the closure.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::{
+    /// #     new_mutex,
+    /// #     sync::{lock::{Backend, Guard, Lock}, Arc, Mutex, Completion},
+    /// #     workqueue::{self, impl_has_work, new_work, Work, WorkItem},
+    /// # };
+    /// #[pin_data]
+    /// struct ExampleWork {
+    ///     #[pin]
+    ///     work: Work<Self>,
+    ///
+    ///     #[pin]
+    ///     lock: Mutex<i32>,
+    ///
+    ///     #[pin]
+    ///     done: Completion,
+    /// }
+    ///
+    /// impl_has_work! {
+    ///     impl HasWork<Self> for ExampleWork { self.work }
+    /// }
+    ///
+    /// impl WorkItem for ExampleWork {
+    ///     type Pointer = Arc<ExampleWork>;
+    ///
+    ///     fn run(this: Arc<ExampleWork>) {
+    ///         let mut g = this.lock.lock();
+    ///
+    ///         assert_eq!(*g, 41);
+    ///         *g += 1;
+    ///
+    ///         this.done.complete_all();
+    ///     }
+    /// }
+    ///
+    /// impl ExampleWork {
+    ///     pub(crate) fn new() -> Result<Arc<Self>> {
+    ///         Arc::pin_init(pin_init!(Self {
+    ///             work <- new_work!(),
+    ///             lock <- new_mutex!(41),
+    ///             done <- Completion::new(),
+    ///         }), GFP_KERNEL)
+    ///     }
+    /// }
+    ///
+    /// let work = ExampleWork::new().unwrap();
+    /// let mut g = work.lock.lock();
+    ///
+    /// let _ = workqueue::system().enqueue(work.clone());
+    ///
+    /// // This would deadlock:
+    /// //
+    /// //     work.done.wait_for_completion()
+    /// //
+    /// // Since we hold work.lock, which work will also try to acquire in WorkItem::run. Dropping
+    /// // the lock temporarily while we wait for completion works around this.
+    /// g.do_unlocked(|| work.done.wait_for_completion());
+    ///
+    /// assert_eq!(*g, 42);
+    /// ```
+    pub fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
         // SAFETY: The caller owns the lock, so it is safe to unlock it.
         unsafe { B::unlock(self.lock.state.get(), &self.state) };
 

base-commit: 3b83f5d5e78ac5cddd811a5e431af73959864390
-- 
2.51.0


             reply	other threads:[~2025-10-29 18:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-29 18:35 Lyude Paul [this message]
2025-10-30 10:43 ` [PATCH v4] rust: lock: Export Guard::do_unlocked() Paolo Bonzini
2025-10-30 17:41   ` Lyude Paul
2025-10-31  9:31     ` Alice Ryhl
2025-10-31  9:38       ` Paolo Bonzini
2025-10-31 10:24         ` Alice Ryhl
2025-11-05 20:41           ` Lyude Paul

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=20251029183538.226257-1-lyude@redhat.com \
    --to=lyude@redhat.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=lossin@kernel.org \
    --cc=mingo@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --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).