rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] rust: locks: Add `get_mut` method to `Lock`
@ 2024-02-12 14:13 Mathys-Gasnier via B4 Relay
  2024-02-12 14:22 ` Alice Ryhl
  2024-02-15 16:52 ` Wedson Almeida Filho
  0 siblings, 2 replies; 5+ messages in thread
From: Mathys-Gasnier via B4 Relay @ 2024-02-12 14:13 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl
  Cc: rust-for-linux, linux-kernel, Martin Rodriguez Reboredo, Mathys-Gasnier

From: Mathys-Gasnier <mathys35.gasnier@gmail.com>

Having a mutable reference guarantees that no other threads have
access to the lock, so we can take advantage of that to grant callers
access to the protected data without the the cost of acquiring and
releasing the locks. Since the lifetime of the data is tied to the
mutable reference, the borrow checker guarantees that the usage is safe.

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Signed-off-by: Mathys-Gasnier <mathys35.gasnier@gmail.com>
---
Changes in v2:
- Improved doc comment. 
- Link to v1: https://lore.kernel.org/r/20240209-rust-locks-get-mut-v1-1-ce351fc3de47@gmail.com
---
 rust/kernel/sync/lock.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index f12a684bc957..d15af6625d01 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -121,6 +121,13 @@ pub fn lock(&self) -> Guard<'_, T, B> {
         // SAFETY: The lock was just acquired.
         unsafe { Guard::new(self, state) }
     }
+
+    /// Gets the data contained in the lock
+    /// Having a mutable reference to the lock guarantees that no other threads have access to the lock.
+    /// Making it safe to get a mutable reference to the lock content.
+    pub fn get_mut(&mut self) -> &mut T {
+        self.data.get_mut()
+    }
 }
 
 /// A lock guard.

---
base-commit: 711cbfc717650532624ca9f56fbaf191bed56e67
change-id: 20240118-rust-locks-get-mut-c42072101d7a

Best regards,
-- 
Mathys-Gasnier <mathys35.gasnier@gmail.com>


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

* Re: [PATCH v2] rust: locks: Add `get_mut` method to `Lock`
  2024-02-12 14:13 [PATCH v2] rust: locks: Add `get_mut` method to `Lock` Mathys-Gasnier via B4 Relay
@ 2024-02-12 14:22 ` Alice Ryhl
  2024-02-15 16:50   ` Wedson Almeida Filho
  2024-02-15 16:52 ` Wedson Almeida Filho
  1 sibling, 1 reply; 5+ messages in thread
From: Alice Ryhl @ 2024-02-12 14:22 UTC (permalink / raw)
  To: mathys35.gasnier
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	rust-for-linux, linux-kernel, Martin Rodriguez Reboredo

On Mon, Feb 12, 2024 at 3:13 PM Mathys-Gasnier via B4 Relay
<devnull+mathys35.gasnier.gmail.com@kernel.org> wrote:
> +    /// Gets the data contained in the lock
> +    /// Having a mutable reference to the lock guarantees that no other threads have access to the lock.
> +    /// Making it safe to get a mutable reference to the lock content.
> +    pub fn get_mut(&mut self) -> &mut T {
> +        self.data.get_mut()
> +    }

It's impossible to call this method. You can never have a mutable
reference to a Linux mutex because we pin our locks. At most, you can
have a Pin<&mut Self>.

Alice

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

* Re: [PATCH v2] rust: locks: Add `get_mut` method to `Lock`
  2024-02-12 14:22 ` Alice Ryhl
@ 2024-02-15 16:50   ` Wedson Almeida Filho
  2024-02-15 20:55     ` Benno Lossin
  0 siblings, 1 reply; 5+ messages in thread
From: Wedson Almeida Filho @ 2024-02-15 16:50 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: mathys35.gasnier, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	rust-for-linux, linux-kernel, Martin Rodriguez Reboredo

On Mon, 12 Feb 2024 at 11:22, Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Mon, Feb 12, 2024 at 3:13 PM Mathys-Gasnier via B4 Relay
> <devnull+mathys35.gasnier.gmail.com@kernel.org> wrote:
> > +    /// Gets the data contained in the lock
> > +    /// Having a mutable reference to the lock guarantees that no other threads have access to the lock.
> > +    /// Making it safe to get a mutable reference to the lock content.
> > +    pub fn get_mut(&mut self) -> &mut T {
> > +        self.data.get_mut()
> > +    }
>
> It's impossible to call this method. You can never have a mutable
> reference to a Linux mutex because we pin our locks. At most, you can
> have a Pin<&mut Self>.

Perhaps you meant to say that it's impossible to call this method
without unsafe blocks? From a `Pin<&mut T>`, we can call
`get_unchecked_mut` to get an `&mut T`.

This is addressing issue 924 opened by Björn some time back. The idea
here is that if there's a path where avoiding the lock/unlock calls
(which are expensive because of the memory barriers) is performance
critical, we can do it as long as we use an unsafe block.

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

* Re: [PATCH v2] rust: locks: Add `get_mut` method to `Lock`
  2024-02-12 14:13 [PATCH v2] rust: locks: Add `get_mut` method to `Lock` Mathys-Gasnier via B4 Relay
  2024-02-12 14:22 ` Alice Ryhl
@ 2024-02-15 16:52 ` Wedson Almeida Filho
  1 sibling, 0 replies; 5+ messages in thread
From: Wedson Almeida Filho @ 2024-02-15 16:52 UTC (permalink / raw)
  To: mathys35.gasnier
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	rust-for-linux, linux-kernel, Martin Rodriguez Reboredo

On Mon, 12 Feb 2024 at 11:13, Mathys-Gasnier via B4 Relay
<devnull+mathys35.gasnier.gmail.com@kernel.org> wrote:
>
> From: Mathys-Gasnier <mathys35.gasnier@gmail.com>
>
> Having a mutable reference guarantees that no other threads have
> access to the lock, so we can take advantage of that to grant callers
> access to the protected data without the the cost of acquiring and
> releasing the locks. Since the lifetime of the data is tied to the
> mutable reference, the borrow checker guarantees that the usage is safe.
>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> Signed-off-by: Mathys-Gasnier <mathys35.gasnier@gmail.com>

Reviewed-by: Wedson Almeida Filho <walmeida@microsoft.com>

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

* Re: [PATCH v2] rust: locks: Add `get_mut` method to `Lock`
  2024-02-15 16:50   ` Wedson Almeida Filho
@ 2024-02-15 20:55     ` Benno Lossin
  0 siblings, 0 replies; 5+ messages in thread
From: Benno Lossin @ 2024-02-15 20:55 UTC (permalink / raw)
  To: Wedson Almeida Filho, Alice Ryhl
  Cc: mathys35.gasnier, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Andreas Hindborg, rust-for-linux,
	linux-kernel, Martin Rodriguez Reboredo

On 15.02.24 17:50, Wedson Almeida Filho wrote:
> On Mon, 12 Feb 2024 at 11:22, Alice Ryhl <aliceryhl@google.com> wrote:
>>
>> On Mon, Feb 12, 2024 at 3:13 PM Mathys-Gasnier via B4 Relay
>> <devnull+mathys35.gasnier.gmail.com@kernel.org> wrote:
>>> +    /// Gets the data contained in the lock
>>> +    /// Having a mutable reference to the lock guarantees that no other threads have access to the lock.
>>> +    /// Making it safe to get a mutable reference to the lock content.
>>> +    pub fn get_mut(&mut self) -> &mut T {
>>> +        self.data.get_mut()
>>> +    }
>>
>> It's impossible to call this method. You can never have a mutable
>> reference to a Linux mutex because we pin our locks. At most, you can
>> have a Pin<&mut Self>.
> 
> Perhaps you meant to say that it's impossible to call this method
> without unsafe blocks? From a `Pin<&mut T>`, we can call
> `get_unchecked_mut` to get an `&mut T`.

That is the wrong way to design this, since it forces an extra
`unsafe` call. Instead this function's receiver type should be
`self: Pin<&mut Self>`.

-- 
Cheers,
Benno


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

end of thread, other threads:[~2024-02-15 20:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-12 14:13 [PATCH v2] rust: locks: Add `get_mut` method to `Lock` Mathys-Gasnier via B4 Relay
2024-02-12 14:22 ` Alice Ryhl
2024-02-15 16:50   ` Wedson Almeida Filho
2024-02-15 20:55     ` Benno Lossin
2024-02-15 16:52 ` Wedson Almeida Filho

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