2023-01-30 06:44:32

by Wedson Almeida Filho

[permalink] [raw]
Subject: [PATCH v2 1/5] rust: types: introduce `ScopeGuard`

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 <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
---
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



2023-01-30 06:44:45

by Wedson Almeida Filho

[permalink] [raw]
Subject: [PATCH v2 2/5] rust: types: introduce `ForeignOwnable`

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 <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
---
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


2023-01-30 06:44:48

by Wedson Almeida Filho

[permalink] [raw]
Subject: [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>`

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

Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Alice Ferrazzi <[email protected]>
---
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


2023-01-30 06:45:10

by Wedson Almeida Filho

[permalink] [raw]
Subject: [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type

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 <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
---
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


2023-01-30 06:46:48

by Wedson Almeida Filho

[permalink] [raw]
Subject: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`

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

Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Alice Ferrazzi <[email protected]>
---
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


2023-01-30 18:42:13

by Gary Guo

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type

On Mon, 30 Jan 2023 03:44:03 -0300
Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

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


2023-01-30 18:49:45

by Gary Guo

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] rust: types: introduce `ForeignOwnable`

On Mon, 30 Jan 2023 03:44:01 -0300
Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>
> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

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


2023-01-30 18:50:26

by Gary Guo

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] rust: types: introduce `ScopeGuard`

On Mon, 30 Jan 2023 03:44:00 -0300
Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

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


2023-01-30 20:08:29

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] rust: types: introduce `ScopeGuard`


Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>
> ---

Reviewed-by: Andreas Hindborg <[email protected]>

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


2023-02-01 01:00:45

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] rust: types: introduce `ScopeGuard`

On Mon, Jan 30, 2023 at 7:44 AM Wedson Almeida Filho <[email protected]> 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

2023-02-01 09:41:01

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] rust: types: introduce `ForeignOwnable`


Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>
> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
> ---

Reviewed-by: Andreas Hindborg <[email protected]>

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.


2023-02-01 09:57:16

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] rust: types: implement `ForeignOwnable` for `Box<T>`


Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Gary Guo <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>
> Reviewed-by: Alice Ferrazzi <[email protected]>
> ---

Reviewed-by: Andreas Hindborg <[email protected]>

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


2023-02-01 09:59:15

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] rust: types: implement `ForeignOwnable` for the unit type


Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>
> ---

Reviewed-by: Andreas Hindborg <[email protected]>

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


2023-02-01 10:36:00

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`


Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> Reviewed-by: Gary Guo <[email protected]>
> Reviewed-by: Vincenzo Palazzo <[email protected]>
> Reviewed-by: Alice Ferrazzi <[email protected]>
> ---
> 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

2023-02-06 23:47:29

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`

On Wed, Feb 1, 2023 at 11:35 AM Andreas Hindborg <[email protected]> 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

2023-02-07 09:34:14

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`


Miguel Ojeda <[email protected]> writes:

> On Wed, Feb 1, 2023 at 11:35 AM Andreas Hindborg <[email protected]> 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


2023-02-07 10:26:28

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] rust: types: implement `ForeignOwnable` for `Arc<T>`

On Tue, Feb 7, 2023 at 10:34 AM Andreas Hindborg <[email protected]> wrote:
>
> Sure, if it's no trouble, add my the tag :)

Done!

Cheers,
Miguel