Skip to content

Commit aae24e9

Browse files
author
Sven Van Asbroeck
committed
rust/kernel: chrdev: remove unnecessary pinning
`chrdev`'s `Registration` and `RegistrationInner` do not contain any self-referential or borrowed-with-kernel structures. Therefore they do not require pinning. Remove all pinning. This simplifies the code greatly, and eliminates an unsafe block, and a fallible `Box` allocation. As part of this commit, also update `chrdev` client drivers. Signed-off-by: Sven Van Asbroeck <[email protected]>
1 parent 698569d commit aae24e9

File tree

2 files changed

+12
-43
lines changed

2 files changed

+12
-43
lines changed

rust/kernel/chrdev.rs

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@
88
//!
99
//! Reference: <https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#char-devices>
1010
11-
use alloc::boxed::Box;
1211
use core::convert::TryInto;
13-
use core::marker::PhantomPinned;
14-
use core::pin::Pin;
1512

1613
use crate::bindings;
1714
use crate::c_types;
@@ -80,12 +77,11 @@ struct RegistrationInner<const N: usize> {
8077
dev: bindings::dev_t,
8178
used: usize,
8279
cdevs: [Option<Cdev>; N],
83-
_pin: PhantomPinned,
8480
}
8581

8682
/// Character device registration.
8783
///
88-
/// May contain up to a fixed number (`N`) of devices. Must be pinned.
84+
/// May contain up to a fixed number (`N`) of devices.
8985
pub struct Registration<const N: usize> {
9086
name: &'static CStr,
9187
minors_start: u16,
@@ -97,12 +93,6 @@ impl<const N: usize> Registration<{ N }> {
9793
/// Creates a [`Registration`] object for a character device.
9894
///
9995
/// This does *not* register the device: see [`Self::register()`].
100-
///
101-
/// This associated function is intended to be used when you need to avoid
102-
/// a memory allocation, e.g. when the [`Registration`] is a member of
103-
/// a bigger structure inside your [`crate::KernelModule`] instance. If you
104-
/// are going to pin the registration right away, call
105-
/// [`Self::new_pinned()`] instead.
10696
pub fn new(
10797
name: &'static CStr,
10898
minors_start: u16,
@@ -116,60 +106,42 @@ impl<const N: usize> Registration<{ N }> {
116106
}
117107
}
118108

119-
/// Creates a pinned [`Registration`] object for a character device.
120-
///
121-
/// This does *not* register the device: see [`Self::register()`].
122-
pub fn new_pinned(
123-
name: &'static CStr,
124-
minors_start: u16,
125-
this_module: &'static crate::ThisModule,
126-
) -> Result<Pin<Box<Self>>> {
127-
Ok(Pin::from(Box::try_new(Self::new(
128-
name,
129-
minors_start,
130-
this_module,
131-
))?))
132-
}
133-
134109
/// Registers a character device.
135110
///
136111
/// You may call this once per device type, up to `N` times.
137-
pub fn register<T: file_operations::FileOpener<()>>(self: Pin<&mut Self>) -> Result {
138-
// SAFETY: We must ensure that we never move out of `this`.
139-
let this = unsafe { self.get_unchecked_mut() };
140-
if this.inner.is_none() {
112+
pub fn register<T: file_operations::FileOpener<()>>(&mut self) -> Result {
113+
if self.inner.is_none() {
141114
let mut dev: bindings::dev_t = 0;
142115
// SAFETY: Calling unsafe function. `this.name` has `'static`
143116
// lifetime.
144117
let res = unsafe {
145118
bindings::alloc_chrdev_region(
146119
&mut dev,
147-
this.minors_start.into(),
120+
self.minors_start.into(),
148121
N.try_into()?,
149-
this.name.as_char_ptr(),
122+
self.name.as_char_ptr(),
150123
)
151124
};
152125
if res != 0 {
153126
return Err(Error::from_kernel_errno(res));
154127
}
155128
const NONE: Option<Cdev> = None;
156-
this.inner = Some(RegistrationInner {
129+
self.inner = Some(RegistrationInner {
157130
dev,
158131
used: 0,
159132
cdevs: [NONE; N],
160-
_pin: PhantomPinned,
161133
});
162134
}
163135

164-
let mut inner = this.inner.as_mut().unwrap();
136+
let mut inner = self.inner.as_mut().unwrap();
165137
if inner.used == N {
166138
return Err(Error::EINVAL);
167139
}
168140

169141
// SAFETY: The adapter doesn't retrieve any state yet, so it's compatible with any
170142
// registration.
171143
let fops = unsafe { file_operations::FileOperationsVtable::<Self, T>::build() };
172-
let mut cdev = Cdev::alloc(fops, &this.this_module)?;
144+
let mut cdev = Cdev::alloc(fops, &self.this_module)?;
173145
cdev.add(inner.dev + inner.used as bindings::dev_t, 1)?;
174146
inner.cdevs[inner.used].replace(cdev);
175147
inner.used += 1;

samples/rust/rust_chrdev.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
#![no_std]
66
#![feature(allocator_api, global_asm)]
77

8-
use alloc::boxed::Box;
9-
use core::pin::Pin;
108
use kernel::prelude::*;
119
use kernel::{c_str, chrdev, file_operations::FileOperations};
1210

@@ -26,21 +24,20 @@ impl FileOperations for RustFile {
2624
}
2725

2826
struct RustChrdev {
29-
_dev: Pin<Box<chrdev::Registration<2>>>,
27+
_dev: chrdev::Registration<2>,
3028
}
3129

3230
impl KernelModule for RustChrdev {
3331
fn init() -> Result<Self> {
3432
pr_info!("Rust character device sample (init)\n");
3533

36-
let mut chrdev_reg =
37-
chrdev::Registration::new_pinned(c_str!("rust_chrdev"), 0, &THIS_MODULE)?;
34+
let mut chrdev_reg = chrdev::Registration::new(c_str!("rust_chrdev"), 0, &THIS_MODULE);
3835

3936
// Register the same kind of device twice, we're just demonstrating
4037
// that you can use multiple minors. There are two minors in this case
4138
// because its type is `chrdev::Registration<2>`
42-
chrdev_reg.as_mut().register::<RustFile>()?;
43-
chrdev_reg.as_mut().register::<RustFile>()?;
39+
chrdev_reg.register::<RustFile>()?;
40+
chrdev_reg.register::<RustFile>()?;
4441

4542
Ok(RustChrdev { _dev: chrdev_reg })
4643
}

0 commit comments

Comments
 (0)