Re: [PATCH] rust: miscdevice: add registration data to MiscDevice
"Gary Guo" <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Fri Aug 7, 2026 at 3:22 PM BST, Alice Ryhl wrote: > Currently when fds for a miscdevice are opened, the only "global" data > they are given access to is the MiscDeviceRegistration value. However, > this value doesn't let you store any user-provided data, so there is no > way for different fds from the same miscdevice to interact with each > other. Thus, let the user specify a type to be stored in the > MiscDeviceRegistration in which the user can store whichever data they > would like. > > The intended user of this patch is Rust course material. Miscdevice is a > nice and relatively easy to use API for someone's first driver, and > being able to persist data from fd to fd allows the student to interact > with their driver using 'cat' and 'echo', even though each call opens a > new fd. > > Signed-off-by: Alice Ryhl <[email protected]> > --- > rust/kernel/miscdevice.rs | 42 ++++++++++++++++++++++++++++++---------- > samples/rust/rust_misc_device.rs | 3 ++- > 2 files changed, 34 insertions(+), 11 deletions(-) > > diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs > index 3abef1b8543d..c4910918cd15 100644 > --- a/rust/kernel/miscdevice.rs > +++ b/rust/kernel/miscdevice.rs > @@ -31,7 +31,10 @@ > Opaque, // > }, > }; > -use core::marker::PhantomData; > +use core::{ > + marker::PhantomData, > + ops::Deref, // > +}; > > /// Options for creating a misc device. > #[derive(Copy, Clone)] > @@ -62,25 +65,30 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice { > /// - Deregistration occurs exactly once in [`Drop`] via `misc_deregister()`. > /// - `inner` wraps a valid, pinned `miscdevice` created using > /// [`MiscDeviceOptions::into_raw`]. > -#[repr(transparent)] > +#[repr(C)] > #[pin_data(PinnedDrop)] > -pub struct MiscDeviceRegistration<T> { > +pub struct MiscDeviceRegistration<T: MiscDevice> { > #[pin] > inner: Opaque<bindings::miscdevice>, > - _t: PhantomData<T>, > + #[pin] > + data: T::RegistrationData, > } > > // SAFETY: It is allowed to call `misc_deregister` on a different thread from where you called > // `misc_register`. > -unsafe impl<T> Send for MiscDeviceRegistration<T> {} > +unsafe impl<T: MiscDevice> Send for MiscDeviceRegistration<T> where T::RegistrationData: Send {} > // SAFETY: All `&self` methods on this type are written to ensure that it is safe to call them in > -// parallel. > -unsafe impl<T> Sync for MiscDeviceRegistration<T> {} > +// parallel. The `RegistrationData` type is always `Sync`. > +unsafe impl<T: MiscDevice> Sync for MiscDeviceRegistration<T> {} > > impl<T: MiscDevice> MiscDeviceRegistration<T> { > /// Register a misc device. > - pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> { > + pub fn register( > + opts: MiscDeviceOptions, > + data: impl PinInit<T::RegistrationData, Error>, We can just skip the `RegistrationData` and use `T`? Best, Gary > + ) -> impl PinInit<Self, Error> { > try_pin_init!(Self { > + data <- data, > inner <- Opaque::try_ffi_init(move |slot: *mut bindings::miscdevice| { > // SAFETY: The initializer can write to the provided `slot`. > unsafe { slot.write(opts.into_raw::<T>()) }; > @@ -88,11 +96,14 @@ pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> { > // SAFETY: We just wrote the misc device options to the slot. The miscdevice will > // get unregistered before `slot` is deallocated because the memory is pinned and > // the destructor of this type deallocates the memory. > + // > + // The `data` field is `Sync + 'static`, so it's okay for the `open` callback to > + // access it until the destructor is invoked. > + // > // INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered > // misc device. > to_result(unsafe { bindings::misc_register(slot) }) > }), > - _t: PhantomData, > }) > } > > @@ -112,8 +123,16 @@ pub fn device(&self) -> &Device { > } > } > > +impl<T: MiscDevice> Deref for MiscDeviceRegistration<T> { > + type Target = T::RegistrationData; > + #[inline] > + fn deref(&self) -> &T::RegistrationData { > + &self.data > + } > +} > + > #[pinned_drop] > -impl<T> PinnedDrop for MiscDeviceRegistration<T> { > +impl<T: MiscDevice> PinnedDrop for MiscDeviceRegistration<T> { > fn drop(self: Pin<&mut Self>) { > // SAFETY: We know that the device is registered by the type invariants. > unsafe { bindings::misc_deregister(self.inner.get()) }; > @@ -126,6 +145,9 @@ pub trait MiscDevice: Sized { > /// What kind of pointer should `Self` be wrapped in. > type Ptr: ForeignOwnable + Send + Sync; > > + /// The registration data shared between all open files for this character device. > + type RegistrationData: Sync + 'static; > + > /// Called when the misc device is opened. > /// > /// The returned pointer will be stored as the private data for the file. > diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs > index 41e26c825060..0bde071743ef 100644 > --- a/samples/rust/rust_misc_device.rs > +++ b/samples/rust/rust_misc_device.rs > @@ -156,7 +156,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> { > }; > > try_pin_init!(Self { > - _miscdev <- MiscDeviceRegistration::register(options), > + _miscdev <- MiscDeviceRegistration::register(options, Ok(())), > }) > } > } > @@ -176,6 +176,7 @@ struct RustMiscDevice { > #[vtable] > impl MiscDevice for RustMiscDevice { > type Ptr = Pin<KBox<Self>>; > + type RegistrationData = (); > > fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> { > let dev = ARef::from(misc.device()); > > --- > base-commit: 220190f97da558e67cd01c62f1b84fe77b267a5a > change-id: 20260807-miscdevice-data-71c727e8b2e5 > > Best regards,