diff mbox series

[RFC,12/19] rust: fs: introduce `FileSystem::statfs`

Message ID 20231018122518.128049-13-wedsonaf@gmail.com (mailing list archive)
State New, archived
Headers show
Series Rust abstractions for VFS | expand

Commit Message

Wedson Almeida Filho Oct. 18, 2023, 12:25 p.m. UTC 
From: Wedson Almeida Filho <walmeida@microsoft.com>

Allow Rust file systems to expose their stats. `overlayfs` requires that
this be implemented by all file systems that are part of an overlay.
The planned file systems need to be overlayed with overlayfs, so they
must be able to implement this.

Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/error.rs            |  1 +
 rust/kernel/fs.rs               | 52 ++++++++++++++++++++++++++++++++-
 3 files changed, 53 insertions(+), 1 deletion(-)

Comments

Andreas Hindborg Jan. 3, 2024, 2:13 p.m. UTC | #1 
Wedson Almeida Filho <wedsonaf@gmail.com> writes:

[...]

> +    unsafe extern "C" fn statfs_callback(
> +        dentry: *mut bindings::dentry,
> +        buf: *mut bindings::kstatfs,
> +    ) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: The C API guarantees that `dentry` is valid for read. `d_sb` is
> +            // immutable, so it's safe to read it. The superblock is guaranteed to be valid dor

"valid dor" -> "valid for"

BR Andreas
Darrick J. Wong Jan. 4, 2024, 5:33 a.m. UTC | #2 
On Wed, Oct 18, 2023 at 09:25:11AM -0300, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> Allow Rust file systems to expose their stats. `overlayfs` requires that
> this be implemented by all file systems that are part of an overlay.
> The planned file systems need to be overlayed with overlayfs, so they
> must be able to implement this.
> 
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> ---
>  rust/bindings/bindings_helper.h |  1 +
>  rust/kernel/error.rs            |  1 +
>  rust/kernel/fs.rs               | 52 ++++++++++++++++++++++++++++++++-
>  3 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index fa754c5e85a2..e2b2ccc835e3 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -11,6 +11,7 @@
>  #include <linux/fs.h>
>  #include <linux/fs_context.h>
>  #include <linux/slab.h>
> +#include <linux/statfs.h>
>  #include <linux/pagemap.h>
>  #include <linux/refcount.h>
>  #include <linux/wait.h>
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 6c167583b275..829756cf6c48 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -83,6 +83,7 @@ macro_rules! declare_err {
>      declare_err!(ENOGRACE, "NFS file lock reclaim refused.");
>      declare_err!(ENODATA, "No data available.");
>      declare_err!(EOPNOTSUPP, "Operation not supported on transport endpoint.");
> +    declare_err!(ENOSYS, "Invalid system call number.");
>  }
>  
>  /// Generic integer kernel error.
> diff --git a/rust/kernel/fs.rs b/rust/kernel/fs.rs
> index adf9cbee16d2..8f34da50e694 100644
> --- a/rust/kernel/fs.rs
> +++ b/rust/kernel/fs.rs
> @@ -50,6 +50,31 @@ pub trait FileSystem {
>      fn read_xattr(_inode: &INode<Self>, _name: &CStr, _outbuf: &mut [u8]) -> Result<usize> {
>          Err(EOPNOTSUPP)
>      }
> +
> +    /// Get filesystem statistics.
> +    fn statfs(_sb: &SuperBlock<Self>) -> Result<Stat> {
> +        Err(ENOSYS)
> +    }
> +}
> +
> +/// File system stats.
> +///
> +/// A subset of C's `kstatfs`.
> +pub struct Stat {
> +    /// Magic number of the file system.
> +    pub magic: u32,
> +
> +    /// The maximum length of a file name.
> +    pub namelen: i64,

Yikes, I hope I never see an 8EB filename.  The C side doesn't handle
names longer than 255 bytes.

> +
> +    /// Block size.
> +    pub bsize: i64,

Or an 8EB block size.  SMR notwithstanding, I think this could be u32.

Why are these values signed?  Nobody has a -1k block filesystem.

> +    /// Number of files in the file system.
> +    pub files: u64,
> +
> +    /// Number of blocks in the file system.
> +    pub blocks: u64,
>  }
>  
>  /// The types of directory entries reported by [`FileSystem::read_dir`].
> @@ -478,7 +503,7 @@ impl<T: FileSystem + ?Sized> Tables<T> {
>          freeze_fs: None,
>          thaw_super: None,
>          unfreeze_fs: None,
> -        statfs: None,
> +        statfs: Some(Self::statfs_callback),
>          remount_fs: None,
>          umount_begin: None,
>          show_options: None,
> @@ -496,6 +521,31 @@ impl<T: FileSystem + ?Sized> Tables<T> {
>          shutdown: None,
>      };
>  
> +    unsafe extern "C" fn statfs_callback(
> +        dentry: *mut bindings::dentry,
> +        buf: *mut bindings::kstatfs,
> +    ) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: The C API guarantees that `dentry` is valid for read. `d_sb` is
> +            // immutable, so it's safe to read it. The superblock is guaranteed to be valid dor
> +            // the duration of the call.
> +            let sb = unsafe { &*(*dentry).d_sb.cast::<SuperBlock<T>>() };
> +            let s = T::statfs(sb)?;
> +
> +            // SAFETY: The C API guarantees that `buf` is valid for read and write.
> +            let buf = unsafe { &mut *buf };
> +            buf.f_type = s.magic.into();
> +            buf.f_namelen = s.namelen;
> +            buf.f_bsize = s.bsize;
> +            buf.f_files = s.files;
> +            buf.f_blocks = s.blocks;
> +            buf.f_bfree = 0;
> +            buf.f_bavail = 0;
> +            buf.f_ffree = 0;

Why is it necessary to fill out the C structure with zeroes?
statfs_by_dentry zeroes the buffer contents before calling ->statfs.

--D

> +            Ok(0)
> +        })
> +    }
> +
>      const XATTR_HANDLERS: [*const bindings::xattr_handler; 2] = [&Self::XATTR_HANDLER, ptr::null()];
>  
>      const XATTR_HANDLER: bindings::xattr_handler = bindings::xattr_handler {
> -- 
> 2.34.1
> 
>
Wedson Almeida Filho Jan. 24, 2024, 4:24 a.m. UTC | #3 
On Wed, Jan 03, 2024 at 09:33:15PM -0800, Darrick J. Wong wrote:
> On Wed, Oct 18, 2023 at 09:25:11AM -0300, Wedson Almeida Filho wrote:
> > From: Wedson Almeida Filho <walmeida@microsoft.com>
> > 
> > +/// File system stats.
> > +///
> > +/// A subset of C's `kstatfs`.
> > +pub struct Stat {
> > +    /// Magic number of the file system.
> > +    pub magic: u32,
> > +
> > +    /// The maximum length of a file name.
> > +    pub namelen: i64,
> 
> Yikes, I hope I never see an 8EB filename.  The C side doesn't handle
> names longer than 255 bytes.

kstatfs::f_namelen is defined as a long in C.

> 
> > +
> > +    /// Block size.
> > +    pub bsize: i64,
> 
> Or an 8EB block size.  SMR notwithstanding, I think this could be u32.
> 
> Why are these values signed?  Nobody has a -1k block filesystem.

I agree, but they're signed in C, I'm just mimicking that. See kstatfs::f_bsize
for this particular case, it's also a long.

> 
> > +    /// Number of files in the file system.
> > +    pub files: u64,
> > +
> > +    /// Number of blocks in the file system.
> > +    pub blocks: u64,
> >  }
> >  
> > +    unsafe extern "C" fn statfs_callback(
> > +        dentry: *mut bindings::dentry,
> > +        buf: *mut bindings::kstatfs,
> > +    ) -> core::ffi::c_int {
> > +        from_result(|| {
> > +            // SAFETY: The C API guarantees that `dentry` is valid for read. `d_sb` is
> > +            // immutable, so it's safe to read it. The superblock is guaranteed to be valid dor
> > +            // the duration of the call.
> > +            let sb = unsafe { &*(*dentry).d_sb.cast::<SuperBlock<T>>() };
> > +            let s = T::statfs(sb)?;
> > +
> > +            // SAFETY: The C API guarantees that `buf` is valid for read and write.
> > +            let buf = unsafe { &mut *buf };
> > +            buf.f_type = s.magic.into();
> > +            buf.f_namelen = s.namelen;
> > +            buf.f_bsize = s.bsize;
> > +            buf.f_files = s.files;
> > +            buf.f_blocks = s.blocks;
> > +            buf.f_bfree = 0;
> > +            buf.f_bavail = 0;
> > +            buf.f_ffree = 0;
> 
> Why is it necessary to fill out the C structure with zeroes?
> statfs_by_dentry zeroes the buffer contents before calling ->statfs.

I didn't know they were zeroed before calling statfs. Removed this from v2.

Thanks,
-Wedson
diff mbox series

Patch

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index fa754c5e85a2..e2b2ccc835e3 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -11,6 +11,7 @@ 
 #include <linux/fs.h>
 #include <linux/fs_context.h>
 #include <linux/slab.h>
+#include <linux/statfs.h>
 #include <linux/pagemap.h>
 #include <linux/refcount.h>
 #include <linux/wait.h>
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 6c167583b275..829756cf6c48 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -83,6 +83,7 @@  macro_rules! declare_err {
     declare_err!(ENOGRACE, "NFS file lock reclaim refused.");
     declare_err!(ENODATA, "No data available.");
     declare_err!(EOPNOTSUPP, "Operation not supported on transport endpoint.");
+    declare_err!(ENOSYS, "Invalid system call number.");
 }
 
 /// Generic integer kernel error.
diff --git a/rust/kernel/fs.rs b/rust/kernel/fs.rs
index adf9cbee16d2..8f34da50e694 100644
--- a/rust/kernel/fs.rs
+++ b/rust/kernel/fs.rs
@@ -50,6 +50,31 @@  pub trait FileSystem {
     fn read_xattr(_inode: &INode<Self>, _name: &CStr, _outbuf: &mut [u8]) -> Result<usize> {
         Err(EOPNOTSUPP)
     }
+
+    /// Get filesystem statistics.
+    fn statfs(_sb: &SuperBlock<Self>) -> Result<Stat> {
+        Err(ENOSYS)
+    }
+}
+
+/// File system stats.
+///
+/// A subset of C's `kstatfs`.
+pub struct Stat {
+    /// Magic number of the file system.
+    pub magic: u32,
+
+    /// The maximum length of a file name.
+    pub namelen: i64,
+
+    /// Block size.
+    pub bsize: i64,
+
+    /// Number of files in the file system.
+    pub files: u64,
+
+    /// Number of blocks in the file system.
+    pub blocks: u64,
 }
 
 /// The types of directory entries reported by [`FileSystem::read_dir`].
@@ -478,7 +503,7 @@  impl<T: FileSystem + ?Sized> Tables<T> {
         freeze_fs: None,
         thaw_super: None,
         unfreeze_fs: None,
-        statfs: None,
+        statfs: Some(Self::statfs_callback),
         remount_fs: None,
         umount_begin: None,
         show_options: None,
@@ -496,6 +521,31 @@  impl<T: FileSystem + ?Sized> Tables<T> {
         shutdown: None,
     };
 
+    unsafe extern "C" fn statfs_callback(
+        dentry: *mut bindings::dentry,
+        buf: *mut bindings::kstatfs,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: The C API guarantees that `dentry` is valid for read. `d_sb` is
+            // immutable, so it's safe to read it. The superblock is guaranteed to be valid dor
+            // the duration of the call.
+            let sb = unsafe { &*(*dentry).d_sb.cast::<SuperBlock<T>>() };
+            let s = T::statfs(sb)?;
+
+            // SAFETY: The C API guarantees that `buf` is valid for read and write.
+            let buf = unsafe { &mut *buf };
+            buf.f_type = s.magic.into();
+            buf.f_namelen = s.namelen;
+            buf.f_bsize = s.bsize;
+            buf.f_files = s.files;
+            buf.f_blocks = s.blocks;
+            buf.f_bfree = 0;
+            buf.f_bavail = 0;
+            buf.f_ffree = 0;
+            Ok(0)
+        })
+    }
+
     const XATTR_HANDLERS: [*const bindings::xattr_handler; 2] = [&Self::XATTR_HANDLER, ptr::null()];
 
     const XATTR_HANDLER: bindings::xattr_handler = bindings::xattr_handler {