Skip to content

Commit 221d99b

Browse files
author
Sven Van Asbroeck
committed
rust/kernel/platdev: get rid of unnecessary Pinning
`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 <[email protected]>
1 parent dbbd1dc commit 221d99b

File tree

2 files changed

+17
-18
lines changed

2 files changed

+17
-18
lines changed

drivers/char/hw_random/bcm2835_rng_rust.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,15 @@ impl PlatformDriver for RngDriver {
6767
}
6868

6969
struct RngModule {
70-
_pdev: Pin<Box<platdev::Registration>>,
70+
_pdev: platdev::Registration,
7171
}
7272

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

78-
let pdev = platdev::Registration::new_pinned::<RngDriver>(
78+
let pdev = platdev::Registration::new::<RngDriver>(
7979
c_str!("bcm2835-rng-rust"),
8080
Some(&OF_MATCH_TBL),
8181
&THIS_MODULE,

rust/kernel/platdev.rs

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,12 @@ use crate::{
1515
types::PointerWrapper,
1616
};
1717
use alloc::boxed::Box;
18-
use core::{marker::PhantomPinned, pin::Pin};
1918

2019
/// A registration of a platform device.
2120
#[derive(Default)]
2221
pub struct Registration {
23-
pdrv: bindings::platform_driver,
24-
_pin: PhantomPinned,
22+
// We never move out of `pdrv`'s `Box`.
23+
pdrv: Box<bindings::platform_driver>,
2524
}
2625

2726
// SAFETY: `Registration` does not expose any of its state across threads
@@ -80,39 +79,39 @@ extern "C" fn remove_callback<P: PlatformDriver>(
8079
impl Registration {
8180
/// Registers a platform device.
8281
///
83-
/// Returns a pinned heap-allocated representation of the registration.
84-
pub fn new_pinned<P: PlatformDriver>(
82+
/// Returns a representation of the registration.
83+
pub fn new<P: PlatformDriver>(
8584
name: &'static CStr,
8685
of_match_table: Option<&'static OfMatchTable>,
8786
module: &'static crate::ThisModule,
88-
) -> Result<Pin<Box<Self>>> {
89-
let mut this = Box::try_new(Self::default())?;
90-
this.pdrv.driver.name = name.as_char_ptr();
87+
) -> Result<Self> {
88+
let mut pdrv = Box::try_new(bindings::platform_driver::default())?;
89+
pdrv.driver.name = name.as_char_ptr();
9190
if let Some(tbl) = of_match_table {
92-
this.pdrv.driver.of_match_table = tbl.as_ptr();
91+
pdrv.driver.of_match_table = tbl.as_ptr();
9392
}
94-
this.pdrv.probe = Some(probe_callback::<P>);
95-
this.pdrv.remove = Some(remove_callback::<P>);
93+
pdrv.probe = Some(probe_callback::<P>);
94+
pdrv.remove = Some(remove_callback::<P>);
9695
// SAFETY:
97-
// - `this.pdrv` lives at least until the call to `platform_driver_unregister()` returns.
98-
// - `name` pointer has static lifetime.
96+
// - `pdrv` will never move out of its `Box`, and lives at least
97+
// until the call to `platform_driver_unregister()` returns.
9998
// - `module.0` lives at least as long as the module.
10099
// - `probe()` and `remove()` are static functions.
101100
// - `of_match_table` is either a raw pointer with static lifetime,
102101
// as guaranteed by the [`of::OfMatchTable::as_ptr()`] return type,
103102
// or null.
104-
let ret = unsafe { bindings::__platform_driver_register(&mut this.pdrv, module.0) };
103+
let ret = unsafe { bindings::__platform_driver_register(&mut *pdrv, module.0) };
105104
if ret < 0 {
106105
return Err(Error::from_kernel_errno(ret));
107106
}
108-
Ok(Pin::from(this))
107+
Ok(Self { pdrv })
109108
}
110109
}
111110

112111
impl Drop for Registration {
113112
fn drop(&mut self) {
114113
// SAFETY: `self.pdev` was registered previously.
115-
unsafe { bindings::platform_driver_unregister(&mut self.pdrv) }
114+
unsafe { bindings::platform_driver_unregister(&mut *self.pdrv) }
116115
}
117116
}
118117

0 commit comments

Comments
 (0)