Skip to content

Add registration struct #1

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

Open
wants to merge 1 commit into
base: basic-fs-abstractions
Choose a base branch
from
Open
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
60 changes: 59 additions & 1 deletion rust/kernel/fs/file_system.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
use core::ptr;
use core::marker::PhantomData;
use core::pin::Pin;
use core::str::from_utf8;

use alloc::boxed::Box;

use crate::{
bindings, c_types::*, error::KernelResultExt, fs::super_block::SuperBlock, str::CStr,
types::FileSystemFlags, Result,
types::FileSystemFlags, Result, pr_warn, Error
};

pub type FileSystemType = bindings::file_system_type;
Expand Down Expand Up @@ -99,3 +104,56 @@ pub const DEFAULT_FS_TYPE: bindings::file_system_type = bindings::file_system_ty
i_mutex_key: bindings::lock_class_key {},
i_mutex_dir_key: bindings::lock_class_key {},
};

pub struct Registration<T: FileSystemBase> {
phantom: PhantomData<T>,
fs_type: FileSystemType,
}

//Pin self
Copy link
Owner

Choose a reason for hiding this comment

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

Does this mean anything?

Suggested change
//Pin self

impl<T: FileSystemBase> Registration<T> {
fn new(fs_type: FileSystemType ) -> Self {
Self {
phantom: PhantomData,
fs_type: fs_type,
}
}

pub fn new_pinned() -> Result<Pin<Box<Self>>> {
let mut c_fs_type = FileSystemType::default(); // may use DEFAULT_FS_TYPE?
Copy link
Owner

Choose a reason for hiding this comment

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

The Default impl just returns mem::zeroed(). I am unsure whether we would want to make that more explicit - then again, this is what C code would do. I don't know if there are any safety concerns here, but that is something that we can leave for the upstream people to decide. Until then, default seems fine.

c_fs_type.mount = Some(mount_callback::<T>);
c_fs_type.kill_sb = Some(kill_superblock_callback::<T>);
c_fs_type.owner = T::OWNER;
c_fs_type.name = T::NAME.as_char_ptr();
c_fs_type.fs_flags = T::FS_FLAGS.into_int();

Ok(Pin::from(Box::try_new(Self::new(c_fs_type))?))
}

pub fn register(&mut self) -> Result {
Copy link
Owner

Choose a reason for hiding this comment

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

We really want to take self: Pin<&mut Self> here

let err = unsafe { bindings::register_filesystem(&mut self.fs_type) };
if err != 0 {
return Err(Error::from_kernel_errno(err));
}

Ok(())
}

fn unregister(&mut self) -> Result {
Copy link
Owner

Choose a reason for hiding this comment

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

Same here I think - this means that the Drop impl has to be implemented similar to https://doc.rust-lang.org/std/pin/index.html#drop-implementation - but that's a good thing in my opinion :)

let err = unsafe { bindings::unregister_filesystem(&mut self.fs_type) };
if err != 0 {
return Err(Error::from_kernel_errno(err));
}

Ok(())
}
}

impl<T: FileSystemBase> Drop for Registration<T> {
fn drop(&mut self) {
if let Err(_) = self.unregister() {
let fs_name = from_utf8(T::NAME.as_bytes()).unwrap();
pr_warn!("Unregister filesystem {} failed", fs_name);
}
}
}