Skip to content

Allow miscdev::Registration to carry some context with it. #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion drivers/char/rust_example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl KernelModule for RustExample {

Ok(RustExample {
message: "on the heap!".to_owned(),
_dev: miscdev::Registration::new_pinned::<RustFile>(cstr!("rust_miscdev"), None)?,
_dev: miscdev::Registration::new_pinned::<RustFile>(cstr!("rust_miscdev"), None, ())?,
_chrdev: chrdev_reg,
})
}
Expand Down
39 changes: 22 additions & 17 deletions rust/kernel/miscdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,41 +14,46 @@ use core::marker::PhantomPinned;
use core::pin::Pin;

/// A registration of a miscellaneous device.
pub struct Registration {
pub struct Registration<T: Sync = ()> {
registered: bool,
mdev: bindings::miscdevice,
_pin: PhantomPinned,

/// Context initialised on construction.
pub context: T,
}

impl Registration {
impl<T: Sync> Registration<T> {
/// Creates a new [`Registration`] but does not register it yet.
///
/// It is allowed to move.
pub fn new() -> Self {
pub fn new(context: T) -> Self {
Self {
registered: false,
mdev: bindings::miscdevice::default(),
_pin: PhantomPinned,
context,
}
}

/// Registers a miscellaneous device.
///
/// Returns a pinned heap-allocated representation of the registration.
pub fn new_pinned<T: FileOperations>(
pub fn new_pinned<F: FileOperations>(
name: CStr<'static>,
minor: Option<i32>,
context: T,
) -> KernelResult<Pin<Box<Self>>> {
let mut r = Pin::from(Box::try_new(Self::new())?);
r.as_mut().register::<T>(name, minor)?;
let mut r = Pin::from(Box::try_new(Self::new(context))?);
r.as_mut().register::<F>(name, minor)?;
Ok(r)
}

/// Registers a miscellaneous device with the rest of the kernel.
///
/// It must be pinned because the memory block that represents the registration is
/// self-referential. If a minor is not given, the kernel allocates a new one if possible.
pub fn register<T: FileOperations>(
pub fn register<F: FileOperations>(
self: Pin<&mut Self>,
name: CStr<'static>,
minor: Option<i32>,
Expand All @@ -60,7 +65,7 @@ impl Registration {
return Err(Error::EINVAL);
}

this.mdev.fops = FileOperationsVtable::<T>::build();
this.mdev.fops = FileOperationsVtable::<F>::build();
this.mdev.name = name.as_ptr() as *const c_types::c_char;
this.mdev.minor = minor.unwrap_or(bindings::MISC_DYNAMIC_MINOR as i32);

Expand All @@ -73,17 +78,17 @@ impl Registration {
}
}

impl Default for Registration {
fn default() -> Self {
Self::new()
}
}

// SAFETY: The only method is `register()`, which requires a (pinned) mutable `Registration`, so it
// is safe to pass `&Registration` to multiple threads because it offers no interior mutability.
unsafe impl Sync for Registration {}
// is safe to pass `&Registration` to multiple threads because it offers no interior mutability,
// except maybe through `Registration::context`, but it is itself `Sync`.
unsafe impl<T: Sync> Sync for Registration<T> {}

// SAFETY: All functions work from any thread. So as long as the `Registration::context` is
// `Send`, so is `Registration<T>`. `T` needs to be `Sync` because it's a requirement of
// `Registration<T>`.
unsafe impl<T: Send + Sync> Send for Registration<T> {}

impl Drop for Registration {
impl<T: Sync> Drop for Registration<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does T need to be Send as well so it can be dropped from any thread?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because its ownership remains with its creator, so it can ensure that it's called from the same thread.

I'm making Registration Send (if T is as well) so that the creator can destroy it from any thread if it wants to.

/// Removes the registration from the kernel if it has completed successfully before.
fn drop(&mut self) {
if self.registered {
Expand Down