Skip to content

Commit

Permalink
rust: experiment with #[derive(SmartPointer)]
Browse files Browse the repository at this point in the history
I am sending this RFC patch to share my experience with using the new
`#[derive(SmartPointer)]` feature [1] with our custom smart pointers.
The feature is being added so that the kernel can stop using the
unstable dispatch_from_dyn and unsize features.

In general, the feature appears to work. As can be seen in the change to
`rust_minimal.rs`, it is possible to use `Arc` together with a dynamic
trait object, and the trait object is object safe even though it uses
the custom smart pointer as a self parameter.

I did run into one nit, which is that `Arc` requires the `#[pointee]`
annotation even though there's only one generic paramter. I filed an
issue [2] about this.

Link: https://rust-lang.github.io/rfcs/3621-derive-smart-pointer.html [1]
Link: rust-lang/rust#129465 [2]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
  • Loading branch information
Darksonn committed Aug 23, 2024
1 parent 13fd550 commit 572e93f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 39 deletions.
3 changes: 1 addition & 2 deletions rust/kernel/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@

#![no_std]
#![feature(coerce_unsized)]
#![feature(dispatch_from_dyn)]
#![feature(derive_smart_pointer)]
#![feature(new_uninit)]
#![feature(receiver_trait)]
#![feature(unsize)]

// Ensure conditional compilation based on the kernel configuration works;
// otherwise we may silently break things like initcall handling.
Expand Down
23 changes: 3 additions & 20 deletions rust/kernel/list/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use crate::alloc::{AllocError, Flags};
use crate::prelude::*;
use crate::sync::{Arc, ArcBorrow, UniqueArc};
use core::marker::{PhantomPinned, Unsize};
use core::marker::{PhantomPinned, SmartPointer};
use core::ops::Deref;
use core::pin::Pin;
use core::sync::atomic::{AtomicBool, Ordering};
Expand Down Expand Up @@ -158,8 +158,9 @@ pub use impl_list_arc_safe;
/// * The tracking inside `T` is aware that a `ListArc` reference exists.
///
/// [`List`]: crate::list::List
#[derive(SmartPointer)]
#[repr(transparent)]
pub struct ListArc<T, const ID: u64 = 0>
pub struct ListArc<#[pointee] T, const ID: u64 = 0>
where
T: ListArcSafe<ID> + ?Sized,
{
Expand Down Expand Up @@ -444,24 +445,6 @@ where
// This is to allow [`ListArc`] (and variants) to be used as the type of `self`.
impl<T, const ID: u64> core::ops::Receiver for ListArc<T, ID> where T: ListArcSafe<ID> + ?Sized {}

// This is to allow coercion from `ListArc<T>` to `ListArc<U>` if `T` can be converted to the
// dynamically-sized type (DST) `U`.
impl<T, U, const ID: u64> core::ops::CoerceUnsized<ListArc<U, ID>> for ListArc<T, ID>
where
T: ListArcSafe<ID> + Unsize<U> + ?Sized,
U: ListArcSafe<ID> + ?Sized,
{
}

// This is to allow `ListArc<U>` to be dispatched on when `ListArc<T>` can be coerced into
// `ListArc<U>`.
impl<T, U, const ID: u64> core::ops::DispatchFromDyn<ListArc<U, ID>> for ListArc<T, ID>
where
T: ListArcSafe<ID> + Unsize<U> + ?Sized,
U: ListArcSafe<ID> + ?Sized,
{
}

/// A utility for tracking whether a [`ListArc`] exists using an atomic.
///
/// # Invariant
Expand Down
24 changes: 7 additions & 17 deletions rust/kernel/sync/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use alloc::boxed::Box;
use core::{
alloc::Layout,
fmt,
marker::{PhantomData, Unsize},
marker::{PhantomData, SmartPointer},
mem::{ManuallyDrop, MaybeUninit},
ops::{Deref, DerefMut},
pin::Pin,
Expand Down Expand Up @@ -126,7 +126,9 @@ mod std_vendor;
/// let coerced: Arc<dyn MyTrait> = obj;
/// # Ok::<(), Error>(())
/// ```
pub struct Arc<T: ?Sized> {
#[derive(SmartPointer)]
#[repr(transparent)]
pub struct Arc<#[pointee] T: ?Sized> {
ptr: NonNull<ArcInner<T>>,
_p: PhantomData<ArcInner<T>>,
}
Expand Down Expand Up @@ -174,13 +176,6 @@ impl<T: ?Sized> ArcInner<T> {
// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
impl<T: ?Sized> core::ops::Receiver for Arc<T> {}

// This is to allow coercion from `Arc<T>` to `Arc<U>` if `T` can be converted to the
// dynamically-sized type (DST) `U`.
impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}

// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}

// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` using a
Expand Down Expand Up @@ -475,21 +470,16 @@ impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
/// obj.as_arc_borrow().use_reference();
/// # Ok::<(), Error>(())
/// ```
pub struct ArcBorrow<'a, T: ?Sized + 'a> {
#[derive(SmartPointer)]
#[repr(transparent)]
pub struct ArcBorrow<'a, #[pointee] T: ?Sized + 'a> {
inner: NonNull<ArcInner<T>>,
_p: PhantomData<&'a ()>,
}

// This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}

// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
// `ArcBorrow<U>`.
impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
for ArcBorrow<'_, T>
{
}

impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
fn clone(&self) -> Self {
*self
Expand Down
15 changes: 15 additions & 0 deletions samples/rust/rust_minimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//! Rust minimal sample.

use kernel::prelude::*;
use kernel::sync::Arc;

module! {
type: RustMinimal,
Expand All @@ -16,6 +17,15 @@ struct RustMinimal {
numbers: Vec<i32>,
}

trait MyTrait {
fn my_fn(self: Arc<Self>);
}
impl MyTrait for Vec<i32> {
fn my_fn(self: Arc<Self>) {
pr_info!("{:?}", self.as_slice());
}
}

impl kernel::Module for RustMinimal {
fn init(_module: &'static ThisModule) -> Result<Self> {
pr_info!("Rust minimal sample (init)\n");
Expand All @@ -26,6 +36,11 @@ impl kernel::Module for RustMinimal {
numbers.push(108, GFP_KERNEL)?;
numbers.push(200, GFP_KERNEL)?;

let in_arc = kernel::sync::Arc::new(numbers)?;
in_arc.my_fn();
let arc_dyn: Arc<dyn MyTrait> = in_arc;
arc_dyn.my_fn();

Ok(RustMinimal { numbers })
}
}
Expand Down

0 comments on commit 572e93f

Please sign in to comment.