Received-SPF: pass (google.com: domain of linux-kernel+bounces-178728-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Wedson Almeida Filho <wedsonaf@gmail.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Dave Chinner <david@fromorbit.com>
Cc: Kent Overstreet <kent.overstreet@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-fsdevel@vger.kernel.org,
	rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Wedson Almeida Filho <walmeida@microsoft.com>
Subject: [RFC PATCH v2 15/30] rust: fs: introduce `inode::Operations::lookup`
Date: Tue, 14 May 2024 10:16:56 -0300
Message-Id: <20240514131711.379322-16-wedsonaf@gmail.com>
In-Reply-To: <20240514131711.379322-1-wedsonaf@gmail.com>
References: <20240514131711.379322-1-wedsonaf@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: Wedson Almeida Filho <walmeida@microsoft.com>

Allow Rust file systems to create inodes that are children of a
directory inode when they're looked up by name.

Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 rust/kernel/error.rs      |  1 -
 rust/kernel/fs/dentry.rs  |  1 -
 rust/kernel/fs/inode.rs   | 58 ++++++++++++++++++++++++-----
 samples/rust/rust_rofs.rs | 77 +++++++++++++++++++++++++++++----------
 4 files changed, 105 insertions(+), 32 deletions(-)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index bb13bd4a7fa6..15628d2fa3b2 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -129,7 +129,6 @@ pub fn to_errno(self) -> core::ffi::c_int {
     }
 
     /// Returns the error encoded as a pointer.
-    #[allow(dead_code)]
     pub(crate) fn to_ptr<T>(self) -> *mut T {
         // SAFETY: `self.0` is a valid error due to its invariant.
         unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
diff --git a/rust/kernel/fs/dentry.rs b/rust/kernel/fs/dentry.rs
index 6a36a48cd28b..c93debb70ea3 100644
--- a/rust/kernel/fs/dentry.rs
+++ b/rust/kernel/fs/dentry.rs
@@ -43,7 +43,6 @@ impl<T: FileSystem + ?Sized> DEntry<T> {
     ///
     /// * `ptr` must be valid for at least the lifetime of the returned reference.
     /// * `ptr` has the correct file system type, or `T` is [`super::UnspecifiedFS`].
-    #[allow(dead_code)]
     pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::dentry) -> &'a Self {
         // SAFETY: The safety requirements guarantee that the reference is and remains valid.
         unsafe { &*ptr.cast::<Self>() }
diff --git a/rust/kernel/fs/inode.rs b/rust/kernel/fs/inode.rs
index 3d65b917af0e..c314d036c87e 100644
--- a/rust/kernel/fs/inode.rs
+++ b/rust/kernel/fs/inode.rs
@@ -6,9 +6,9 @@
 //!
 //! C headers: [`include/linux/fs.h`](srctree/include/linux/fs.h)
 
-use super::{file, sb::SuperBlock, FileSystem, Offset, UnspecifiedFS};
-use crate::error::Result;
-use crate::types::{ARef, AlwaysRefCounted, Lockable, Opaque};
+use super::{dentry, dentry::DEntry, file, sb::SuperBlock, FileSystem, Offset, UnspecifiedFS};
+use crate::error::{code::*, Result};
+use crate::types::{ARef, AlwaysRefCounted, Lockable, Locked, Opaque};
 use crate::{bindings, block, time::Timespec};
 use core::mem::ManuallyDrop;
 use core::{marker::PhantomData, ptr};
@@ -22,6 +22,14 @@
 pub trait Operations {
     /// File system that these operations are compatible with.
     type FileSystem: FileSystem + ?Sized;
+
+    /// Returns the inode corresponding to the directory entry with the given name.
+    fn lookup(
+        _parent: &Locked<&INode<Self::FileSystem>, ReadSem>,
+        _dentry: dentry::Unhashed<'_, Self::FileSystem>,
+    ) -> Result<Option<ARef<DEntry<Self::FileSystem>>>> {
+        Err(ENOTSUPP)
+    }
 }
 
 /// A node (inode) in the file index.
@@ -118,12 +126,7 @@ pub fn init(mut self, params: Params) -> Result<ARef<INode<T>>> {
         // SAFETY: This is a new inode, so it's safe to manipulate it mutably.
         let inode = unsafe { self.0.as_mut() };
         let mode = match params.typ {
-            Type::Dir => {
-                // SAFETY: `simple_dir_inode_operations` never changes, it's safe to reference it.
-                inode.i_op = unsafe { &bindings::simple_dir_inode_operations };
-
-                bindings::S_IFDIR
-            }
+            Type::Dir => bindings::S_IFDIR,
         };
 
         inode.i_mode = (params.mode & 0o777) | u16::try_from(mode)?;
@@ -148,6 +151,14 @@ pub fn init(mut self, params: Params) -> Result<ARef<INode<T>>> {
         Ok(unsafe { ARef::from_raw(manual.0.cast::<INode<T>>()) })
     }
 
+    /// Sets the inode operations on this new inode.
+    pub fn set_iops(&mut self, iops: Ops<T>) -> &mut Self {
+        // SAFETY: By the type invariants, it's ok to modify the inode.
+        let inode = unsafe { self.0.as_mut() };
+        inode.i_op = iops.0;
+        self
+    }
+
     /// Sets the file operations on this new inode.
     pub fn set_fops(&mut self, fops: file::Ops<T>) -> &mut Self {
         // SAFETY: By the type invariants, it's ok to modify the inode.
@@ -221,7 +232,11 @@ pub const fn new<U: Operations<FileSystem = T> + ?Sized>() -> Self {
         struct Table<T: Operations + ?Sized>(PhantomData<T>);
         impl<T: Operations + ?Sized> Table<T> {
             const TABLE: bindings::inode_operations = bindings::inode_operations {
-                lookup: None,
+                lookup: if T::HAS_LOOKUP {
+                    Some(Self::lookup_callback)
+                } else {
+                    None
+                },
                 get_link: None,
                 permission: None,
                 get_inode_acl: None,
@@ -247,6 +262,29 @@ impl<T: Operations + ?Sized> Table<T> {
                 fileattr_get: None,
                 get_offset_ctx: None,
             };
+
+            extern "C" fn lookup_callback(
+                parent_ptr: *mut bindings::inode,
+                dentry_ptr: *mut bindings::dentry,
+                _flags: u32,
+            ) -> *mut bindings::dentry {
+                // SAFETY: The C API guarantees that `parent_ptr` is a valid inode.
+                let parent = unsafe { INode::from_raw(parent_ptr) };
+
+                // SAFETY: The C API guarantees that `dentry_ptr` is a valid dentry.
+                let dentry = unsafe { DEntry::from_raw(dentry_ptr) };
+
+                // SAFETY: The C API guarantees that the inode's rw semaphore is locked at least in
+                // read mode. It does not expect callees to unlock it, so we make the locked object
+                // manually dropped to avoid unlocking it.
+                let locked = ManuallyDrop::new(unsafe { Locked::new(parent) });
+
+                match T::lookup(&locked, dentry::Unhashed(dentry)) {
+                    Err(e) => e.to_ptr(),
+                    Ok(None) => ptr::null_mut(),
+                    Ok(Some(ret)) => ManuallyDrop::new(ret).0.get(),
+                }
+            }
         }
         Self(&Table::<U>::TABLE, PhantomData)
     }
diff --git a/samples/rust/rust_rofs.rs b/samples/rust/rust_rofs.rs
index f4be5908369c..2a87e524e0e1 100644
--- a/samples/rust/rust_rofs.rs
+++ b/samples/rust/rust_rofs.rs
@@ -2,9 +2,11 @@
 
 //! Rust read-only file system sample.
 
-use kernel::fs::{dentry, file, file::File, inode, inode::INode, sb::SuperBlock, Offset};
+use kernel::fs::{
+    dentry, dentry::DEntry, file, file::File, inode, inode::INode, sb::SuperBlock, Offset,
+};
 use kernel::prelude::*;
-use kernel::{c_str, fs, time::UNIX_EPOCH, types::Either, types::Locked, user};
+use kernel::{c_str, fs, time::UNIX_EPOCH, types::ARef, types::Either, types::Locked, user};
 
 kernel::module_fs! {
     type: RoFs,
@@ -39,8 +41,36 @@ struct Entry {
 ];
 
 const DIR_FOPS: file::Ops<RoFs> = file::Ops::new::<RoFs>();
+const DIR_IOPS: inode::Ops<RoFs> = inode::Ops::new::<RoFs>();
 
 struct RoFs;
+
+impl RoFs {
+    fn iget(sb: &SuperBlock<Self>, e: &'static Entry) -> Result<ARef<INode<Self>>> {
+        let mut new = match sb.get_or_create_inode(e.ino)? {
+            Either::Left(existing) => return Ok(existing),
+            Either::Right(new) => new,
+        };
+
+        match e.etype {
+            inode::Type::Dir => new.set_iops(DIR_IOPS).set_fops(DIR_FOPS),
+        };
+
+        new.init(inode::Params {
+            typ: e.etype,
+            mode: 0o555,
+            size: ENTRIES.len().try_into()?,
+            blocks: 1,
+            nlink: 2,
+            uid: 0,
+            gid: 0,
+            atime: UNIX_EPOCH,
+            ctime: UNIX_EPOCH,
+            mtime: UNIX_EPOCH,
+        })
+    }
+}
+
 impl fs::FileSystem for RoFs {
     const NAME: &'static CStr = c_str!("rust_rofs");
 
@@ -50,28 +80,35 @@ fn fill_super(sb: &mut SuperBlock<Self>) -> Result {
     }
 
     fn init_root(sb: &SuperBlock<Self>) -> Result<dentry::Root<Self>> {
-        let inode = match sb.get_or_create_inode(1)? {
-            Either::Left(existing) => existing,
-            Either::Right(mut new) => {
-                new.set_fops(DIR_FOPS);
-                new.init(inode::Params {
-                    typ: inode::Type::Dir,
-                    mode: 0o555,
-                    size: ENTRIES.len().try_into()?,
-                    blocks: 1,
-                    nlink: 2,
-                    uid: 0,
-                    gid: 0,
-                    atime: UNIX_EPOCH,
-                    ctime: UNIX_EPOCH,
-                    mtime: UNIX_EPOCH,
-                })?
-            }
-        };
+        let inode = Self::iget(sb, &ENTRIES[0])?;
         dentry::Root::try_new(inode)
     }
 }
 
+#[vtable]
+impl inode::Operations for RoFs {
+    type FileSystem = Self;
+
+    fn lookup(
+        parent: &Locked<&INode<Self>, inode::ReadSem>,
+        dentry: dentry::Unhashed<'_, Self>,
+    ) -> Result<Option<ARef<DEntry<Self>>>> {
+        if parent.ino() != 1 {
+            return dentry.splice_alias(None);
+        }
+
+        let name = dentry.name();
+        for e in &ENTRIES {
+            if name == e.name {
+                let inode = Self::iget(parent.super_block(), e)?;
+                return dentry.splice_alias(Some(inode));
+            }
+        }
+
+        dentry.splice_alias(None)
+    }
+}
+
 #[vtable]
 impl file::Operations for RoFs {
     type FileSystem = Self;
-- 
2.34.1