Support creating DebugFS directories and subdirectories. Similar to the
original DebugFS API, errors are hidden.

By default, when a root directory handle leaves scope, it will be cleaned
up.

Subdirectories will by default persist until their root directory is
cleaned up, but can be converted into a root directory if a scoped
lifecycle is desired.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/debugfs.rs          | 159 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 4 files changed, 162 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 906881b6c5cb6ff743e13b251873b89138c69a1c..a3b835e427b083a4ddd690d9e7739851f0af47ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7271,6 +7271,7 @@ F:	include/linux/kobj*
 F:	include/linux/property.h
 F:	include/linux/sysfs.h
 F:	lib/kobj*
+F:	rust/kernel/debugfs.rs
 F:	rust/kernel/device.rs
 F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 8a2add69e5d66d1c2ebed9d2c950380e61c48842..787f928467faabd02a7f3cf041378fac856c4f89 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -13,6 +13,7 @@
 #include <linux/blkdev.h>
 #include <linux/cpumask.h>
 #include <linux/cred.h>
+#include <linux/debugfs.h>
 #include <linux/device/faux.h>
 #include <linux/dma-mapping.h>
 #include <linux/errname.h>
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
new file mode 100644
index 0000000000000000000000000000000000000000..41ac1711e9c0e66de1a434217c363176f806f434
--- /dev/null
+++ b/rust/kernel/debugfs.rs
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! DebugFS Abstraction
+//!
+//! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
+
+use crate::str::CStr;
+use core::marker::PhantomData;
+
+/// Owning handle to a DebugFS directory.
+///
+/// This directory will be cleaned up when it goes out of scope.
+///
+/// # Invariants
+///
+/// The wrapped pointer will always be `NULL`, an error, or an owned DebugFS `dentry`.
+#[repr(transparent)]
+pub struct Dir<'a, const KEEP: bool = false> {
+    #[cfg(CONFIG_DEBUG_FS)]
+    dir: *mut bindings::dentry,
+    // We need to be outlived by our parent, if they exist, but we don't actually need to be able
+    // to access the data.
+    _phantom: PhantomData<&'a Dir<'a, true>>,
+}
+
+// SAFETY: [`Dir`] is just a `dentry` under the hood, which the API promises can be transferred
+// between threads.
+unsafe impl<const KEEP: bool> Send for Dir<'_, KEEP> {}
+
+// SAFETY: All the native functions we re-export use interior locking, and the contents of the
+// struct are opaque to Rust.
+unsafe impl<const KEEP: bool> Sync for Dir<'_, KEEP> {}
+
+impl<'a, const KEEP: bool> Dir<'a, KEEP> {
+    /// Create a new directory in DebugFS. If `parent` is [`None`], it will be created at the root.
+    #[cfg(CONFIG_DEBUG_FS)]
+    fn create<const PARENT_KEEP: bool>(name: &CStr, parent: Option<&Dir<'a, PARENT_KEEP>>) -> Self {
+        let parent_ptr = match parent {
+            Some(parent) => parent.as_ptr(),
+            None => core::ptr::null_mut(),
+        };
+        // SAFETY:
+        // * `name` argument points to a NUL-terminated string that lives across the call, by
+        //   invariants of `&CStr`.
+        // * If `parent` is `None`, `parent` accepts null pointers to mean create at root.
+        // * If `parent` is `Some`, `parent` accepts live dentry debugfs pointers.
+        //   so we can call `Self::from_ptr`.
+        let dir = unsafe { bindings::debugfs_create_dir(name.as_char_ptr(), parent_ptr) };
+
+        // SAFETY: `debugfs_create_dir` either returns an error code or a legal `dentry` pointer,
+        unsafe { Self::from_ptr(dir) }
+    }
+
+    #[cfg(not(CONFIG_DEBUG_FS))]
+    fn create<const PARENT_KEEP: bool>(
+        _name: &CStr,
+        _parent: Option<&Dir<'a, PARENT_KEEP>>,
+    ) -> Self {
+        Self()
+    }
+
+    /// Constructs a new DebugFS [`Dir`] from the underlying pointer.
+    ///
+    /// # Safety
+    ///
+    /// The pointer must either be an error code, `NULL`, or represent a transfer of ownership of a
+    /// live DebugFS directory.
+    #[cfg(CONFIG_DEBUG_FS)]
+    unsafe fn from_ptr(dir: *mut bindings::dentry) -> Self {
+        Self {
+            dir,
+            _phantom: PhantomData,
+        }
+    }
+
+    /// Returns the pointer representation of the DebugFS directory.
+    ///
+    /// # Guarantees
+    ///
+    /// Due to the type invariant, the value returned from this function will always be an error
+    /// code, NUL, or a live DebugFS directory.
+    // If this function is ever needed with `not(CONFIG_DEBUG_FS)`, hardcode it to return
+    // `ERR_PTR(ENODEV)`.
+    #[cfg(CONFIG_DEBUG_FS)]
+    fn as_ptr(&self) -> *mut bindings::dentry {
+        self.dir
+    }
+
+    /// Create a DebugFS subdirectory.
+    ///
+    /// This returns a [`Dir<'_, true>`], which will not be automatically cleaned up when it
+    /// leaves scope.
+    /// To convert this to a handle governing the lifetime of the directory, use [`Dir::owning`].
+    ///
+    /// Regardless of conversion, subdirectory handles cannot outlive the directory handle they
+    /// were created from.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let parent = Dir::new(c_str!("parent"));
+    /// let child = parent.subdir(c_str!("child"));
+    /// ```
+    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
+        Dir::create(name, Some(self))
+    }
+}
+
+impl<'a> Dir<'a, false> {
+    /// Create a new directory in DebugFS at the root.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let debugfs = Dir::new(c_str!("parent"));
+    /// ```
+    pub fn new(name: &CStr) -> Self {
+        Dir::create::<false>(name, None)
+    }
+}
+
+impl<'a> Dir<'a, true> {
+    /// Upgrade a non-owning directory to one which will be removed on drop.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let debugfs = Dir::new(c_str!("parent"));
+    /// let subdir = debugfs.subdir(c_str!("child"));
+    /// // If subdir were dropped, the directory would not be removed.
+    /// let owned_subdir = subdir.owning();
+    /// // If owned_subdir is dropped, "child" will be removed.
+    /// ```
+    pub fn owning(self) -> Dir<'a, false> {
+        Dir {
+            dir: self.dir,
+            _phantom: self._phantom,
+        }
+    }
+}
+
+impl<const KEEP: bool> Drop for Dir<'_, KEEP> {
+    fn drop(&mut self) {
+        #[cfg(CONFIG_DEBUG_FS)]
+        if !KEEP {
+            // SAFETY: `debugfs_remove` can take `NULL`, error values, and legal DebugFS dentries.
+            // `as_ptr` guarantees that the pointer is of this form.
+            unsafe { bindings::debugfs_remove(self.as_ptr()) }
+        }
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c3762e80b314316b4b0cee3bfd9442f8f0510b91..86f6055b828d5f711578293d8916a517f2436977 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -45,6 +45,7 @@
 #[doc(hidden)]
 pub mod build_assert;
 pub mod cred;
+pub mod debugfs;
 pub mod device;
 pub mod device_id;
 pub mod devres;

-- 
2.49.0.906.g1f30a19c02-goog

On Fri, May 02, 2025 at 07:49:30PM +0000, Matthew Maurer wrote:
> +/// Owning handle to a DebugFS directory.
> +///
> +/// This directory will be cleaned up when it goes out of scope.
> +///
> +/// # Invariants
> +///
> +/// The wrapped pointer will always be `NULL`, an error, or an owned DebugFS `dentry`.
> +#[repr(transparent)]
> +pub struct Dir<'a, const KEEP: bool = false> {

Why did you move to a const generic, rather than a new type? What's the
advantage? AFAICS, it just makes it less obvious to see from the type itself how
it will behave. Reading Dir<true> doesn't make it obvious what it does.

While I prefer a new type over the const generic, I'm fine with it. But I think
we should use something more descriptive than a bool. Please see
device::DeviceContext for reference.

> +    /// Create a DebugFS subdirectory.
> +    ///
> +    /// This returns a [`Dir<'_, true>`], which will not be automatically cleaned up when it
> +    /// leaves scope.
> +    /// To convert this to a handle governing the lifetime of the directory, use [`Dir::owning`].
> +    ///
> +    /// Regardless of conversion, subdirectory handles cannot outlive the directory handle they
> +    /// were created from.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// let parent = Dir::new(c_str!("parent"));
> +    /// let child = parent.subdir(c_str!("child"));
> +    /// ```
> +    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
> +        Dir::create(name, Some(self))
> +    }

The default should be that the directory is removed when the Dir instance is
dropped.

The common case (which people expect) is that an object is cleaned up on drop().

> +impl<'a> Dir<'a, true> {
> +    /// Upgrade a non-owning directory to one which will be removed on drop.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// let debugfs = Dir::new(c_str!("parent"));
> +    /// let subdir = debugfs.subdir(c_str!("child"));
> +    /// // If subdir were dropped, the directory would not be removed.
> +    /// let owned_subdir = subdir.owning();
> +    /// // If owned_subdir is dropped, "child" will be removed.
> +    /// ```
> +    pub fn owning(self) -> Dir<'a, false> {
> +        Dir {
> +            dir: self.dir,
> +            _phantom: self._phantom,
> +        }
> +    }

As mentioned above, please make it the other way around.

On Sat, May 3, 2025 at 5:36 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri, May 02, 2025 at 07:49:30PM +0000, Matthew Maurer wrote:
> > +/// Owning handle to a DebugFS directory.
> > +///
> > +/// This directory will be cleaned up when it goes out of scope.
> > +///
> > +/// # Invariants
> > +///
> > +/// The wrapped pointer will always be `NULL`, an error, or an owned DebugFS `dentry`.
> > +#[repr(transparent)]
> > +pub struct Dir<'a, const KEEP: bool = false> {
>
> Why did you move to a const generic, rather than a new type? What's the
> advantage? AFAICS, it just makes it less obvious to see from the type itself how
> it will behave. Reading Dir<true> doesn't make it obvious what it does.
>
> While I prefer a new type over the const generic, I'm fine with it. But I think
> we should use something more descriptive than a bool. Please see
> device::DeviceContext for reference.

I'm fine with a new type or a using a more descriptive const generic -
I did the const-generic to avoid the need to make one variant the
derefee, which can sometimes complicate structure. I'll default to a
more descriptive const-generic.

>
> > +    /// Create a DebugFS subdirectory.
> > +    ///
> > +    /// This returns a [`Dir<'_, true>`], which will not be automatically cleaned up when it
> > +    /// leaves scope.
> > +    /// To convert this to a handle governing the lifetime of the directory, use [`Dir::owning`].
> > +    ///
> > +    /// Regardless of conversion, subdirectory handles cannot outlive the directory handle they
> > +    /// were created from.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// # use kernel::c_str;
> > +    /// # use kernel::debugfs::Dir;
> > +    /// let parent = Dir::new(c_str!("parent"));
> > +    /// let child = parent.subdir(c_str!("child"));
> > +    /// ```
> > +    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
> > +        Dir::create(name, Some(self))
> > +    }
>
> The default should be that the directory is removed when the Dir instance is
> dropped.
>
> The common case (which people expect) is that an object is cleaned up on drop().

In general for Rust, I agree with you. For this particular case, I
have a strong disagreement - take a look at calls to
`debugfs_create_dir` in existing C code - new code chooses to discard
subdirectory handles when done and rely on the recursive remove of the
root directory to clean up subdirectories. If you and Greg K-H both
agree that I should make them drop by default, I'll switch it, but I
think this is me following the subsystem maintainer's intentions here.

>
> > +impl<'a> Dir<'a, true> {
> > +    /// Upgrade a non-owning directory to one which will be removed on drop.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// # use kernel::c_str;
> > +    /// # use kernel::debugfs::Dir;
> > +    /// let debugfs = Dir::new(c_str!("parent"));
> > +    /// let subdir = debugfs.subdir(c_str!("child"));
> > +    /// // If subdir were dropped, the directory would not be removed.
> > +    /// let owned_subdir = subdir.owning();
> > +    /// // If owned_subdir is dropped, "child" will be removed.
> > +    /// ```
> > +    pub fn owning(self) -> Dir<'a, false> {
> > +        Dir {
> > +            dir: self.dir,
> > +            _phantom: self._phantom,
> > +        }
> > +    }
>
> As mentioned above, please make it the other way around.

On Mon, May 05, 2025 at 09:21:51AM -0700, Matthew Maurer wrote:
> On Sat, May 3, 2025 at 5:36 AM Danilo Krummrich <dakr@kernel.org> wrote:
> > On Fri, May 02, 2025 at 07:49:30PM +0000, Matthew Maurer wrote:
> > > +/// Owning handle to a DebugFS directory.
> > > +///
> > > +/// This directory will be cleaned up when it goes out of scope.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// The wrapped pointer will always be `NULL`, an error, or an owned DebugFS `dentry`.
> > > +#[repr(transparent)]
> > > +pub struct Dir<'a, const KEEP: bool = false> {
> >
> > Why did you move to a const generic, rather than a new type? What's the
> > advantage? AFAICS, it just makes it less obvious to see from the type itself how
> > it will behave. Reading Dir<true> doesn't make it obvious what it does.
> >
> > While I prefer a new type over the const generic, I'm fine with it. But I think
> > we should use something more descriptive than a bool. Please see
> > device::DeviceContext for reference.
> 
> I'm fine with a new type or a using a more descriptive const generic -
> I did the const-generic to avoid the need to make one variant the
> derefee, which can sometimes complicate structure. I'll default to a
> more descriptive const-generic.

Sounds good to me.

> > > +    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
> > > +        Dir::create(name, Some(self))
> > > +    }
> >
> > The default should be that the directory is removed when the Dir instance is
> > dropped.
> >
> > The common case (which people expect) is that an object is cleaned up on drop().
> 
> In general for Rust, I agree with you. For this particular case, I
> have a strong disagreement - take a look at calls to
> `debugfs_create_dir` in existing C code - new code chooses to discard
> subdirectory handles when done and rely on the recursive remove of the
> root directory to clean up subdirectories. If you and Greg K-H both
> agree that I should make them drop by default, I'll switch it, but I
> think this is me following the subsystem maintainer's intentions here.

I don't see how picking a (Rust) consitent default is contrary to relying on
recursive remove of directories.

I don't want this API to have "inverse drop semantics"; it's gonna confuse
people and, AFAIK, it'd be the only API doing that.

Your original code with Dir::keep() was perfectly fine and it was still
supporting the "recursive remove case" properly.

So, again this *not* an either-or case, we can have both.

On Mon, May 05, 2025 at 09:21:51AM -0700, Matthew Maurer wrote:
> On Sat, May 3, 2025 at 5:36 AM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Fri, May 02, 2025 at 07:49:30PM +0000, Matthew Maurer wrote:
> > > +/// Owning handle to a DebugFS directory.
> > > +///
> > > +/// This directory will be cleaned up when it goes out of scope.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// The wrapped pointer will always be `NULL`, an error, or an owned DebugFS `dentry`.
> > > +#[repr(transparent)]
> > > +pub struct Dir<'a, const KEEP: bool = false> {
> >
> > Why did you move to a const generic, rather than a new type? What's the
> > advantage? AFAICS, it just makes it less obvious to see from the type itself how
> > it will behave. Reading Dir<true> doesn't make it obvious what it does.
> >
> > While I prefer a new type over the const generic, I'm fine with it. But I think
> > we should use something more descriptive than a bool. Please see
> > device::DeviceContext for reference.
> 
> I'm fine with a new type or a using a more descriptive const generic -
> I did the const-generic to avoid the need to make one variant the
> derefee, which can sometimes complicate structure. I'll default to a
> more descriptive const-generic.
> 
> >
> > > +    /// Create a DebugFS subdirectory.
> > > +    ///
> > > +    /// This returns a [`Dir<'_, true>`], which will not be automatically cleaned up when it
> > > +    /// leaves scope.
> > > +    /// To convert this to a handle governing the lifetime of the directory, use [`Dir::owning`].
> > > +    ///
> > > +    /// Regardless of conversion, subdirectory handles cannot outlive the directory handle they
> > > +    /// were created from.
> > > +    ///
> > > +    /// # Examples
> > > +    ///
> > > +    /// ```
> > > +    /// # use kernel::c_str;
> > > +    /// # use kernel::debugfs::Dir;
> > > +    /// let parent = Dir::new(c_str!("parent"));
> > > +    /// let child = parent.subdir(c_str!("child"));
> > > +    /// ```
> > > +    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
> > > +        Dir::create(name, Some(self))
> > > +    }
> >
> > The default should be that the directory is removed when the Dir instance is
> > dropped.
> >
> > The common case (which people expect) is that an object is cleaned up on drop().
> 
> In general for Rust, I agree with you. For this particular case, I
> have a strong disagreement - take a look at calls to
> `debugfs_create_dir` in existing C code - new code chooses to discard
> subdirectory handles when done and rely on the recursive remove of the
> root directory to clean up subdirectories. If you and Greg K-H both
> agree that I should make them drop by default, I'll switch it, but I
> think this is me following the subsystem maintainer's intentions here.

I'm ok with the directory being cleaned up when it goes out of scope.
That makes way more sense overall, and will prevent leaks and other
messes.

For the C api, what I really don't like is when we were returning a
dentry to the caller, as then they had to manage it.  Ideally I didn't
want them to have to manage anything, hence the "lookup_and_remove"
function I added, all that was needed to remember is the name of the
directory, which the driver almost always already knew.

With Rust it's simpler, you can save off a reference and when it goes
away, it gets cleaned up.  You don't have access to the raw dentry,
which takes away my original objection, and if debugfs is not enabled,
there's no additional storage requirements.

So let's keep it simple here, and rely on lifetime rules when we can.

thanks,

greg k-h

On Mon, May 5, 2025 at 9:29 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, May 05, 2025 at 09:21:51AM -0700, Matthew Maurer wrote:
> > On Sat, May 3, 2025 at 5:36 AM Danilo Krummrich <dakr@kernel.org> wrote:
> > >
> > > On Fri, May 02, 2025 at 07:49:30PM +0000, Matthew Maurer wrote:
> > > > +/// Owning handle to a DebugFS directory.
> > > > +///
> > > > +/// This directory will be cleaned up when it goes out of scope.
> > > > +///
> > > > +/// # Invariants
> > > > +///
> > > > +/// The wrapped pointer will always be `NULL`, an error, or an owned DebugFS `dentry`.
> > > > +#[repr(transparent)]
> > > > +pub struct Dir<'a, const KEEP: bool = false> {
> > >
> > > Why did you move to a const generic, rather than a new type? What's the
> > > advantage? AFAICS, it just makes it less obvious to see from the type itself how
> > > it will behave. Reading Dir<true> doesn't make it obvious what it does.
> > >
> > > While I prefer a new type over the const generic, I'm fine with it. But I think
> > > we should use something more descriptive than a bool. Please see
> > > device::DeviceContext for reference.
> >
> > I'm fine with a new type or a using a more descriptive const generic -
> > I did the const-generic to avoid the need to make one variant the
> > derefee, which can sometimes complicate structure. I'll default to a
> > more descriptive const-generic.
> >
> > >
> > > > +    /// Create a DebugFS subdirectory.
> > > > +    ///
> > > > +    /// This returns a [`Dir<'_, true>`], which will not be automatically cleaned up when it
> > > > +    /// leaves scope.
> > > > +    /// To convert this to a handle governing the lifetime of the directory, use [`Dir::owning`].
> > > > +    ///
> > > > +    /// Regardless of conversion, subdirectory handles cannot outlive the directory handle they
> > > > +    /// were created from.
> > > > +    ///
> > > > +    /// # Examples
> > > > +    ///
> > > > +    /// ```
> > > > +    /// # use kernel::c_str;
> > > > +    /// # use kernel::debugfs::Dir;
> > > > +    /// let parent = Dir::new(c_str!("parent"));
> > > > +    /// let child = parent.subdir(c_str!("child"));
> > > > +    /// ```
> > > > +    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
> > > > +        Dir::create(name, Some(self))
> > > > +    }
> > >
> > > The default should be that the directory is removed when the Dir instance is
> > > dropped.
> > >
> > > The common case (which people expect) is that an object is cleaned up on drop().
> >
> > In general for Rust, I agree with you. For this particular case, I
> > have a strong disagreement - take a look at calls to
> > `debugfs_create_dir` in existing C code - new code chooses to discard
> > subdirectory handles when done and rely on the recursive remove of the
> > root directory to clean up subdirectories. If you and Greg K-H both
> > agree that I should make them drop by default, I'll switch it, but I
> > think this is me following the subsystem maintainer's intentions here.
>
> I'm ok with the directory being cleaned up when it goes out of scope.
> That makes way more sense overall, and will prevent leaks and other
> messes.

OK, I'll switch to defaulting to cleanups.

>
> For the C api, what I really don't like is when we were returning a
> dentry to the caller, as then they had to manage it.  Ideally I didn't
> want them to have to manage anything, hence the "lookup_and_remove"
> function I added, all that was needed to remember is the name of the
> directory, which the driver almost always already knew.

Yeah, `lookup_and_remove` actually kind of complicates things for the
Rust model if we ever bind it, as it can be used to invalidate the
object we're holding in a way that has no lifetime constraints.

>
> With Rust it's simpler, you can save off a reference and when it goes
> away, it gets cleaned up.  You don't have access to the raw dentry,
> which takes away my original objection, and if debugfs is not enabled,
> there's no additional storage requirements.
>
> So let's keep it simple here, and rely on lifetime rules when we can.
>
> thanks,
>
> greg k-h