linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock
@ 2025-07-18 14:20 TruongSinh Tran-Nguyen
  2025-07-18 17:59 ` Miguel Ojeda
  2025-07-21 11:06 ` Alice Ryhl
  0 siblings, 2 replies; 3+ messages in thread
From: TruongSinh Tran-Nguyen @ 2025-07-18 14:20 UTC (permalink / raw)
  To: rust-for-linux
  Cc: linux-kernel, ojeda, alex.gaynor, gary, bjorn3_gh, lossin,
	a.hindborg, aliceryhl, tmgross, dakr, TruongSinh Tran-Nguyen

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock
  2025-07-18 14:20 [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock TruongSinh Tran-Nguyen
@ 2025-07-18 17:59 ` Miguel Ojeda
  2025-07-21 11:06 ` Alice Ryhl
  1 sibling, 0 replies; 3+ messages in thread
From: Miguel Ojeda @ 2025-07-18 17:59 UTC (permalink / raw)
  To: TruongSinh Tran-Nguyen, Jason Devers
  Cc: rust-for-linux, linux-kernel, ojeda, alex.gaynor, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, dakr

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock
  2025-07-18 14:20 [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock TruongSinh Tran-Nguyen
  2025-07-18 17:59 ` Miguel Ojeda
@ 2025-07-21 11:06 ` Alice Ryhl
  1 sibling, 0 replies; 3+ messages in thread
From: Alice Ryhl @ 2025-07-21 11:06 UTC (permalink / raw)
  To: TruongSinh Tran-Nguyen
  Cc: rust-for-linux, linux-kernel, ojeda, alex.gaynor, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, dakr

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-21 11:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-18 14:20 [PATCH] rust: Add #[must_use] to Lock::try_lock, GlobalLock::try_lock, and XArray::try_lock TruongSinh Tran-Nguyen
2025-07-18 17:59 ` Miguel Ojeda
2025-07-21 11:06 ` Alice Ryhl

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).