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

[Wedson Almeida Filho <wedsonaf@gmail.com>]

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