Merge pull request #145 from wedsonaf/file-open

Allow files to share state.

Author: ojeda

drivers/char/rust_example.rs

@@ -6,14 +6,16 @@
 #![feature(allocator_api, global_asm)]
 #![feature(test)]
 
-use alloc::boxed::Box;
+use alloc::{boxed::Box, sync::Arc};
 use core::pin::Pin;
 use kernel::prelude::*;
 use kernel::{
     chrdev, condvar_init, cstr,
-    file_operations::FileOperations,
+    file_operations::{File, FileOpener, FileOperations},
     miscdev, mutex_init, spinlock_init,
     sync::{CondVar, Mutex, SpinLock},
+    user_ptr::{UserSlicePtrReader, UserSlicePtrWriter},
+    Error,
 };
 
 module! {
@@ -51,23 +53,120 @@ module! {
     },
 }
 
-struct RustFile;
+const MAX_TOKENS: usize = 3;
 
-impl FileOperations for RustFile {
+struct SharedStateInner {
+    token_count: usize,
+}
+
+struct SharedState {
+    state_changed: CondVar,
+    inner: Mutex<SharedStateInner>,
+}
+
+impl SharedState {
+    fn try_new() -> KernelResult<Arc<Self>> {
+        let state = Arc::try_new(Self {
+            // SAFETY: `condvar_init!` is called below.
+            state_changed: unsafe { CondVar::new() },
+            // SAFETY: `mutex_init!` is called below.
+            inner: unsafe { Mutex::new(SharedStateInner { token_count: 0 }) },
+        })?;
+        // SAFETY: `state_changed` is pinned behind `Arc`.
+        let state_changed = unsafe { Pin::new_unchecked(&state.state_changed) };
+        kernel::condvar_init!(state_changed, "SharedState::state_changed");
+        // SAFETY: `inner` is pinned behind `Arc`.
+        let inner = unsafe { Pin::new_unchecked(&state.inner) };
+        kernel::mutex_init!(inner, "SharedState::inner");
+        Ok(state)
+    }
+}
+
+struct Token {
+    shared: Arc<SharedState>,
+}
+
+impl FileOpener<Arc<SharedState>> for Token {
+    fn open(shared: &Arc<SharedState>) -> KernelResult<Self::Wrapper> {
+        Ok(Box::try_new(Self {
+            shared: shared.clone(),
+        })?)
+    }
+}
+
+impl FileOperations for Token {
     type Wrapper = Box<Self>;
 
-    kernel::declare_file_operations!();
+    kernel::declare_file_operations!(read, write);
+
+    fn read(&self, _: &File, data: &mut UserSlicePtrWriter, offset: u64) -> KernelResult<usize> {
+        // Succeed if the caller doesn't provide a buffer or if not at the start.
+        if data.is_empty() || offset != 0 {
+            return Ok(0);
+        }
+
+        {
+            let mut inner = self.shared.inner.lock();
+
+            // Wait until we are allowed to decrement the token count or a signal arrives.
+            while inner.token_count == 0 {
+                if self.shared.state_changed.wait(&mut inner) {
+                    return Err(Error::EINTR);
+                }
+            }
+
+            // Consume a token.
+            inner.token_count -= 1;
+        }
+
+        // Notify a possible writer waiting.
+        self.shared.state_changed.notify_all();
+
+        // Write a one-byte 1 to the reader.
+        data.write_slice(&[1u8; 1])?;
+        Ok(1)
+    }
+
+    fn write(&self, data: &mut UserSlicePtrReader, _offset: u64) -> KernelResult<usize> {
+        {
+            let mut inner = self.shared.inner.lock();
 
-    fn open() -> KernelResult<Self::Wrapper> {
+            // Wait until we are allowed to increment the token count or a signal arrives.
+            while inner.token_count == MAX_TOKENS {
+                if self.shared.state_changed.wait(&mut inner) {
+                    return Err(Error::EINTR);
+                }
+            }
+
+            // Increment the number of token so that a reader can be released.
+            inner.token_count += 1;
+        }
+
+        // Notify a possible reader waiting.
+        self.shared.state_changed.notify_all();
+        Ok(data.len())
+    }
+}
+
+struct RustFile;
+
+impl FileOpener<()> for RustFile {
+    fn open(_ctx: &()) -> KernelResult<Self::Wrapper> {
         println!("rust file was opened!");
         Ok(Box::try_new(Self)?)
     }
 }
 
+impl FileOperations for RustFile {
+    type Wrapper = Box<Self>;
+
+    kernel::declare_file_operations!();
+}
+
 struct RustExample {
     message: String,
     _chrdev: Pin<Box<chrdev::Registration<2>>>,
-    _dev: Pin<Box<miscdev::Registration>>,
+    _dev: Pin<Box<miscdev::Registration<Arc<SharedState>>>>,
 }
 
 impl KernelModule for RustExample {
@@ -146,9 +245,11 @@ impl KernelModule for RustExample {
         chrdev_reg.as_mut().register::<RustFile>()?;
         chrdev_reg.as_mut().register::<RustFile>()?;
 
+        let state = SharedState::try_new()?;
+
         Ok(RustExample {
             message: "on the heap!".to_owned(),
-            _dev: miscdev::Registration::new_pinned::<RustFile>(cstr!("rust_miscdev"), None, ())?,
+            _dev: miscdev::Registration::new_pinned::<Token>(cstr!("rust_miscdev"), None, state)?,
             _chrdev: chrdev_reg,
         })
     }

rust/kernel/chrdev.rs

@@ -78,7 +78,7 @@ impl<const N: usize> Registration<{ N }> {
     /// Registers a character device.
     ///
     /// You may call this once per device type, up to `N` times.
-    pub fn register<T: file_operations::FileOperations>(self: Pin<&mut Self>) -> KernelResult {
+    pub fn register<T: file_operations::FileOpener<()>>(self: Pin<&mut Self>) -> KernelResult {
         // SAFETY: We must ensure that we never move out of `this`.
         let this = unsafe { self.get_unchecked_mut() };
         if this.inner.is_none() {
@@ -112,7 +112,12 @@ impl<const N: usize> Registration<{ N }> {
         // SAFETY: Calling unsafe functions and manipulating `MaybeUninit`
         // pointer.
         unsafe {
-            bindings::cdev_init(cdev, file_operations::FileOperationsVtable::<T>::build());
+            bindings::cdev_init(
+                cdev,
+                // SAFETY: The adapter doesn't retrieve any state yet, so it's compatible with any
+                // registration.
+                file_operations::FileOperationsVtable::<Self, T>::build(),
+            );
             (*cdev).owner = this.this_module.0;
             let rc = bindings::cdev_add(cdev, inner.dev + inner.used as bindings::dev_t, 1);
             if rc != 0 {
@@ -124,6 +129,19 @@ impl<const N: usize> Registration<{ N }> {
     }
 }
 
+impl<const N: usize> file_operations::FileOpenAdapter for Registration<{ N }> {
+    type Arg = ();
+
+    unsafe fn convert(
+        _inode: *mut bindings::inode,
+        _file: *mut bindings::file,
+    ) -> *const Self::Arg {
+        // TODO: Update the SAFETY comment on the call to `FileOperationsVTable::build` above once
+        // this is updated to retrieve state.
+        &()
+    }
+}
+
 // SAFETY: `Registration` does not expose any of its state across threads
 // (it is fine for multiple threads to have a shared reference to it).
 unsafe impl<const N: usize> Sync for Registration<{ N }> {}

rust/kernel/error.rs

@@ -45,6 +45,9 @@ impl Error {
     /// No such file or directory.
     pub const ENOENT: Self = Error(-(bindings::ENOENT as i32));
 
+    /// Interrupted system call.
+    pub const EINTR: Self = Error(-(bindings::EINTR as i32));
+
     /// Creates an [`Error`] from a kernel error code.
     pub fn from_kernel_errno(errno: c_types::c_int) -> Error {
         Error(errno)

rust/kernel/file_operations.rs

@@ -82,12 +82,13 @@ macro_rules! from_kernel_result {
     }};
 }
 
-unsafe extern "C" fn open_callback<T: FileOperations>(
-    _inode: *mut bindings::inode,
+unsafe extern "C" fn open_callback<A: FileOpenAdapter, T: FileOpener<A::Arg>>(
+    inode: *mut bindings::inode,
     file: *mut bindings::file,
 ) -> c_types::c_int {
     from_kernel_result! {
-        let ptr = T::open()?.into_pointer();
+        let arg = A::convert(inode, file);
+        let ptr = T::open(&*arg)?.into_pointer();
         (*file).private_data = ptr as *mut c_types::c_void;
         Ok(0)
     }
@@ -198,11 +199,11 @@ unsafe extern "C" fn fsync_callback<T: FileOperations>(
     }
 }
 
-pub(crate) struct FileOperationsVtable<T>(marker::PhantomData<T>);
+pub(crate) struct FileOperationsVtable<A, T>(marker::PhantomData<A>, marker::PhantomData<T>);
 
-impl<T: FileOperations> FileOperationsVtable<T> {
+impl<A: FileOpenAdapter, T: FileOpener<A::Arg>> FileOperationsVtable<A, T> {
     const VTABLE: bindings::file_operations = bindings::file_operations {
-        open: Some(open_callback::<T>),
+        open: Some(open_callback::<A, T>),
         release: Some(release_callback::<T>),
         read: if T::TO_USE.read {
             Some(read_callback::<T>)
@@ -262,7 +263,11 @@ impl<T: FileOperations> FileOperationsVtable<T> {
     };
 
     /// Builds an instance of [`struct file_operations`].
-    pub(crate) const fn build() -> &'static bindings::file_operations {
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the adapter is compatible with the way the device is registered.
+    pub(crate) const unsafe fn build() -> &'static bindings::file_operations {
         &Self::VTABLE
     }
 }
@@ -400,6 +405,39 @@ impl IoctlCommand {
     }
 }
 
+/// Trait for extracting file open arguments from kernel data structures.
+///
+/// This is meant to be implemented by registration managers.
+pub trait FileOpenAdapter {
+    /// The type of argument this adapter extracts.
+    type Arg;
+
+    /// Converts untyped data stored in [`struct inode`] and [`struct file`] (when [`struct
+    /// file_operations::open`] is called) into the given type. For example, for `miscdev`
+    /// devices, a pointer to the registered [`struct miscdev`] is stored in [`struct
+    /// file::private_data`].
+    ///
+    /// # Safety
+    ///
+    /// This function must be called only when [`struct file_operations::open`] is being called for
+    /// a file that was registered by the implementer.
+    unsafe fn convert(_inode: *mut bindings::inode, _file: *mut bindings::file)
+        -> *const Self::Arg;
+}
+
+/// Trait for implementers of kernel files.
+///
+/// In addition to the methods in [`FileOperations`], implementers must also provide
+/// [`FileOpener::open`] with a customised argument. This allows a single implementation of
+/// [`FileOperations`] to be used for different types of registrations, for example, `miscdev` and
+/// `chrdev`.
+pub trait FileOpener<T: ?Sized>: FileOperations {
+    /// Creates a new instance of this file.
+    ///
+    /// Corresponds to the `open` function pointer in `struct file_operations`.
+    fn open(context: &T) -> KernelResult<Self::Wrapper>;
+}
+
 /// Corresponds to the kernel's `struct file_operations`.
 ///
 /// You implement this trait whenever you would create a `struct file_operations`.
@@ -414,11 +452,6 @@ pub trait FileOperations: Send + Sync + Sized {
     /// The pointer type that will be used to hold ourselves.
     type Wrapper: PointerWrapper<Self>;
 
-    /// Creates a new instance of this file.
-    ///
-    /// Corresponds to the `open` function pointer in `struct file_operations`.
-    fn open() -> KernelResult<Self::Wrapper>;
-
     /// Cleans up after the last reference to the file goes away.
     ///
     /// Note that the object is moved, so it will be freed automatically unless the implementation

rust/kernel/miscdev.rs

@@ -7,7 +7,7 @@
 //! Reference: <https://www.kernel.org/doc/html/latest/driver-api/misc_devices.html>
 
 use crate::error::{Error, KernelResult};
-use crate::file_operations::{FileOperations, FileOperationsVtable};
+use crate::file_operations::{FileOpenAdapter, FileOpener, FileOperationsVtable};
 use crate::{bindings, c_types, CStr};
 use alloc::boxed::Box;
 use core::marker::PhantomPinned;
@@ -19,7 +19,8 @@ pub struct Registration<T: Sync = ()> {
     mdev: bindings::miscdevice,
     _pin: PhantomPinned,
 
-    /// Context initialised on construction.
+    /// Context initialised on construction and made available to all file instances on
+    /// [`FileOpener::open`].
     pub context: T,
 }
 
@@ -39,7 +40,7 @@ impl<T: Sync> Registration<T> {
     /// Registers a miscellaneous device.
     ///
     /// Returns a pinned heap-allocated representation of the registration.
-    pub fn new_pinned<F: FileOperations>(
+    pub fn new_pinned<F: FileOpener<T>>(
         name: CStr<'static>,
         minor: Option<i32>,
         context: T,
@@ -53,7 +54,7 @@ impl<T: Sync> Registration<T> {
     ///
     /// 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<F: FileOperations>(
+    pub fn register<F: FileOpener<T>>(
         self: Pin<&mut Self>,
         name: CStr<'static>,
         minor: Option<i32>,
@@ -65,7 +66,8 @@ impl<T: Sync> Registration<T> {
             return Err(Error::EINVAL);
         }
 
-        this.mdev.fops = FileOperationsVtable::<F>::build();
+        // SAFETY: The adapter is compatible with `misc_register`.
+        this.mdev.fops = unsafe { FileOperationsVtable::<Self, 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);
 
@@ -78,6 +80,15 @@ impl<T: Sync> Registration<T> {
     }
 }
 
+impl<T: Sync> FileOpenAdapter for Registration<T> {
+    type Arg = T;
+
+    unsafe fn convert(_inode: *mut bindings::inode, file: *mut bindings::file) -> *const Self::Arg {
+        let reg = crate::container_of!((*file).private_data, Self, mdev);
+        &(*reg).context
+    }
+}
+
 // 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,
 // except maybe through `Registration::context`, but it is itself `Sync`.