linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/5] rust: types: introduce `ScopeGuard`
@ 2023-01-30  6:44 Wedson Almeida Filho
  2023-01-30  6:44 ` [PATCH v2 2/5] rust: types: introduce `ForeignOwnable` Wedson Almeida Filho
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Wedson Almeida Filho @ 2023-01-30  6:44 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Wedson Almeida Filho,
	Vincenzo Palazzo

This allows us to run some code when the guard is dropped (e.g.,
implicitly when it goes out of scope). We can also prevent the
guard from running by calling its `dismiss()` method.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
---
v1 -> v2: Simpler type for `ScopeGuard::new()` impl block

 rust/kernel/types.rs | 126 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index e84e51ec9716..dd834bfcb57b 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -2,7 +2,131 @@
 
 //! Kernel types.
 
-use core::{cell::UnsafeCell, mem::MaybeUninit};
+use core::{
+    cell::UnsafeCell,
+    mem::MaybeUninit,
+    ops::{Deref, DerefMut},
+};
+
+/// Runs a cleanup function/closure when dropped.
+///
+/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
+///
+/// # Examples
+///
+/// In the example below, we have multiple exit paths and we want to log regardless of which one is
+/// taken:
+/// ```
+/// # use kernel::ScopeGuard;
+/// fn example1(arg: bool) {
+///     let _log = ScopeGuard::new(|| pr_info!("example1 completed\n"));
+///
+///     if arg {
+///         return;
+///     }
+///
+///     pr_info!("Do something...\n");
+/// }
+///
+/// # example1(false);
+/// # example1(true);
+/// ```
+///
+/// In the example below, we want to log the same message on all early exits but a different one on
+/// the main exit path:
+/// ```
+/// # use kernel::ScopeGuard;
+/// fn example2(arg: bool) {
+///     let log = ScopeGuard::new(|| pr_info!("example2 returned early\n"));
+///
+///     if arg {
+///         return;
+///     }
+///
+///     // (Other early returns...)
+///
+///     log.dismiss();
+///     pr_info!("example2 no early return\n");
+/// }
+///
+/// # example2(false);
+/// # example2(true);
+/// ```
+///
+/// In the example below, we need a mutable object (the vector) to be accessible within the log
+/// function, so we wrap it in the [`ScopeGuard`]:
+/// ```
+/// # use kernel::ScopeGuard;
+/// fn example3(arg: bool) -> Result {
+///     let mut vec =
+///         ScopeGuard::new_with_data(Vec::new(), |v| pr_info!("vec had {} elements\n", v.len()));
+///
+///     vec.try_push(10u8)?;
+///     if arg {
+///         return Ok(());
+///     }
+///     vec.try_push(20u8)?;
+///     Ok(())
+/// }
+///
+/// # assert_eq!(example3(false), Ok(()));
+/// # assert_eq!(example3(true), Ok(()));
+/// ```
+///
+/// # Invariants
+///
+/// The value stored in the struct is nearly always `Some(_)`, except between
+/// [`ScopeGuard::dismiss`] and [`ScopeGuard::drop`]: in this case, it will be `None` as the value
+/// will have been returned to the caller. Since  [`ScopeGuard::dismiss`] consumes the guard,
+/// callers won't be able to use it anymore.
+pub struct ScopeGuard<T, F: FnOnce(T)>(Option<(T, F)>);
+
+impl<T, F: FnOnce(T)> ScopeGuard<T, F> {
+    /// Creates a new guarded object wrapping the given data and with the given cleanup function.
+    pub fn new_with_data(data: T, cleanup_func: F) -> Self {
+        // INVARIANT: The struct is being initialised with `Some(_)`.
+        Self(Some((data, cleanup_func)))
+    }
+
+    /// Prevents the cleanup function from running and returns the guarded data.
+    pub fn dismiss(mut self) -> T {
+        // INVARIANT: This is the exception case in the invariant; it is not visible to callers
+        // because this function consumes `self`.
+        self.0.take().unwrap().0
+    }
+}
+
+impl ScopeGuard<(), fn(())> {
+    /// Creates a new guarded object with the given cleanup function.
+    pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
+        ScopeGuard::new_with_data((), move |_| cleanup())
+    }
+}
+
+impl<T, F: FnOnce(T)> Deref for ScopeGuard<T, F> {
+    type Target = T;
+
+    fn deref(&self) -> &T {
+        // The type invariants guarantee that `unwrap` will succeed.
+        &self.0.as_ref().unwrap().0
+    }
+}
+
+impl<T, F: FnOnce(T)> DerefMut for ScopeGuard<T, F> {
+    fn deref_mut(&mut self) -> &mut T {
+        // The type invariants guarantee that `unwrap` will succeed.
+        &mut self.0.as_mut().unwrap().0
+    }
+}
+
+impl<T, F: FnOnce(T)> Drop for ScopeGuard<T, F> {
+    fn drop(&mut self) {
+        // Run the cleanup function if one is still present.
+        if let Some((data, cleanup)) = self.0.take() {
+            cleanup(data)
+        }
+    }
+}
 
 /// Stores an opaque value.
 ///
-- 
2.34.1


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

* [PATCH v2 2/5] rust: types: introduce `ForeignOwnable`
  2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
@ 2023-01-30  6:44 ` Wedson Almeida Filho
  2023-01-30 18:49   ` Gary Guo
  2023-02-01  9:35   ` Andreas Hindborg
  2023-01-30  6:44 ` [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>` Wedson Almeida Filho
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: Wedson Almeida Filho @ 2023-01-30  6:44 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Wedson Almeida Filho,
	Vincenzo Palazzo, Martin Rodriguez Reboredo

It was originally called `PointerWrapper`. It is used to convert
a Rust object to a pointer representation (void *) that can be
stored on the C side, used, and eventually returned to Rust.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
---
v1 -> v2: Use `Self` instead of generic type in `borrow_mut`

 rust/kernel/lib.rs   |  1 +
 rust/kernel/types.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index e0b0e953907d..223564f9f0cc 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -16,6 +16,7 @@
 #![feature(coerce_unsized)]
 #![feature(core_ffi_c)]
 #![feature(dispatch_from_dyn)]
+#![feature(generic_associated_types)]
 #![feature(receiver_trait)]
 #![feature(unsize)]
 
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index dd834bfcb57b..72710b7442a3 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -8,6 +8,60 @@ use core::{
     ops::{Deref, DerefMut},
 };
 
+/// Used to transfer ownership to and from foreign (non-Rust) languages.
+///
+/// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
+/// later may be transferred back to Rust by calling [`Self::from_foreign`].
+///
+/// This trait is meant to be used in cases when Rust objects are stored in C objects and
+/// eventually "freed" back to Rust.
+pub trait ForeignOwnable: Sized {
+    /// Type of values borrowed between calls to [`ForeignOwnable::into_foreign`] and
+    /// [`ForeignOwnable::from_foreign`].
+    type Borrowed<'a>;
+
+    /// Converts a Rust-owned object to a foreign-owned one.
+    ///
+    /// The foreign representation is a pointer to void.
+    fn into_foreign(self) -> *const core::ffi::c_void;
+
+    /// Borrows a foreign-owned object.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
+    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
+    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow_mut`]
+    /// for this object must have been dropped.
+    unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>;
+
+    /// Mutably borrows a foreign-owned object.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
+    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
+    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
+    /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
+    unsafe fn borrow_mut(ptr: *const core::ffi::c_void) -> ScopeGuard<Self, fn(Self)> {
+        // SAFETY: The safety requirements ensure that `ptr` came from a previous call to
+        // `into_foreign`.
+        ScopeGuard::new_with_data(unsafe { Self::from_foreign(ptr) }, |d| {
+            d.into_foreign();
+        })
+    }
+
+    /// Converts a foreign-owned object back to a Rust-owned one.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
+    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
+    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
+    /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
+    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
+}
+
 /// Runs a cleanup function/closure when dropped.
 ///
 /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
-- 
2.34.1


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

* [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>`
  2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
  2023-01-30  6:44 ` [PATCH v2 2/5] rust: types: introduce `ForeignOwnable` Wedson Almeida Filho
@ 2023-01-30  6:44 ` Wedson Almeida Filho
  2023-02-01  9:56   ` Andreas Hindborg
  2023-01-30  6:44 ` [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type Wedson Almeida Filho
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Wedson Almeida Filho @ 2023-01-30  6:44 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Wedson Almeida Filho,
	Vincenzo Palazzo, Alice Ferrazzi

This allows us to hand ownership of Rust dynamically allocated
objects to the C side of the kernel.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Reviewed-by: Alice Ferrazzi <alice.ferrazzi@miraclelinux.com>
---
v1 -> v2: Add `use alloc::boxed::Box`, which wasn't needed before

 rust/kernel/types.rs | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 72710b7442a3..411655eca3e9 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -2,6 +2,7 @@
 
 //! Kernel types.
 
+use alloc::boxed::Box;
 use core::{
     cell::UnsafeCell,
     mem::MaybeUninit,
@@ -62,6 +63,28 @@ pub trait ForeignOwnable: Sized {
     unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
 }
 
+impl<T: 'static> ForeignOwnable for Box<T> {
+    type Borrowed<'a> = &'a T;
+
+    fn into_foreign(self) -> *const core::ffi::c_void {
+        Box::into_raw(self) as _
+    }
+
+    unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T {
+        // SAFETY: The safety requirements for this function ensure that the object is still alive,
+        // so it is safe to dereference the raw pointer.
+        // The safety requirements of `from_foreign` also ensure that the object remains alive for
+        // the lifetime of the returned value.
+        unsafe { &*ptr.cast() }
+    }
+
+    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
+        // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
+        // call to `Self::into_foreign`.
+        unsafe { Box::from_raw(ptr as _) }
+    }
+}
+
 /// Runs a cleanup function/closure when dropped.
 ///
 /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
-- 
2.34.1


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

* [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type
  2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
  2023-01-30  6:44 ` [PATCH v2 2/5] rust: types: introduce `ForeignOwnable` Wedson Almeida Filho
  2023-01-30  6:44 ` [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>` Wedson Almeida Filho
@ 2023-01-30  6:44 ` Wedson Almeida Filho
  2023-01-30 18:41   ` Gary Guo
  2023-02-01  9:58   ` Andreas Hindborg
  2023-01-30  6:44 ` [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>` Wedson Almeida Filho
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: Wedson Almeida Filho @ 2023-01-30  6:44 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Wedson Almeida Filho,
	Vincenzo Palazzo

This allows us to use the unit type `()` when we have no object whose
ownership must be managed but one implementing the `ForeignOwnable`
trait is needed.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
---
v1 -> v2: use `NonNull::dangling` to generate pointer

 rust/kernel/types.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 411655eca3e9..9d0fdbc55843 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -85,6 +85,18 @@ impl<T: 'static> ForeignOwnable for Box<T> {
     }
 }
 
+impl ForeignOwnable for () {
+    type Borrowed<'a> = ();
+
+    fn into_foreign(self) -> *const core::ffi::c_void {
+        core::ptr::NonNull::dangling().as_ptr()
+    }
+
+    unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
+
+    unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
+}
+
 /// Runs a cleanup function/closure when dropped.
 ///
 /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
-- 
2.34.1


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

* [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`
  2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
                   ` (2 preceding siblings ...)
  2023-01-30  6:44 ` [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type Wedson Almeida Filho
@ 2023-01-30  6:44 ` Wedson Almeida Filho
  2023-02-01 10:17   ` Andreas Hindborg
  2023-01-30 18:50 ` [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Gary Guo
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Wedson Almeida Filho @ 2023-01-30  6:44 UTC (permalink / raw)
  To: rust-for-linux
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Wedson Almeida Filho,
	Vincenzo Palazzo, Alice Ferrazzi

This allows us to hand ownership of Rust ref-counted objects to
the C side of the kernel.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Reviewed-by: Alice Ferrazzi <alice.ferrazzi@miraclelinux.com>
---
v1 -> v2: Unmodified

 rust/kernel/sync/arc.rs | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index ff73f9240ca1..519a6ec43644 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -15,7 +15,11 @@
 //!
 //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
 
-use crate::{bindings, error::Result, types::Opaque};
+use crate::{
+    bindings,
+    error::Result,
+    types::{ForeignOwnable, Opaque},
+};
 use alloc::boxed::Box;
 use core::{
     marker::{PhantomData, Unsize},
@@ -189,6 +193,32 @@ impl<T: ?Sized> Arc<T> {
     }
 }
 
+impl<T: 'static> ForeignOwnable for Arc<T> {
+    type Borrowed<'a> = ArcBorrow<'a, T>;
+
+    fn into_foreign(self) -> *const core::ffi::c_void {
+        ManuallyDrop::new(self).ptr.as_ptr() as _
+    }
+
+    unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
+        // SAFETY: By the safety requirement of this function, we know that `ptr` came from
+        // a previous call to `Arc::into_foreign`.
+        let inner = NonNull::new(ptr as *mut ArcInner<T>).unwrap();
+
+        // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive
+        // for the lifetime of the returned value. Additionally, the safety requirements of
+        // `ForeignOwnable::borrow_mut` ensure that no new mutable references are created.
+        unsafe { ArcBorrow::new(inner) }
+    }
+
+    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
+        // SAFETY: By the safety requirement of this function, we know that `ptr` came from
+        // a previous call to `Arc::into_foreign`, which owned guarantees that `ptr` is valid and
+        // owns a reference.
+        unsafe { Self::from_inner(NonNull::new(ptr as _).unwrap()) }
+    }
+}
+
 impl<T: ?Sized> Deref for Arc<T> {
     type Target = T;
 
-- 
2.34.1


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

* Re: [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type
  2023-01-30  6:44 ` [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type Wedson Almeida Filho
@ 2023-01-30 18:41   ` Gary Guo
  2023-02-01  9:58   ` Andreas Hindborg
  1 sibling, 0 replies; 17+ messages in thread
From: Gary Guo @ 2023-01-30 18:41 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo

On Mon, 30 Jan 2023 03:44:03 -0300
Wedson Almeida Filho <wedsonaf@gmail.com> wrote:

> This allows us to use the unit type `()` when we have no object whose
> ownership must be managed but one implementing the `ForeignOwnable`
> trait is needed.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> v1 -> v2: use `NonNull::dangling` to generate pointer
> 
>  rust/kernel/types.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 411655eca3e9..9d0fdbc55843 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -85,6 +85,18 @@ impl<T: 'static> ForeignOwnable for Box<T> {
>      }
>  }
>  
> +impl ForeignOwnable for () {
> +    type Borrowed<'a> = ();
> +
> +    fn into_foreign(self) -> *const core::ffi::c_void {
> +        core::ptr::NonNull::dangling().as_ptr()
> +    }
> +
> +    unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
> +
> +    unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
> +}
> +
>  /// Runs a cleanup function/closure when dropped.
>  ///
>  /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.


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

* Re: [PATCH v2 2/5] rust: types: introduce `ForeignOwnable`
  2023-01-30  6:44 ` [PATCH v2 2/5] rust: types: introduce `ForeignOwnable` Wedson Almeida Filho
@ 2023-01-30 18:49   ` Gary Guo
  2023-02-01  9:35   ` Andreas Hindborg
  1 sibling, 0 replies; 17+ messages in thread
From: Gary Guo @ 2023-01-30 18:49 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo,
	Martin Rodriguez Reboredo

On Mon, 30 Jan 2023 03:44:01 -0300
Wedson Almeida Filho <wedsonaf@gmail.com> wrote:

> It was originally called `PointerWrapper`. It is used to convert
> a Rust object to a pointer representation (void *) that can be
> stored on the C side, used, and eventually returned to Rust.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> v1 -> v2: Use `Self` instead of generic type in `borrow_mut`
> 
>  rust/kernel/lib.rs   |  1 +
>  rust/kernel/types.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index e0b0e953907d..223564f9f0cc 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -16,6 +16,7 @@
>  #![feature(coerce_unsized)]
>  #![feature(core_ffi_c)]
>  #![feature(dispatch_from_dyn)]
> +#![feature(generic_associated_types)]
>  #![feature(receiver_trait)]
>  #![feature(unsize)]
>  
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index dd834bfcb57b..72710b7442a3 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -8,6 +8,60 @@ use core::{
>      ops::{Deref, DerefMut},
>  };
>  
> +/// Used to transfer ownership to and from foreign (non-Rust) languages.
> +///
> +/// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> +/// later may be transferred back to Rust by calling [`Self::from_foreign`].
> +///
> +/// This trait is meant to be used in cases when Rust objects are stored in C objects and
> +/// eventually "freed" back to Rust.
> +pub trait ForeignOwnable: Sized {
> +    /// Type of values borrowed between calls to [`ForeignOwnable::into_foreign`] and
> +    /// [`ForeignOwnable::from_foreign`].
> +    type Borrowed<'a>;
> +
> +    /// Converts a Rust-owned object to a foreign-owned one.
> +    ///
> +    /// The foreign representation is a pointer to void.
> +    fn into_foreign(self) -> *const core::ffi::c_void;
> +
> +    /// Borrows a foreign-owned object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
> +    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
> +    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow_mut`]
> +    /// for this object must have been dropped.
> +    unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>;
> +
> +    /// Mutably borrows a foreign-owned object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
> +    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
> +    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
> +    /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
> +    unsafe fn borrow_mut(ptr: *const core::ffi::c_void) -> ScopeGuard<Self, fn(Self)> {
> +        // SAFETY: The safety requirements ensure that `ptr` came from a previous call to
> +        // `into_foreign`.
> +        ScopeGuard::new_with_data(unsafe { Self::from_foreign(ptr) }, |d| {
> +            d.into_foreign();
> +        })
> +    }
> +
> +    /// Converts a foreign-owned object back to a Rust-owned one.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
> +    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
> +    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
> +    /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
> +    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
> +}
> +
>  /// Runs a cleanup function/closure when dropped.
>  ///
>  /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.


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

* Re: [PATCH v2 1/5] rust: types: introduce `ScopeGuard`
  2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
                   ` (3 preceding siblings ...)
  2023-01-30  6:44 ` [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>` Wedson Almeida Filho
@ 2023-01-30 18:50 ` Gary Guo
  2023-01-30 20:07 ` Andreas Hindborg
  2023-02-01  1:00 ` Miguel Ojeda
  6 siblings, 0 replies; 17+ messages in thread
From: Gary Guo @ 2023-01-30 18:50 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo

On Mon, 30 Jan 2023 03:44:00 -0300
Wedson Almeida Filho <wedsonaf@gmail.com> wrote:

> This allows us to run some code when the guard is dropped (e.g.,
> implicitly when it goes out of scope). We can also prevent the
> guard from running by calling its `dismiss()` method.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> v1 -> v2: Simpler type for `ScopeGuard::new()` impl block
> 
>  rust/kernel/types.rs | 126 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 125 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index e84e51ec9716..dd834bfcb57b 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -2,7 +2,131 @@
>  
>  //! Kernel types.
>  
> -use core::{cell::UnsafeCell, mem::MaybeUninit};
> +use core::{
> +    cell::UnsafeCell,
> +    mem::MaybeUninit,
> +    ops::{Deref, DerefMut},
> +};
> +
> +/// Runs a cleanup function/closure when dropped.
> +///
> +/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
> +///
> +/// # Examples
> +///
> +/// In the example below, we have multiple exit paths and we want to log regardless of which one is
> +/// taken:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example1(arg: bool) {
> +///     let _log = ScopeGuard::new(|| pr_info!("example1 completed\n"));
> +///
> +///     if arg {
> +///         return;
> +///     }
> +///
> +///     pr_info!("Do something...\n");
> +/// }
> +///
> +/// # example1(false);
> +/// # example1(true);
> +/// ```
> +///
> +/// In the example below, we want to log the same message on all early exits but a different one on
> +/// the main exit path:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example2(arg: bool) {
> +///     let log = ScopeGuard::new(|| pr_info!("example2 returned early\n"));
> +///
> +///     if arg {
> +///         return;
> +///     }
> +///
> +///     // (Other early returns...)
> +///
> +///     log.dismiss();
> +///     pr_info!("example2 no early return\n");
> +/// }
> +///
> +/// # example2(false);
> +/// # example2(true);
> +/// ```
> +///
> +/// In the example below, we need a mutable object (the vector) to be accessible within the log
> +/// function, so we wrap it in the [`ScopeGuard`]:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example3(arg: bool) -> Result {
> +///     let mut vec =
> +///         ScopeGuard::new_with_data(Vec::new(), |v| pr_info!("vec had {} elements\n", v.len()));
> +///
> +///     vec.try_push(10u8)?;
> +///     if arg {
> +///         return Ok(());
> +///     }
> +///     vec.try_push(20u8)?;
> +///     Ok(())
> +/// }
> +///
> +/// # assert_eq!(example3(false), Ok(()));
> +/// # assert_eq!(example3(true), Ok(()));
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// The value stored in the struct is nearly always `Some(_)`, except between
> +/// [`ScopeGuard::dismiss`] and [`ScopeGuard::drop`]: in this case, it will be `None` as the value
> +/// will have been returned to the caller. Since  [`ScopeGuard::dismiss`] consumes the guard,
> +/// callers won't be able to use it anymore.
> +pub struct ScopeGuard<T, F: FnOnce(T)>(Option<(T, F)>);
> +
> +impl<T, F: FnOnce(T)> ScopeGuard<T, F> {
> +    /// Creates a new guarded object wrapping the given data and with the given cleanup function.
> +    pub fn new_with_data(data: T, cleanup_func: F) -> Self {
> +        // INVARIANT: The struct is being initialised with `Some(_)`.
> +        Self(Some((data, cleanup_func)))
> +    }
> +
> +    /// Prevents the cleanup function from running and returns the guarded data.
> +    pub fn dismiss(mut self) -> T {
> +        // INVARIANT: This is the exception case in the invariant; it is not visible to callers
> +        // because this function consumes `self`.
> +        self.0.take().unwrap().0
> +    }
> +}
> +
> +impl ScopeGuard<(), fn(())> {
> +    /// Creates a new guarded object with the given cleanup function.
> +    pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
> +        ScopeGuard::new_with_data((), move |_| cleanup())
> +    }
> +}
> +
> +impl<T, F: FnOnce(T)> Deref for ScopeGuard<T, F> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &T {
> +        // The type invariants guarantee that `unwrap` will succeed.
> +        &self.0.as_ref().unwrap().0
> +    }
> +}
> +
> +impl<T, F: FnOnce(T)> DerefMut for ScopeGuard<T, F> {
> +    fn deref_mut(&mut self) -> &mut T {
> +        // The type invariants guarantee that `unwrap` will succeed.
> +        &mut self.0.as_mut().unwrap().0
> +    }
> +}
> +
> +impl<T, F: FnOnce(T)> Drop for ScopeGuard<T, F> {
> +    fn drop(&mut self) {
> +        // Run the cleanup function if one is still present.
> +        if let Some((data, cleanup)) = self.0.take() {
> +            cleanup(data)
> +        }
> +    }
> +}
>  
>  /// Stores an opaque value.
>  ///


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

* Re: [PATCH v2 1/5] rust: types: introduce `ScopeGuard`
  2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
                   ` (4 preceding siblings ...)
  2023-01-30 18:50 ` [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Gary Guo
@ 2023-01-30 20:07 ` Andreas Hindborg
  2023-02-01  1:00 ` Miguel Ojeda
  6 siblings, 0 replies; 17+ messages in thread
From: Andreas Hindborg @ 2023-01-30 20:07 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo,
	Andreas Hindborg


Wedson Almeida Filho <wedsonaf@gmail.com> writes:

> This allows us to run some code when the guard is dropped (e.g.,
> implicitly when it goes out of scope). We can also prevent the
> guard from running by calling its `dismiss()` method.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> ---

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>

> v1 -> v2: Simpler type for `ScopeGuard::new()` impl block
>
>  rust/kernel/types.rs | 126 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 125 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index e84e51ec9716..dd834bfcb57b 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -2,7 +2,131 @@
>  
>  //! Kernel types.
>  
> -use core::{cell::UnsafeCell, mem::MaybeUninit};
> +use core::{
> +    cell::UnsafeCell,
> +    mem::MaybeUninit,
> +    ops::{Deref, DerefMut},
> +};
> +
> +/// Runs a cleanup function/closure when dropped.
> +///
> +/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
> +///
> +/// # Examples
> +///
> +/// In the example below, we have multiple exit paths and we want to log regardless of which one is
> +/// taken:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example1(arg: bool) {
> +///     let _log = ScopeGuard::new(|| pr_info!("example1 completed\n"));
> +///
> +///     if arg {
> +///         return;
> +///     }
> +///
> +///     pr_info!("Do something...\n");
> +/// }
> +///
> +/// # example1(false);
> +/// # example1(true);
> +/// ```
> +///
> +/// In the example below, we want to log the same message on all early exits but a different one on
> +/// the main exit path:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example2(arg: bool) {
> +///     let log = ScopeGuard::new(|| pr_info!("example2 returned early\n"));
> +///
> +///     if arg {
> +///         return;
> +///     }
> +///
> +///     // (Other early returns...)
> +///
> +///     log.dismiss();
> +///     pr_info!("example2 no early return\n");
> +/// }
> +///
> +/// # example2(false);
> +/// # example2(true);
> +/// ```
> +///
> +/// In the example below, we need a mutable object (the vector) to be accessible within the log
> +/// function, so we wrap it in the [`ScopeGuard`]:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example3(arg: bool) -> Result {
> +///     let mut vec =
> +///         ScopeGuard::new_with_data(Vec::new(), |v| pr_info!("vec had {} elements\n", v.len()));
> +///
> +///     vec.try_push(10u8)?;
> +///     if arg {
> +///         return Ok(());
> +///     }
> +///     vec.try_push(20u8)?;
> +///     Ok(())
> +/// }
> +///
> +/// # assert_eq!(example3(false), Ok(()));
> +/// # assert_eq!(example3(true), Ok(()));
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// The value stored in the struct is nearly always `Some(_)`, except between
> +/// [`ScopeGuard::dismiss`] and [`ScopeGuard::drop`]: in this case, it will be `None` as the value
> +/// will have been returned to the caller. Since  [`ScopeGuard::dismiss`] consumes the guard,
> +/// callers won't be able to use it anymore.
> +pub struct ScopeGuard<T, F: FnOnce(T)>(Option<(T, F)>);
> +
> +impl<T, F: FnOnce(T)> ScopeGuard<T, F> {
> +    /// Creates a new guarded object wrapping the given data and with the given cleanup function.
> +    pub fn new_with_data(data: T, cleanup_func: F) -> Self {
> +        // INVARIANT: The struct is being initialised with `Some(_)`.
> +        Self(Some((data, cleanup_func)))
> +    }
> +
> +    /// Prevents the cleanup function from running and returns the guarded data.
> +    pub fn dismiss(mut self) -> T {
> +        // INVARIANT: This is the exception case in the invariant; it is not visible to callers
> +        // because this function consumes `self`.
> +        self.0.take().unwrap().0
> +    }
> +}
> +
> +impl ScopeGuard<(), fn(())> {
> +    /// Creates a new guarded object with the given cleanup function.
> +    pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
> +        ScopeGuard::new_with_data((), move |_| cleanup())
> +    }
> +}
> +
> +impl<T, F: FnOnce(T)> Deref for ScopeGuard<T, F> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &T {
> +        // The type invariants guarantee that `unwrap` will succeed.
> +        &self.0.as_ref().unwrap().0
> +    }
> +}
> +
> +impl<T, F: FnOnce(T)> DerefMut for ScopeGuard<T, F> {
> +    fn deref_mut(&mut self) -> &mut T {
> +        // The type invariants guarantee that `unwrap` will succeed.
> +        &mut self.0.as_mut().unwrap().0
> +    }
> +}
> +
> +impl<T, F: FnOnce(T)> Drop for ScopeGuard<T, F> {
> +    fn drop(&mut self) {
> +        // Run the cleanup function if one is still present.
> +        if let Some((data, cleanup)) = self.0.take() {
> +            cleanup(data)
> +        }
> +    }
> +}
>  
>  /// Stores an opaque value.
>  ///


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

* Re: [PATCH v2 1/5] rust: types: introduce `ScopeGuard`
  2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
                   ` (5 preceding siblings ...)
  2023-01-30 20:07 ` Andreas Hindborg
@ 2023-02-01  1:00 ` Miguel Ojeda
  6 siblings, 0 replies; 17+ messages in thread
From: Miguel Ojeda @ 2023-02-01  1:00 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo

On Mon, Jan 30, 2023 at 7:44 AM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> This allows us to run some code when the guard is dropped (e.g.,
> implicitly when it goes out of scope). We can also prevent the
> guard from running by calling its `dismiss()` method.

Series applied to `rust-next`, thanks a lot!

Cheers,
Miguel

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

* Re: [PATCH v2 2/5] rust: types: introduce `ForeignOwnable`
  2023-01-30  6:44 ` [PATCH v2 2/5] rust: types: introduce `ForeignOwnable` Wedson Almeida Filho
  2023-01-30 18:49   ` Gary Guo
@ 2023-02-01  9:35   ` Andreas Hindborg
  1 sibling, 0 replies; 17+ messages in thread
From: Andreas Hindborg @ 2023-02-01  9:35 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo,
	Martin Rodriguez Reboredo, Andreas Hindborg


Wedson Almeida Filho <wedsonaf@gmail.com> writes:

> It was originally called `PointerWrapper`. It is used to convert
> a Rust object to a pointer representation (void *) that can be
> stored on the C side, used, and eventually returned to Rust.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> ---

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>

It would be cool to see a debug implementation of this trait that verify
safety properties by using a counter at runtime. Panic if borrow_mut()
is called while there are live references in existence returned by
borrow().

> v1 -> v2: Use `Self` instead of generic type in `borrow_mut`
>
>  rust/kernel/lib.rs   |  1 +
>  rust/kernel/types.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index e0b0e953907d..223564f9f0cc 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -16,6 +16,7 @@
>  #![feature(coerce_unsized)]
>  #![feature(core_ffi_c)]
>  #![feature(dispatch_from_dyn)]
> +#![feature(generic_associated_types)]
>  #![feature(receiver_trait)]
>  #![feature(unsize)]
>  
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index dd834bfcb57b..72710b7442a3 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -8,6 +8,60 @@ use core::{
>      ops::{Deref, DerefMut},
>  };
>  
> +/// Used to transfer ownership to and from foreign (non-Rust) languages.
> +///
> +/// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> +/// later may be transferred back to Rust by calling [`Self::from_foreign`].
> +///
> +/// This trait is meant to be used in cases when Rust objects are stored in C objects and
> +/// eventually "freed" back to Rust.
> +pub trait ForeignOwnable: Sized {
> +    /// Type of values borrowed between calls to [`ForeignOwnable::into_foreign`] and
> +    /// [`ForeignOwnable::from_foreign`].
> +    type Borrowed<'a>;
> +
> +    /// Converts a Rust-owned object to a foreign-owned one.
> +    ///
> +    /// The foreign representation is a pointer to void.
> +    fn into_foreign(self) -> *const core::ffi::c_void;
> +
> +    /// Borrows a foreign-owned object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
> +    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
> +    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow_mut`]
> +    /// for this object must have been dropped.
> +    unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>;
> +
> +    /// Mutably borrows a foreign-owned object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
> +    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
> +    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
> +    /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
> +    unsafe fn borrow_mut(ptr: *const core::ffi::c_void) -> ScopeGuard<Self, fn(Self)> {
> +        // SAFETY: The safety requirements ensure that `ptr` came from a previous call to
> +        // `into_foreign`.
> +        ScopeGuard::new_with_data(unsafe { Self::from_foreign(ptr) }, |d| {
> +            d.into_foreign();
> +        })
> +    }
> +
> +    /// Converts a foreign-owned object back to a Rust-owned one.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
> +    /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
> +    /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
> +    /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
> +    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
> +}
> +
>  /// Runs a cleanup function/closure when dropped.
>  ///
>  /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.


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

* Re: [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>`
  2023-01-30  6:44 ` [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>` Wedson Almeida Filho
@ 2023-02-01  9:56   ` Andreas Hindborg
  0 siblings, 0 replies; 17+ messages in thread
From: Andreas Hindborg @ 2023-02-01  9:56 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo,
	Alice Ferrazzi, Andreas Hindborg


Wedson Almeida Filho <wedsonaf@gmail.com> writes:

> This allows us to hand ownership of Rust dynamically allocated
> objects to the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> Reviewed-by: Alice Ferrazzi <alice.ferrazzi@miraclelinux.com>
> ---

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>

> v1 -> v2: Add `use alloc::boxed::Box`, which wasn't needed before
>
>  rust/kernel/types.rs | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 72710b7442a3..411655eca3e9 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -2,6 +2,7 @@
>  
>  //! Kernel types.
>  
> +use alloc::boxed::Box;
>  use core::{
>      cell::UnsafeCell,
>      mem::MaybeUninit,
> @@ -62,6 +63,28 @@ pub trait ForeignOwnable: Sized {
>      unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
>  }
>  
> +impl<T: 'static> ForeignOwnable for Box<T> {
> +    type Borrowed<'a> = &'a T;
> +
> +    fn into_foreign(self) -> *const core::ffi::c_void {
> +        Box::into_raw(self) as _
> +    }
> +
> +    unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T {
> +        // SAFETY: The safety requirements for this function ensure that the object is still alive,
> +        // so it is safe to dereference the raw pointer.
> +        // The safety requirements of `from_foreign` also ensure that the object remains alive for
> +        // the lifetime of the returned value.
> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
> +        // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
> +        // call to `Self::into_foreign`.
> +        unsafe { Box::from_raw(ptr as _) }
> +    }
> +}
> +
>  /// Runs a cleanup function/closure when dropped.
>  ///
>  /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.


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

* Re: [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type
  2023-01-30  6:44 ` [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type Wedson Almeida Filho
  2023-01-30 18:41   ` Gary Guo
@ 2023-02-01  9:58   ` Andreas Hindborg
  1 sibling, 0 replies; 17+ messages in thread
From: Andreas Hindborg @ 2023-02-01  9:58 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo,
	Andreas Hindborg


Wedson Almeida Filho <wedsonaf@gmail.com> writes:

> This allows us to use the unit type `()` when we have no object whose
> ownership must be managed but one implementing the `ForeignOwnable`
> trait is needed.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> ---

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>

> v1 -> v2: use `NonNull::dangling` to generate pointer
>
>  rust/kernel/types.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 411655eca3e9..9d0fdbc55843 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -85,6 +85,18 @@ impl<T: 'static> ForeignOwnable for Box<T> {
>      }
>  }
>  
> +impl ForeignOwnable for () {
> +    type Borrowed<'a> = ();
> +
> +    fn into_foreign(self) -> *const core::ffi::c_void {
> +        core::ptr::NonNull::dangling().as_ptr()
> +    }
> +
> +    unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
> +
> +    unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
> +}
> +
>  /// Runs a cleanup function/closure when dropped.
>  ///
>  /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.


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

* Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`
  2023-01-30  6:44 ` [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>` Wedson Almeida Filho
@ 2023-02-01 10:17   ` Andreas Hindborg
  2023-02-06 23:47     ` Miguel Ojeda
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Hindborg @ 2023-02-01 10:17 UTC (permalink / raw)
  To: Wedson Almeida Filho
  Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, linux-kernel, Vincenzo Palazzo,
	Alice Ferrazzi, Andreas Hindborg


Wedson Almeida Filho <wedsonaf@gmail.com> writes:

> This allows us to hand ownership of Rust ref-counted objects to
> the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> Reviewed-by: Alice Ferrazzi <alice.ferrazzi@miraclelinux.com>
> ---
> v1 -> v2: Unmodified
>
>  rust/kernel/sync/arc.rs | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index ff73f9240ca1..519a6ec43644 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -15,7 +15,11 @@
>  //!
>  //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
>  
> -use crate::{bindings, error::Result, types::Opaque};
> +use crate::{
> +    bindings,
> +    error::Result,
> +    types::{ForeignOwnable, Opaque},
> +};
>  use alloc::boxed::Box;
>  use core::{
>      marker::{PhantomData, Unsize},
> @@ -189,6 +193,32 @@ impl<T: ?Sized> Arc<T> {
>      }
>  }
>  
> +impl<T: 'static> ForeignOwnable for Arc<T> {
> +    type Borrowed<'a> = ArcBorrow<'a, T>;
> +
> +    fn into_foreign(self) -> *const core::ffi::c_void {
> +        ManuallyDrop::new(self).ptr.as_ptr() as _
> +    }
> +
> +    unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
> +        // SAFETY: By the safety requirement of this function, we know that `ptr` came from
> +        // a previous call to `Arc::into_foreign`.
> +        let inner = NonNull::new(ptr as *mut ArcInner<T>).unwrap();
> +
> +        // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive
> +        // for the lifetime of the returned value. Additionally, the safety requirements of
> +        // `ForeignOwnable::borrow_mut` ensure that no new mutable references are created.
> +        unsafe { ArcBorrow::new(inner) }
> +    }
> +
> +    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
> +        // SAFETY: By the safety requirement of this function, we know that `ptr` came from
> +        // a previous call to `Arc::into_foreign`, which owned guarantees that `ptr` is valid and
> +        // owns a reference.

The last part of the sentence does not read clearly to me. Would this
make sense instead:

// SAFETY: By the safety requirement of this function, we know that `ptr` came from
// a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
// holds a reference count increment that is transferrable to us.

> +        unsafe { Self::from_inner(NonNull::new(ptr as _).unwrap()) }
> +    }
> +}
> +
>  impl<T: ?Sized> Deref for Arc<T> {
>      type Target = T;


Cheers,
Andreas

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

* Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`
  2023-02-01 10:17   ` Andreas Hindborg
@ 2023-02-06 23:47     ` Miguel Ojeda
  2023-02-07  9:32       ` Andreas Hindborg
  0 siblings, 1 reply; 17+ messages in thread
From: Miguel Ojeda @ 2023-02-06 23:47 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Wedson Almeida Filho, rust-for-linux, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, linux-kernel,
	Vincenzo Palazzo, Alice Ferrazzi, Andreas Hindborg

On Wed, Feb 1, 2023 at 11:35 AM Andreas Hindborg <nmi@metaspace.dk> wrote:
>
> The last part of the sentence does not read clearly to me. Would this
> make sense instead:
>
> // SAFETY: By the safety requirement of this function, we know that `ptr` came from
> // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
> // holds a reference count increment that is transferrable to us.

In a private chat with Wedson he agreed the "owned" was a typo and he
is fine with this change. Thus I rebased to apply this and avoid a v3
given it is trivial and almost at the top of the stack. If you want
the `Reviewed-by`, please let me know!

Cheers,
Miguel

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

* Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`
  2023-02-06 23:47     ` Miguel Ojeda
@ 2023-02-07  9:32       ` Andreas Hindborg
  2023-02-07 10:26         ` Miguel Ojeda
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Hindborg @ 2023-02-07  9:32 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Wedson Almeida Filho, rust-for-linux, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, linux-kernel,
	Vincenzo Palazzo, Alice Ferrazzi


Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:

> On Wed, Feb 1, 2023 at 11:35 AM Andreas Hindborg <nmi@metaspace.dk> wrote:
>>
>> The last part of the sentence does not read clearly to me. Would this
>> make sense instead:
>>
>> // SAFETY: By the safety requirement of this function, we know that `ptr` came from
>> // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
>> // holds a reference count increment that is transferrable to us.
>
> In a private chat with Wedson he agreed the "owned" was a typo and he
> is fine with this change. Thus I rebased to apply this and avoid a v3
> given it is trivial and almost at the top of the stack. If you want
> the `Reviewed-by`, please let me know!

Sure, if it's no trouble, add my the tag :)

- Andreas


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

* Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`
  2023-02-07  9:32       ` Andreas Hindborg
@ 2023-02-07 10:26         ` Miguel Ojeda
  0 siblings, 0 replies; 17+ messages in thread
From: Miguel Ojeda @ 2023-02-07 10:26 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Wedson Almeida Filho, rust-for-linux, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, linux-kernel,
	Vincenzo Palazzo, Alice Ferrazzi

On Tue, Feb 7, 2023 at 10:34 AM Andreas Hindborg <nmi@metaspace.dk> wrote:
>
> Sure, if it's no trouble, add my the tag :)

Done!

Cheers,
Miguel

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

end of thread, other threads:[~2023-02-07 10:26 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30  6:44 [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Wedson Almeida Filho
2023-01-30  6:44 ` [PATCH v2 2/5] rust: types: introduce `ForeignOwnable` Wedson Almeida Filho
2023-01-30 18:49   ` Gary Guo
2023-02-01  9:35   ` Andreas Hindborg
2023-01-30  6:44 ` [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>` Wedson Almeida Filho
2023-02-01  9:56   ` Andreas Hindborg
2023-01-30  6:44 ` [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type Wedson Almeida Filho
2023-01-30 18:41   ` Gary Guo
2023-02-01  9:58   ` Andreas Hindborg
2023-01-30  6:44 ` [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>` Wedson Almeida Filho
2023-02-01 10:17   ` Andreas Hindborg
2023-02-06 23:47     ` Miguel Ojeda
2023-02-07  9:32       ` Andreas Hindborg
2023-02-07 10:26         ` Miguel Ojeda
2023-01-30 18:50 ` [PATCH v2 1/5] rust: types: introduce `ScopeGuard` Gary Guo
2023-01-30 20:07 ` Andreas Hindborg
2023-02-01  1:00 ` Miguel Ojeda

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