diff mbox series

[RFC,v2,06/10] rust: file: pass the filesystem context to the open function

Message ID 20230726164535.230515-7-amiculas@cisco.com (mailing list archive)
State New, archived
Headers show
Series Rust PuzleFS filesystem driver | expand

Commit Message

Ariel Miculas July 26, 2023, 4:45 p.m. UTC 
This allows us to create a Vfsmount structure and pass it to the read
callback.

Signed-off-by: Ariel Miculas <amiculas@cisco.com>
---
 rust/kernel/file.rs      | 17 +++++++++++++++--
 samples/rust/puzzlefs.rs | 40 +++++++++++++++++++++++++++++++++++-----
 samples/rust/rust_fs.rs  |  3 ++-
 3 files changed, 52 insertions(+), 8 deletions(-)

Comments

Ariel Miculas July 27, 2023, 1:32 p.m. UTC | #1 
On Wed, Jul 26, 2023 at 7:58 PM Ariel Miculas <amiculas@cisco.com> wrote:
>
> This allows us to create a Vfsmount structure and pass it to the read
> callback.
>
> Signed-off-by: Ariel Miculas <amiculas@cisco.com>
> ---
>  rust/kernel/file.rs      | 17 +++++++++++++++--
>  samples/rust/puzzlefs.rs | 40 +++++++++++++++++++++++++++++++++++-----
>  samples/rust/rust_fs.rs  |  3 ++-
>  3 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/rust/kernel/file.rs b/rust/kernel/file.rs
> index a3002c416dbb..af1eb1ee9267 100644
> --- a/rust/kernel/file.rs
> +++ b/rust/kernel/file.rs
> @@ -457,9 +457,15 @@ impl<A: OpenAdapter<T::OpenData>, T: Operations> OperationsVtable<A, T> {
>              // `fileref` never outlives this function, so it is guaranteed to be
>              // valid.
>              let fileref = unsafe { File::from_ptr(file) };
> +
> +            // SAFETY: into_foreign was called in fs::NewSuperBlock<..., NeedsInit>::init and
> +            // it is valid until from_foreign will be called in fs::Tables::free_callback
> +            let fs_info =
> +                unsafe { <T::Filesystem as fs::Type>::Data::borrow((*(*inode).i_sb).s_fs_info) };
> +
>              // SAFETY: `arg` was previously returned by `A::convert` and must
>              // be a valid non-null pointer.
> -            let ptr = T::open(unsafe { &*arg }, fileref)?.into_foreign();
> +            let ptr = T::open(fs_info, unsafe { &*arg }, fileref)?.into_foreign();
>              // SAFETY: The C contract guarantees that `private_data` is available
>              // for implementers of the file operations (no other C code accesses
>              // it), so we know that there are no concurrent threads/CPUs accessing
> @@ -930,10 +936,17 @@ pub trait Operations {
>      /// The type of the context data passed to [`Operations::open`].
>      type OpenData: Sync = ();
>
> +    /// Data associated with each file system instance.
> +    type Filesystem: fs::Type;
> +
>      /// Creates a new instance of this file.
>      ///
>      /// Corresponds to the `open` function pointer in `struct file_operations`.
> -    fn open(context: &Self::OpenData, file: &File) -> Result<Self::Data>;
> +    fn open(
> +        fs_info: <<Self::Filesystem as fs::Type>::Data as ForeignOwnable>::Borrowed<'_>,
> +        context: &Self::OpenData,
> +        file: &File,
> +    ) -> Result<Self::Data>;
>
>      /// Cleans up after the last reference to the file goes away.
>      ///
> diff --git a/samples/rust/puzzlefs.rs b/samples/rust/puzzlefs.rs
> index 9afd82745b64..8a64e0bd437d 100644
> --- a/samples/rust/puzzlefs.rs
> +++ b/samples/rust/puzzlefs.rs
> @@ -3,8 +3,14 @@
>  //! Rust file system sample.
>
>  use kernel::module_fs;
> +use kernel::mount::Vfsmount;
>  use kernel::prelude::*;
> -use kernel::{c_str, file, fs, io_buffer::IoBufferWriter};
> +use kernel::{
> +    c_str, file, fmt, fs,
> +    io_buffer::IoBufferWriter,
> +    str::CString,
> +    sync::{Arc, ArcBorrow},
> +};
>
>  mod puzzle;
>  // Required by the autogenerated '_capnp.rs' files
> @@ -19,6 +25,12 @@
>
>  struct PuzzleFsModule;
>
> +#[derive(Debug)]
> +struct PuzzlefsInfo {
> +    base_path: CString,
> +    vfs_mount: Arc<Vfsmount>,
> +}
> +
>  #[vtable]
>  impl fs::Context<Self> for PuzzleFsModule {
>      type Data = ();
> @@ -46,14 +58,23 @@ fn try_new() -> Result {
>  impl fs::Type for PuzzleFsModule {
>      type Context = Self;
>      type INodeData = &'static [u8];
> +    type Data = Box<PuzzlefsInfo>;
>      const SUPER_TYPE: fs::Super = fs::Super::Independent;
>      const NAME: &'static CStr = c_str!("puzzlefs");
>      const FLAGS: i32 = fs::flags::USERNS_MOUNT;
>      const DCACHE_BASED: bool = true;
>
>      fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBlock<Self>> {
> +        let base_path = CString::try_from_fmt(fmt!("hello world"))?;
> +        pr_info!("base_path {:?}\n", base_path);
> +        let vfs_mount = Vfsmount::new_private_mount(c_str!("/home/puzzlefs_oci"))?;
> +        pr_info!("vfs_mount {:?}\n", vfs_mount);
> +
>          let sb = sb.init(
> -            (),
> +            Box::try_new(PuzzlefsInfo {
> +                base_path,
> +                vfs_mount: Arc::try_new(vfs_mount)?,
> +            })?,
>              &fs::SuperParams {
>                  magic: 0x72757374,
>                  ..fs::SuperParams::DEFAULT
> @@ -88,14 +109,23 @@ fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBl
>
>  #[vtable]
>  impl file::Operations for FsFile {
> +    // must be the same as INodeData
>      type OpenData = &'static [u8];
> +    type Filesystem = PuzzleFsModule;
> +    // this is an Arc because Data must be ForeignOwnable and the only implementors of it are Box,
> +    // Arc and (); we cannot pass a reference to read, so we share Vfsmount using and Arc
> +    type Data = Arc<Vfsmount>;
>
> -    fn open(_context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
> -        Ok(())
> +    fn open(
> +        fs_info: &PuzzlefsInfo,
> +        _context: &Self::OpenData,
> +        _file: &file::File,
> +    ) -> Result<Self::Data> {
> +        Ok(fs_info.vfs_mount.clone())
>      }
>
>      fn read(
> -        _data: (),
> +        data: ArcBorrow<'_, Vfsmount>,
>          file: &file::File,
>          writer: &mut impl IoBufferWriter,
>          offset: u64,
> diff --git a/samples/rust/rust_fs.rs b/samples/rust/rust_fs.rs
> index 7527681ee024..c58ed1560e06 100644
> --- a/samples/rust/rust_fs.rs
> +++ b/samples/rust/rust_fs.rs
> @@ -85,8 +85,9 @@ fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBl
>  #[vtable]
>  impl file::Operations for FsFile {
>      type OpenData = &'static [u8];
> +    type Filesystem = RustFs;
>
> -    fn open(_context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
> +    fn open(_fs_info: (), _context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
>          Ok(())
>      }
>
> --
> 2.41.0
>
>
Hey Wedson,

Is it ok to couple file::Operations with fs::Type? I didn't find a
better way to implement this.
I'm asking because I've seen you've gone to great lengths to decouple them.

Cheers,
Ariel
diff mbox series

Patch

diff --git a/rust/kernel/file.rs b/rust/kernel/file.rs
index a3002c416dbb..af1eb1ee9267 100644
--- a/rust/kernel/file.rs
+++ b/rust/kernel/file.rs
@@ -457,9 +457,15 @@  impl<A: OpenAdapter<T::OpenData>, T: Operations> OperationsVtable<A, T> {
             // `fileref` never outlives this function, so it is guaranteed to be
             // valid.
             let fileref = unsafe { File::from_ptr(file) };
+
+            // SAFETY: into_foreign was called in fs::NewSuperBlock<..., NeedsInit>::init and
+            // it is valid until from_foreign will be called in fs::Tables::free_callback
+            let fs_info =
+                unsafe { <T::Filesystem as fs::Type>::Data::borrow((*(*inode).i_sb).s_fs_info) };
+
             // SAFETY: `arg` was previously returned by `A::convert` and must
             // be a valid non-null pointer.
-            let ptr = T::open(unsafe { &*arg }, fileref)?.into_foreign();
+            let ptr = T::open(fs_info, unsafe { &*arg }, fileref)?.into_foreign();
             // SAFETY: The C contract guarantees that `private_data` is available
             // for implementers of the file operations (no other C code accesses
             // it), so we know that there are no concurrent threads/CPUs accessing
@@ -930,10 +936,17 @@  pub trait Operations {
     /// The type of the context data passed to [`Operations::open`].
     type OpenData: Sync = ();
 
+    /// Data associated with each file system instance.
+    type Filesystem: fs::Type;
+
     /// Creates a new instance of this file.
     ///
     /// Corresponds to the `open` function pointer in `struct file_operations`.
-    fn open(context: &Self::OpenData, file: &File) -> Result<Self::Data>;
+    fn open(
+        fs_info: <<Self::Filesystem as fs::Type>::Data as ForeignOwnable>::Borrowed<'_>,
+        context: &Self::OpenData,
+        file: &File,
+    ) -> Result<Self::Data>;
 
     /// Cleans up after the last reference to the file goes away.
     ///
diff --git a/samples/rust/puzzlefs.rs b/samples/rust/puzzlefs.rs
index 9afd82745b64..8a64e0bd437d 100644
--- a/samples/rust/puzzlefs.rs
+++ b/samples/rust/puzzlefs.rs
@@ -3,8 +3,14 @@ 
 //! Rust file system sample.
 
 use kernel::module_fs;
+use kernel::mount::Vfsmount;
 use kernel::prelude::*;
-use kernel::{c_str, file, fs, io_buffer::IoBufferWriter};
+use kernel::{
+    c_str, file, fmt, fs,
+    io_buffer::IoBufferWriter,
+    str::CString,
+    sync::{Arc, ArcBorrow},
+};
 
 mod puzzle;
 // Required by the autogenerated '_capnp.rs' files
@@ -19,6 +25,12 @@ 
 
 struct PuzzleFsModule;
 
+#[derive(Debug)]
+struct PuzzlefsInfo {
+    base_path: CString,
+    vfs_mount: Arc<Vfsmount>,
+}
+
 #[vtable]
 impl fs::Context<Self> for PuzzleFsModule {
     type Data = ();
@@ -46,14 +58,23 @@  fn try_new() -> Result {
 impl fs::Type for PuzzleFsModule {
     type Context = Self;
     type INodeData = &'static [u8];
+    type Data = Box<PuzzlefsInfo>;
     const SUPER_TYPE: fs::Super = fs::Super::Independent;
     const NAME: &'static CStr = c_str!("puzzlefs");
     const FLAGS: i32 = fs::flags::USERNS_MOUNT;
     const DCACHE_BASED: bool = true;
 
     fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBlock<Self>> {
+        let base_path = CString::try_from_fmt(fmt!("hello world"))?;
+        pr_info!("base_path {:?}\n", base_path);
+        let vfs_mount = Vfsmount::new_private_mount(c_str!("/home/puzzlefs_oci"))?;
+        pr_info!("vfs_mount {:?}\n", vfs_mount);
+
         let sb = sb.init(
-            (),
+            Box::try_new(PuzzlefsInfo {
+                base_path,
+                vfs_mount: Arc::try_new(vfs_mount)?,
+            })?,
             &fs::SuperParams {
                 magic: 0x72757374,
                 ..fs::SuperParams::DEFAULT
@@ -88,14 +109,23 @@  fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBl
 
 #[vtable]
 impl file::Operations for FsFile {
+    // must be the same as INodeData
     type OpenData = &'static [u8];
+    type Filesystem = PuzzleFsModule;
+    // this is an Arc because Data must be ForeignOwnable and the only implementors of it are Box,
+    // Arc and (); we cannot pass a reference to read, so we share Vfsmount using and Arc
+    type Data = Arc<Vfsmount>;
 
-    fn open(_context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
-        Ok(())
+    fn open(
+        fs_info: &PuzzlefsInfo,
+        _context: &Self::OpenData,
+        _file: &file::File,
+    ) -> Result<Self::Data> {
+        Ok(fs_info.vfs_mount.clone())
     }
 
     fn read(
-        _data: (),
+        data: ArcBorrow<'_, Vfsmount>,
         file: &file::File,
         writer: &mut impl IoBufferWriter,
         offset: u64,
diff --git a/samples/rust/rust_fs.rs b/samples/rust/rust_fs.rs
index 7527681ee024..c58ed1560e06 100644
--- a/samples/rust/rust_fs.rs
+++ b/samples/rust/rust_fs.rs
@@ -85,8 +85,9 @@  fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBl
 #[vtable]
 impl file::Operations for FsFile {
     type OpenData = &'static [u8];
+    type Filesystem = RustFs;
 
-    fn open(_context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
+    fn open(_fs_info: (), _context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
         Ok(())
     }