On 18.10.23 14:25, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
>
> Allow basic registration and unregistration of Rust file system types.
> Unregistration happens automatically when a registration variable is
> dropped (e.g., when it goes out of scope).
>
> File systems registered this way are visible in `/proc/filesystems` but
> cannot be mounted yet because `init_fs_context` fails.
>
> Signed-off-by: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/kernel/error.rs | 2 -
> rust/kernel/fs.rs | 80 +++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 4 files changed, 82 insertions(+), 2 deletions(-)
> create mode 100644 rust/kernel/fs.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3b620ae07021..9c23037b33d0 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -8,6 +8,7 @@
>
> #include <kunit/test.h>
> #include <linux/errname.h>
> +#include <linux/fs.h>
> #include <linux/slab.h>
> #include <linux/refcount.h>
> #include <linux/wait.h>
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 05fcab6abfe6..e6d7ce46be55 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -320,8 +320,6 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
> /// })
> /// }
> /// ```
> -// TODO: Remove `dead_code` marker once an in-kernel client is available.
> -#[allow(dead_code)]
> pub(crate) fn from_result<T, F>(f: F) -> T
> where
> T: From<i16>,
> diff --git a/rust/kernel/fs.rs b/rust/kernel/fs.rs
> new file mode 100644
> index 000000000000..f3fb09db41ba
> --- /dev/null
> +++ b/rust/kernel/fs.rs
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Kernel file systems.
> +//!
> +//! This module allows Rust code to register new kernel file systems.
> +//!
> +//! C headers: [`include/linux/fs.h`](../../include/linux/fs.h)
> +
> +use crate::error::{code::*, from_result, to_result, Error};
> +use crate::types::Opaque;
> +use crate::{bindings, init::PinInit, str::CStr, try_pin_init, ThisModule};
> +use core::{marker::PhantomPinned, pin::Pin};
> +use macros::{pin_data, pinned_drop};
> +
> +/// A file system type.
> +pub trait FileSystem {
> + /// The name of the file system type.
> + const NAME: &'static CStr;
> +}
> +
> +/// A registration of a file system.
> +#[pin_data(PinnedDrop)]
> +pub struct Registration {
> + #[pin]
> + fs: Opaque<bindings::file_system_type>,
> + #[pin]
> + _pin: PhantomPinned,
Note that since commit 0b4e3b6f6b79 ("rust: types: make `Opaque` be
`!Unpin`") you do not need an extra pinned `PhantomPinned` in your struct
(if you already have a pinned `Opaque`), since `Opaque` already is
`!Unpin`.
> +}
> +
> +// SAFETY: `Registration` doesn't provide any `&self` methods, so it is safe to pass references
> +// to it around.
> +unsafe impl Sync for Registration {}
> +
> +// SAFETY: Both registration and unregistration are implemented in C and safe to be performed
> +// from any thread, so `Registration` is `Send`.
> +unsafe impl Send for Registration {}
> +
> +impl Registration {
> + /// Creates the initialiser of a new file system registration.
> + pub fn new<T: FileSystem + ?Sized>(module: &'static ThisModule) -> impl PinInit<Self, Error> {
I am a bit curious why you specify `?Sized` here, is it common
for types that implement `FileSystem` to not be `Sized`?
Or do you want to use `dyn FileSystem`?
> + try_pin_init!(Self {
> + _pin: PhantomPinned,
> + fs <- Opaque::try_ffi_init(|fs_ptr: *mut bindings::file_system_type| {
> + // SAFETY: `try_ffi_init` guarantees that `fs_ptr` is valid for write.
> + unsafe { fs_ptr.write(bindings::file_system_type::default()) };
> +
> + // SAFETY: `try_ffi_init` guarantees that `fs_ptr` is valid for write, and it has
> + // just been initialised above, so it's also valid for read.
> + let fs = unsafe { &mut *fs_ptr };
> + fs.owner = module.0;
> + fs.name = T::NAME.as_char_ptr();
> + fs.init_fs_context = Some(Self::init_fs_context_callback);
> + fs.kill_sb = Some(Self::kill_sb_callback);
> + fs.fs_flags = 0;
> +
> + // SAFETY: Pointers stored in `fs` are static so will live for as long as the
> + // registration is active (it is undone in `drop`).
> + to_result(unsafe { bindings::register_filesystem(fs_ptr) })
> + }),
> + })
> + }
> +
> + unsafe extern "C" fn init_fs_context_callback(
> + _fc_ptr: *mut bindings::fs_context,
> + ) -> core::ffi::c_int {
> + from_result(|| Err(ENOTSUPP))
> + }
> +
> + unsafe extern "C" fn kill_sb_callback(_sb_ptr: *mut bindings::super_block) {}
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for Registration {
> + fn drop(self: Pin<&mut Self>) {
> + // SAFETY: If an instance of `Self` has been successfully created, a call to
> + // `register_filesystem` has necessarily succeeded. So it's ok to call
> + // `unregister_filesystem` on the previously registered fs.
I would simply add an invariant on `Registration` that `self.fs` is
registered, then you do not need such a lengthy explanation here.
--
Cheers,
Benno
> + unsafe { bindings::unregister_filesystem(self.fs.get()) };
> + }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 187d58f906a5..00059b80c240 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -34,6 +34,7 @@
> mod allocator;
> mod build_assert;
> pub mod error;
> +pub mod fs;
> pub mod init;
> pub mod ioctl;
> #[cfg(CONFIG_KUNIT)]
> --
> 2.34.1
>
>
