Message-Id: <20250313-try_with-v1-1-adcae7ed98a9@nvidia.com>
Date: Thu, 13 Mar 2025 21:40:59 +0900
From: Alexandre Courbot <acourbot@...dia.com>
To: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
 Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
 Danilo Krummrich <dakr@...nel.org>, 
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
 Benno Lossin <benno.lossin@...ton.me>, 
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
 Trevor Gross <tmgross@...ch.edu>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Alexandre Courbot <acourbot@...dia.com>
Subject: [PATCH] rust/revocable: add try_with() convenience method

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 that the caller will explicitly need to 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_with() and try_with_ok() 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 is forgetting to release the guard when
needed is considerably reduced:

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

    something_that_might_sleep();

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

Unlike try_access() which returns an Option, try_with() and
try_with_ok() return Err(ENXIO) if the object cannot be acquired. The
Option returned by try_access() is typically converted to an error in
practice, so this saves one step for the caller.

try_with() requires the callback itself to return a Result that is
passed to the caller. try_with_ok() accepts a callback that never fails.

Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
---
This is a feature I found useful to have while writing Nova driver code
that accessed registers alongside other operations. I would find myself
quite confused about whether the guard was held or dropped at a given
point of the code, and it felt like walking through a minefield ; this
pattern makes things safer and easier to read IMHO.
---
 rust/kernel/revocable.rs | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 1e5a9d25c21b279b01f90b02997492aa4880d84f..0157b20373b5b2892cb618b46958bfe095e428b6 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -105,6 +105,28 @@ pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
         }
     }
 
+    /// Tries to access the wrapped object and run the closure `f` on it with the guard 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 `Err(ENXIO)` if the wrapped object has been revoked, or the result of `f` after it
+    /// has been run.
+    pub fn try_with<R, F: Fn(&T) -> Result<R>>(&self, f: F) -> Result<R> {
+        self.try_access().ok_or(ENXIO).and_then(|t| f(&*t))
+    }
+
+    /// Tries to access the wrapped object and run the closure `f` on it with the guard held.
+    ///
+    /// This is the same as [`Self::try_with`], with the exception that `f` is expected to
+    /// always succeed and thus does not need to return a `Result`. Thus the only error case is if
+    /// the wrapped object has been revoked.
+    pub fn try_with_ok<R, F: Fn(&T) -> R>(&self, f: F) -> Result<R> {
+        self.try_with(|t| Ok(f(t)))
+    }
+
     /// Tries to access the revocable wrapped object.
     ///
     /// Returns `None` if the object has been revoked and is therefore no longer accessible.

---
base-commit: 4d872d51bc9d7b899c1f61534e3dbde72613f627
change-id: 20250313-try_with-cc9f91dd3b60

Best regards,
-- 
Alexandre Courbot <acourbot@...dia.com>

Powered by blists - more mailing lists