Skip to content

Commit 9888b04

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 621dcf9 commit 9888b04

File tree

2 files changed

+25
-21
lines changed

2 files changed

+25
-21
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,14 +67,14 @@ 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
let of_match_tbl = OfMatchTable::new(&c_str!("brcm,bcm2835-rng"))?;
7676

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

rust/kernel/platdev.rs

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@ 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.
21-
#[derive(Default)]
20+
///
21+
/// # Invariants
22+
///
23+
/// We never move out of `pdrv`'s `Box`.
2224
pub struct Registration {
2325
of_table: Option<*const c_types::c_void>,
24-
pdrv: bindings::platform_driver,
25-
_pin: PhantomPinned,
26+
pdrv: Box<bindings::platform_driver>,
2627
}
2728

2829
// SAFETY: `Registration` does not expose any of its state across threads
@@ -81,42 +82,45 @@ extern "C" fn remove_callback<P: PlatformDriver>(
8182
impl Registration {
8283
/// Registers a platform device.
8384
///
84-
/// Returns a pinned heap-allocated representation of the registration.
85-
pub fn new_pinned<P: PlatformDriver>(
85+
/// Returns a representation of the registration.
86+
pub fn new<P: PlatformDriver>(
8687
name: &'static CStr,
8788
of_match_table: Option<OfMatchTable>,
8889
module: &'static crate::ThisModule,
89-
) -> Result<Pin<Box<Self>>> {
90-
let mut this = Box::try_new(Self::default())?;
91-
this.pdrv.driver.name = name.as_char_ptr();
92-
if let Some(tbl) = of_match_table {
90+
) -> Result<Self> {
91+
let mut pdrv = Box::try_new(bindings::platform_driver::default())?;
92+
pdrv.driver.name = name.as_char_ptr();
93+
let of_table = if let Some(tbl) = of_match_table {
9394
let ptr = tbl.into_pointer();
94-
this.of_table = Some(ptr);
95-
this.pdrv.driver.of_match_table = ptr.cast();
96-
}
97-
this.pdrv.probe = Some(probe_callback::<P>);
98-
this.pdrv.remove = Some(remove_callback::<P>);
95+
pdrv.driver.of_match_table = ptr.cast();
96+
Some(ptr)
97+
} else {
98+
None
99+
};
100+
pdrv.probe = Some(probe_callback::<P>);
101+
pdrv.remove = Some(remove_callback::<P>);
99102
// SAFETY:
100-
// - `this.pdrv` lives at least until the call to `platform_driver_unregister()` returns.
103+
// - `pdrv` will never move out of its `Box`, and lives at least
104+
// until the call to `platform_driver_unregister()` returns.
101105
// - `name` pointer has static lifetime.
102106
// - `module.0` lives at least as long as the module.
103107
// - `probe()` and `remove()` are static functions.
104108
// - `of_match_table` is either:
105109
// - a raw pointer which lives until after the call to
106110
// `bindings::platform_driver_unregister()`, or
107111
// - null.
108-
let ret = unsafe { bindings::__platform_driver_register(&mut this.pdrv, module.0) };
112+
let ret = unsafe { bindings::__platform_driver_register(&mut *pdrv, module.0) };
109113
if ret < 0 {
110114
return Err(Error::from_kernel_errno(ret));
111115
}
112-
Ok(Pin::from(this))
116+
Ok(Self { of_table, pdrv })
113117
}
114118
}
115119

116120
impl Drop for Registration {
117121
fn drop(&mut self) {
118122
// SAFETY: `self.pdev` was registered previously.
119-
unsafe { bindings::platform_driver_unregister(&mut self.pdrv) }
123+
unsafe { bindings::platform_driver_unregister(&mut *self.pdrv) }
120124
if let Some(ptr) = self.of_table {
121125
// SAFETY: `ptr` came from an `OfMatchTable`.
122126
let tbl = unsafe { OfMatchTable::from_pointer(ptr) };

0 commit comments

Comments
 (0)