2023-07-26 17:32:52

by Ariel Miculas

[permalink] [raw]
Subject: [RFC PATCH v2 06/10] rust: file: pass the filesystem context to the open function

This allows us to create a Vfsmount structure and pass it to the read
callback.

Signed-off-by: Ariel Miculas <[email protected]>
---
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



2023-07-27 13:59:00

by Ariel Miculas

[permalink] [raw]
Subject: Re: [RFC PATCH v2 06/10] rust: file: pass the filesystem context to the open function

On Wed, Jul 26, 2023 at 7:58 PM Ariel Miculas <[email protected]> wrote:
>
> This allows us to create a Vfsmount structure and pass it to the read
> callback.
>
> Signed-off-by: Ariel Miculas <[email protected]>
> ---
> 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