rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexandre Courbot <acourbot@nvidia.com>
To: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Bjorn Helgaas" <bhelgaas@google.com>
Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	 linux-pci@vger.kernel.org,
	Alexandre Courbot <acourbot@nvidia.com>
Subject: [PATCH v4 1/2] rust/revocable: add try_access_with() convenience method
Date: Fri, 11 Apr 2025 21:09:38 +0900	[thread overview]
Message-ID: <20250411-try_with-v4-1-f470ac79e2e2@nvidia.com> (raw)
In-Reply-To: <20250411-try_with-v4-0-f470ac79e2e2@nvidia.com>

Revocable::try_access() returns a guard through which the wrapped object
can be accessed. Code that can sleep is not allowed while the guard is
held; thus, it is common for the caller to explicitly drop it before
running sleepable code, e.g:

    let b = bar.try_access()?;
    let reg = b.readl(...);

    // Don't forget this or things could go wrong!
    drop(b);

    something_that_might_sleep();

    let b = bar.try_access()?;
    let reg2 = b.readl(...);

This is arguably error-prone. try_access_with() provides an arguably
safer alternative, by taking a closure that is run while the guard is
held, and by dropping the guard automatically after the closure
completes. This way, code can be organized more clearly around the
critical sections and the risk of forgetting to release the guard when
needed is considerably reduced:

    let reg = bar.try_access_with(|b| b.readl(...))?;

    something_that_might_sleep();

    let reg2 = bar.try_access_with(|b| b.readl(...))?;

The closure can return nothing, or any value including a Result which is
then wrapped inside the Option returned by try_access_with. Error
management is driver-specific, so users are encouraged to create their
own macros that map and flatten the returned values to something
appropriate for the code they are working on.

Acked-by: Danilo Krummrich <dakr@kernel.org>
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 rust/kernel/revocable.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 1e5a9d25c21b279b01f90b02997492aa4880d84f..b91e40e8160be0cc0ff8e0699e48e063c9dbce22 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -123,6 +123,22 @@ pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a
         }
     }
 
+    /// Tries to access the wrapped object and run a closure on it while the guard is held.
+    ///
+    /// This is a convenience method to run short non-sleepable code blocks while ensuring the
+    /// guard is dropped afterwards. [`Self::try_access`] carries the risk that the caller will
+    /// forget to explicitly drop that returned guard before calling sleepable code; this method
+    /// adds an extra safety to make sure it doesn't happen.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible, or the
+    /// result of the closure wrapped in `Some`. If the closure returns a [`Result`] then the
+    /// return type becomes `Option<Result<>>`, which can be inconvenient. Users are encouraged to
+    /// define their own macro that turns the `Option` into a proper error code and flattens the
+    /// inner result into it if it makes sense within their subsystem.
+    pub fn try_access_with<R, F: FnOnce(&T) -> R>(&self, f: F) -> Option<R> {
+        self.try_access().map(|t| f(&*t))
+    }
+
     /// # Safety
     ///
     /// Callers must ensure that there are no more concurrent users of the revocable object.

-- 
2.49.0


  reply	other threads:[~2025-04-11 12:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-11 12:09 [PATCH v4 0/2] rust/revocable: add try_access_with() convenience method Alexandre Courbot
2025-04-11 12:09 ` Alexandre Courbot [this message]
2025-04-15  4:37   ` [v4,1/2] " Joel Fernandes
2025-04-15  4:37   ` Joel Fernandes
2025-04-22 14:23   ` [PATCH v4 1/2] " Miguel Ojeda
2025-04-11 12:09 ` [PATCH v4 2/2] samples: rust: convert PCI rust sample driver to use try_access_with() Alexandre Courbot
2025-04-11 13:15 ` [PATCH v4 0/2] rust/revocable: add try_access_with() convenience method Danilo Krummrich
2025-04-22  8:01   ` Danilo Krummrich
2025-04-22 14:21     ` Miguel Ojeda
2025-04-22 15:27 ` 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=20250411-try_with-v4-1-f470ac79e2e2@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bhelgaas@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=linux-pci@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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).