On Wed, Oct 18, 2023 at 09:25:04AM -0300, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
>
> Allow Rust file systems to handle typed and ref-counted inodes.
>
> This is in preparation for creating new inodes (for example, to create
> the root inode of a new superblock), which comes in the next patch in
> the series.
>
> Signed-off-by: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
> ---
> rust/helpers.c | 7 +++++++
> rust/kernel/fs.rs | 53 +++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index 4c86fe4a7e05..fe45f8ddb31f 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -25,6 +25,7 @@
> #include <linux/build_bug.h>
> #include <linux/err.h>
> #include <linux/errname.h>
> +#include <linux/fs.h>
> #include <linux/mutex.h>
> #include <linux/refcount.h>
> #include <linux/sched/signal.h>
> @@ -144,6 +145,12 @@ struct kunit *rust_helper_kunit_get_current_test(void)
> }
> EXPORT_SYMBOL_GPL(rust_helper_kunit_get_current_test);
>
> +off_t rust_helper_i_size_read(const struct inode *inode)
i_size_read returns a loff_t (aka __kernel_loff_t (aka long long)),
but this returns an off_t (aka __kernel_off_t (aka long)).
Won't that cause truncation issues for files larger than 4GB on some
architectures?
> +{
> + return i_size_read(inode);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_i_size_read);
> +
> /*
> * `bindgen` binds the C `size_t` type as the Rust `usize` type, so we can
> * use it in contexts where Rust expects a `usize` like slice (array) indices.
> diff --git a/rust/kernel/fs.rs b/rust/kernel/fs.rs
> index 31cf643aaded..30fa1f312f33 100644
> --- a/rust/kernel/fs.rs
> +++ b/rust/kernel/fs.rs
> @@ -7,9 +7,9 @@
> //! C headers: [`include/linux/fs.h`](../../include/linux/fs.h)
>
> use crate::error::{code::*, from_result, to_result, Error, Result};
> -use crate::types::Opaque;
> +use crate::types::{AlwaysRefCounted, Opaque};
> use crate::{bindings, init::PinInit, str::CStr, try_pin_init, ThisModule};
> -use core::{marker::PhantomData, marker::PhantomPinned, pin::Pin};
> +use core::{marker::PhantomData, marker::PhantomPinned, pin::Pin, ptr};
> use macros::{pin_data, pinned_drop};
>
> /// Maximum size of an inode.
> @@ -94,6 +94,55 @@ fn drop(self: Pin<&mut Self>) {
> }
> }
>
> +/// The number of an inode.
> +pub type Ino = u64;
> +
> +/// A node in the file system index (inode).
> +///
> +/// Wraps the kernel's `struct inode`.
> +///
> +/// # Invariants
> +///
> +/// Instances of this type are always ref-counted, that is, a call to `ihold` ensures that the
> +/// allocation remains valid at least until the matching call to `iput`.
> +#[repr(transparent)]
> +pub struct INode<T: FileSystem + ?Sized>(Opaque<bindings::inode>, PhantomData<T>);
> +
> +impl<T: FileSystem + ?Sized> INode<T> {
> + /// Returns the number of the inode.
> + pub fn ino(&self) -> Ino {
> + // SAFETY: `i_ino` is immutable, and `self` is guaranteed to be valid by the existence of a
> + // shared reference (&self) to it.
> + unsafe { (*self.0.get()).i_ino }
Is "*self.0.get()" the means by which the Rust bindings get at the
actual C object?
(Forgive me, I've barely finished drying the primer coat on my rust-fu.)
> + }
> +
> + /// Returns the super-block that owns the inode.
> + pub fn super_block(&self) -> &SuperBlock<T> {
> + // SAFETY: `i_sb` is immutable, and `self` is guaranteed to be valid by the existence of a
> + // shared reference (&self) to it.
> + unsafe { &*(*self.0.get()).i_sb.cast() }
> + }
> +
> + /// Returns the size of the inode contents.
> + pub fn size(&self) -> i64 {
I'm a little surprised I didn't see a
pub type loff_t = i64
followed by this function returning a loff_t. Or maybe it would be
better to define it as:
struct loff_t(i64);
So that dopey fs developers like me cannot so easily assign a file
position (bytes) to a pgoff_t (page index) without either supplying an
actual conversion operator or seeing complaints from the compiler.
> + // SAFETY: `self` is guaranteed to be valid by the existence of a shared reference.
> + unsafe { bindings::i_size_read(self.0.get()) }
It's confusing that rust_i_size_read returns a long but on the rust side,
INode::size returns an i64.
--D
> + }
> +}
> +
> +// SAFETY: The type invariants guarantee that `INode` is always ref-counted.
> +unsafe impl<T: FileSystem + ?Sized> AlwaysRefCounted for INode<T> {
> + fn inc_ref(&self) {
> + // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> + unsafe { bindings::ihold(self.0.get()) };
> + }
> +
> + unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> + // SAFETY: The safety requirements guarantee that the refcount is nonzero.
> + unsafe { bindings::iput(obj.cast().as_ptr()) }
> + }
> +}
> +
> /// A file system super block.
> ///
> /// Wraps the kernel's `struct super_block`.
> --
> 2.34.1
>
>
