[PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock

TruongSinh Tran-Nguyen posted 1 patch 2 months, 2 weeks ago
rust/kernel/sync/lock.rs        | 1 +
rust/kernel/sync/lock/global.rs | 1 +
rust/kernel/xarray.rs           | 1 +
3 files changed, 3 insertions(+)
[PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock
Posted by TruongSinh Tran-Nguyen 2 months, 2 weeks ago
These methods return an RAII guard that unlocks the lock when dropped.
If the return value is ignored, the lock is released immediately,
which is likely not the intended behavior.

This addresses issue #1133 in the rust-for-linux project.

Signed-off-by: TruongSinh Tran-Nguyen <i@truongsinh.pro>
---
 rust/kernel/sync/lock.rs        | 1 +
 rust/kernel/sync/lock/global.rs | 1 +
 rust/kernel/xarray.rs           | 1 +
 3 files changed, 3 insertions(+)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index e82fa5be289c..1c2ddade6d6d 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -175,6 +175,7 @@ pub fn lock(&self) -> Guard<'_, T, B> {
     /// Tries to acquire the lock.
     ///
     /// Returns a guard that can be used to access the data protected by the lock if successful.
+    #[must_use = "the lock unlocks immediately when the guard is unused"]
     pub fn try_lock(&self) -> Option<Guard<'_, T, B>> {
         // SAFETY: The constructor of the type calls `init`, so the existence of the object proves
         // that `init` was called.
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index d65f94b5caf2..fdce038b5e0a 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -84,6 +84,7 @@ pub fn lock(&'static self) -> GlobalGuard<B> {
     }
 
     /// Try to lock this global lock.
+    #[must_use = "the lock unlocks immediately when the guard is unused"]
     pub fn try_lock(&'static self) -> Option<GlobalGuard<B>> {
         Some(GlobalGuard {
             inner: self.inner.try_lock()?,
diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
index 75719e7bb491..3cca717a3cb4 100644
--- a/rust/kernel/xarray.rs
+++ b/rust/kernel/xarray.rs
@@ -118,6 +118,7 @@ fn iter(&self) -> impl Iterator<Item = NonNull<T::PointedTo>> + '_ {
     }
 
     /// Attempts to lock the [`XArray`] for exclusive access.
+    #[must_use = "the lock unlocks immediately when the guard is unused"]
     pub fn try_lock(&self) -> Option<Guard<'_, T>> {
         // SAFETY: `self.xa` is always valid by the type invariant.
         if (unsafe { bindings::xa_trylock(self.xa.get()) } != 0) {
-- 
2.43.0
Re: [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock
Posted by Alice Ryhl 2 months, 2 weeks ago
On Fri, Jul 18, 2025 at 4:20 PM TruongSinh Tran-Nguyen <i@truongsinh.pro> wrote:
>
> These methods return an RAII guard that unlocks the lock when dropped.
> If the return value is ignored, the lock is released immediately,
> which is likely not the intended behavior.
>
> This addresses issue #1133 in the rust-for-linux project.
>
> Signed-off-by: TruongSinh Tran-Nguyen <i@truongsinh.pro>

I like the reason string that the previous patch used:
if unused, the lock will be immediately unlocked

Perhaps we could update this to use that wording for the remaining
try_lock methods?

Alice
Re: [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock
Posted by Miguel Ojeda 2 months, 2 weeks ago
On Fri, Jul 18, 2025 at 4:20 PM TruongSinh Tran-Nguyen <i@truongsinh.pro> wrote:
>
> This addresses issue #1133 in the rust-for-linux project.

We typically use the "Suggested-by:" and "Links:" tags for this,
please see e.g. how it was done in the patch linked below.

> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index e82fa5be289c..1c2ddade6d6d 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -175,6 +175,7 @@ pub fn lock(&self) -> Guard<'_, T, B> {
>      /// Tries to acquire the lock.
>      ///
>      /// Returns a guard that can be used to access the data protected by the lock if successful.
> +    #[must_use = "the lock unlocks immediately when the guard is unused"]
>      pub fn try_lock(&self) -> Option<Guard<'_, T, B>> {
>          // SAFETY: The constructor of the type calls `init`, so the existence of the object proves
>          // that `init` was called.

This part was done at:

    https://lore.kernel.org/rust-for-linux/20241212154753.139563-1-dev.json2@gmail.com/

which is soon landing in mainline.

We may want to add the comment on top here like in that patch to the
other 2, and possibly make the "reason string" after the `=` the same
one too.

Thanks for the patch!

Cheers,
Miguel