Skip to content

Commit

Permalink
rust/kernel/platdev: get rid of unnecessary Pinning
Browse files Browse the repository at this point in the history
`Registration` contains a self-referential structure.
We need to ensure that users never move that structure around.
This is currently done by marking `Registration` as `!Unpin`,
and returning it as a `Pin<Box<Registration>>`, which users cannot
move out of - not without the use of `unsafe`, anyway.

However, the self-referenial structure is an implementation detail,
which we are pushing onto users, by using `Pin` in the public function
signature.

Treat the self-referential structure as a true, internal implementation
detail: store it into a private `Box` member of `Registration`. This is
safe as long as `Registration` never moves that structure out of its
`Box`.

Now `Registration` can safely be moved by its users, and there is no
longer a need to mark `Registration` as `!Unpin`, or return it
`Pin`ned to users. This greatly simplifies and clarifies driver-side
code.

Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
  • Loading branch information
Sven Van Asbroeck committed Jun 29, 2021
1 parent dbbd1dc commit 221d99b
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 18 deletions.
4 changes: 2 additions & 2 deletions drivers/char/hw_random/bcm2835_rng_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ impl PlatformDriver for RngDriver {
}

struct RngModule {
_pdev: Pin<Box<platdev::Registration>>,
_pdev: platdev::Registration,
}

impl KernelModule for RngModule {
fn init() -> Result<Self> {
const OF_MATCH_TBL: ConstOfMatchTable<1> =
ConstOfMatchTable::new_const([&c_str!("brcm,bcm2835-rng")]);

let pdev = platdev::Registration::new_pinned::<RngDriver>(
let pdev = platdev::Registration::new::<RngDriver>(
c_str!("bcm2835-rng-rust"),
Some(&OF_MATCH_TBL),
&THIS_MODULE,
Expand Down
31 changes: 15 additions & 16 deletions rust/kernel/platdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@ use crate::{
types::PointerWrapper,
};
use alloc::boxed::Box;
use core::{marker::PhantomPinned, pin::Pin};

/// A registration of a platform device.
#[derive(Default)]
pub struct Registration {
pdrv: bindings::platform_driver,
_pin: PhantomPinned,
// We never move out of `pdrv`'s `Box`.
pdrv: Box<bindings::platform_driver>,
}

// SAFETY: `Registration` does not expose any of its state across threads
Expand Down Expand Up @@ -80,39 +79,39 @@ extern "C" fn remove_callback<P: PlatformDriver>(
impl Registration {
/// Registers a platform device.
///
/// Returns a pinned heap-allocated representation of the registration.
pub fn new_pinned<P: PlatformDriver>(
/// Returns a representation of the registration.
pub fn new<P: PlatformDriver>(
name: &'static CStr,
of_match_table: Option<&'static OfMatchTable>,
module: &'static crate::ThisModule,
) -> Result<Pin<Box<Self>>> {
let mut this = Box::try_new(Self::default())?;
this.pdrv.driver.name = name.as_char_ptr();
) -> Result<Self> {
let mut pdrv = Box::try_new(bindings::platform_driver::default())?;
pdrv.driver.name = name.as_char_ptr();
if let Some(tbl) = of_match_table {
this.pdrv.driver.of_match_table = tbl.as_ptr();
pdrv.driver.of_match_table = tbl.as_ptr();
}
this.pdrv.probe = Some(probe_callback::<P>);
this.pdrv.remove = Some(remove_callback::<P>);
pdrv.probe = Some(probe_callback::<P>);
pdrv.remove = Some(remove_callback::<P>);
// SAFETY:
// - `this.pdrv` lives at least until the call to `platform_driver_unregister()` returns.
// - `name` pointer has static lifetime.
// - `pdrv` will never move out of its `Box`, and lives at least
// until the call to `platform_driver_unregister()` returns.
// - `module.0` lives at least as long as the module.
// - `probe()` and `remove()` are static functions.
// - `of_match_table` is either a raw pointer with static lifetime,
// as guaranteed by the [`of::OfMatchTable::as_ptr()`] return type,
// or null.
let ret = unsafe { bindings::__platform_driver_register(&mut this.pdrv, module.0) };
let ret = unsafe { bindings::__platform_driver_register(&mut *pdrv, module.0) };
if ret < 0 {
return Err(Error::from_kernel_errno(ret));
}
Ok(Pin::from(this))
Ok(Self { pdrv })
}
}

impl Drop for Registration {
fn drop(&mut self) {
// SAFETY: `self.pdev` was registered previously.
unsafe { bindings::platform_driver_unregister(&mut self.pdrv) }
unsafe { bindings::platform_driver_unregister(&mut *self.pdrv) }
}
}

Expand Down

0 comments on commit 221d99b

Please sign in to comment.