[PATCH v4 0/4] rust: DebugFS Bindings

[PATCH v4 0/4] rust: DebugFS Bindings

[Matthew Maurer]

This series provides safe DebugFS bindings for Rust, with a sample
driver using them.

This series currently only supports referencing `'static` lifetime
objects, because we need to know the objects outlive the DebugFS files.
A subsequent series may be able to relax this.

* Directories created in the root of debugfs are assumed to be owning,
  and will clean up after themselves when dropped.
* Subdirectories are assumed non-owning, remaining alive until their
  containing directory is cleaned up. They can be promoted to owning
  variants if needed.
* Files are non-owning, but can be manually removed by handle if needed.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
Changes in v4:
- Remove SubDir, replace with type-level constant.
- Add lifetime to Dir to prevent subdirectories and files from outliving
  their parents and triggering an Oops when accessed.
- Split unsafe blocks with two calls into two blocks
- Access `private` field through direct pointer dereference, avoiding
  creation of a reference to it.
- Notably not changed - owning/non-owning handle defaults. The best read
  I had from the thread was to continue with this mode, but I'm willing
  to change if need be.
- Comment changes
  - More comment markdown
  - Remove scopes from examples
  - Put `as_ptr` properties into a `# Guarantees` section.
- Link to v3: https://lore.kernel.org/r/20250501-debugfs-rust-v3-0-850869fab672@google.com

Changes in v3:
- Split `Dir` into `Dir`/`SubDir`/`File` to improve API.
- Add "." to end of all comments.
- Convert INVARIANT to # Invariants on types.
- Add backticks everywhere I found variables/types in my comments.
- Promoted invariant comment to doc comment.
- Extended sample commenting to make it clearer what is happening.
- Link to v2: https://lore.kernel.org/r/20250430-debugfs-rust-v2-0-2e8d3985812b@google.com

Changes in v2:
- Drop support for builder / pinned bindings in initial series
- Remove `ARef` usage to abstract the dentry nature of handles
- Remove error handling to discourage users from caring whether DebugFS
  is enabled.
- Support CONFIG_DEBUG_FS=n while leaving the API available
- Fixed mistaken decimal/octal mixup
- Doc/comment cleanup
- Link to v1: https://lore.kernel.org/r/20250429-debugfs-rust-v1-0-6b6e7cb7929f@google.com

---
Matthew Maurer (4):
      rust: debugfs: Bind DebugFS directory creation
      rust: debugfs: Bind file creation for long-lived Display
      rust: debugfs: Support format hooks
      rust: samples: Add debugfs sample

 MAINTAINERS                     |   2 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/debugfs.rs          | 418 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 samples/rust/Kconfig            |  11 ++
 samples/rust/Makefile           |   1 +
 samples/rust/rust_debugfs.rs    |  56 ++++++
 7 files changed, 490 insertions(+)
---
base-commit: b6a7783d306baf3150ac54cd5124f6e85dd375b0
change-id: 20250428-debugfs-rust-3cd5c97eb7d1

Best regards,
-- 
Matthew Maurer <mmaurer@google.com>

[PATCH v4 1/4] rust: debugfs: Bind DebugFS directory creation

[Matthew Maurer]

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

[PATCH v4 2/4] rust: debugfs: Bind file creation for long-lived Display

[Matthew Maurer]

Allows creation of files for references that live forever and lack
metadata through the `Display` implementation.

The reference must live forever because we do not have a maximum
lifetime for the file we are creating.

The `Display` implementation is used because `seq_printf` needs to route
through `%pA`, which in turn routes through Arguments. A more generic
API is provided later in the series, implemented in terms of this one.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/debugfs.rs | 139 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 41ac1711e9c0e66de1a434217c363176f806f434..21b116abad864d303f11cc515fe6f86ce5d51cbf 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -7,6 +7,7 @@
 //! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
 
 use crate::str::CStr;
+use core::fmt::Display;
 use core::marker::PhantomData;
 
 /// Owning handle to a DebugFS directory.
@@ -108,6 +109,57 @@ fn as_ptr(&self) -> *mut bindings::dentry {
     pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
         Dir::create(name, Some(self))
     }
+
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking
+    /// [`Display::fmt`] on the provided reference.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let dir = Dir::new(c_str!("my_debugfs_dir"));
+    /// dir.display_file(c_str!("foo"), &200);
+    /// // "my_debugfs_dir/foo" now contains the number 200.
+    /// ```
+    pub fn display_file<'b, T: Display + Sized>(
+        &'a self,
+        name: &CStr,
+        data: &'static T,
+    ) -> File<'b> {
+        // SAFETY:
+        // * `name` is a NUL-terminated C string, living across the call, by `CStr` invariant.
+        // * `parent` is a live `dentry` since we have a reference to it.
+        // * `vtable` is all stock `seq_file` implementations except for `open`.
+        //   `open`'s only requirement beyond what is provided to all open functions is that the
+        //   inode's data pointer must point to a `T` that will outlive it, which we know because
+        //   we have a static reference.
+        #[cfg(CONFIG_DEBUG_FS)]
+        let ptr = unsafe {
+            bindings::debugfs_create_file_full(
+                name.as_char_ptr(),
+                0o444,
+                self.as_ptr(),
+                data as *const _ as *mut _,
+                core::ptr::null(),
+                &<T as DisplayFile>::VTABLE,
+            )
+        };
+
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        let ptr = {
+            // Mark parameters used
+            let (_, _) = (name, data);
+            ERR_PTR(ENODEV)
+        };
+
+        // SAFETY: `debugfs_create_file_full` either returns an error code or a legal
+        // dentry pointer, and without `CONFIG_DEBUGFS` we return an error pointer, so
+        // `Dir::from_ptr` is safe to call here.
+        let dir = unsafe { Dir::from_ptr(ptr) };
+
+        File(dir)
+    }
 }
 
 impl<'a> Dir<'a, false> {
@@ -157,3 +209,90 @@ fn drop(&mut self) {
         }
     }
 }
+/// Handle to a DebugFS file.
+#[repr(transparent)]
+pub struct File<'a>(Dir<'a, true>);
+
+impl<'a> File<'a> {
+    /// Remove the file from DebugFS.
+    ///
+    /// # Examples
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let dir = Dir::new(c_str!("foo"));
+    /// let file = dir.display_file(c_str!("bar"), &0);
+    /// // "foo/bar" is created.
+    /// file.remove()
+    /// // "foo/bar" is removed"
+    pub fn remove(self) {
+        drop(self.0.owning())
+    }
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+mod helpers {
+    use crate::seq_file::SeqFile;
+    use crate::seq_print;
+    use core::fmt::Display;
+    use core::ptr::addr_of;
+
+    /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
+    ///
+    /// # Safety
+    ///
+    /// * `inode`'s private pointer must point to a value of type `T` which will outlive the `inode`
+    ///   and will not be mutated during this call.
+    /// * `file` must point to a live, not-yet-initialized file object.
+    pub(crate) unsafe extern "C" fn display_open<T: Display>(
+        inode: *mut bindings::inode,
+        file: *mut bindings::file,
+    ) -> i32 {
+        // SAFETY:
+        // * `file` is acceptable by caller precondition.
+        // * `print_act` will be called on a `seq_file` with private data set to the third argument,
+        //   so we meet its safety requirements.
+        // * The `data` pointer passed in the third argument is a valid `T` pointer that outlives
+        //   this call by caller preconditions.
+        unsafe { bindings::single_open(file, Some(display_act::<T>), (*inode).i_private) }
+    }
+
+    /// Prints private data stashed in a seq_file to that seq file.
+    ///
+    /// # Safety
+    ///
+    /// `seq` must point to a live `seq_file` whose private data is a live pointer to a `T` which is
+    /// not being mutated.
+    pub(crate) unsafe extern "C" fn display_act<T: Display>(
+        seq: *mut bindings::seq_file,
+        _: *mut core::ffi::c_void,
+    ) -> i32 {
+        // SAFETY: By caller precondition, seq points to a live seq_file.
+        let private_addr = unsafe { addr_of!((*seq).private) };
+        // SAFETY: By caller precondition, this pointer is live, points to a value of type `T`, and
+        // is not being mutated.
+        let data = unsafe { &*(*private_addr as *mut T) };
+        // SAFETY: By caller precondition, `seq_file` points to a live `seq_file`, so we can lift
+        // it.
+        let seq_file = unsafe { SeqFile::from_raw(seq) };
+        seq_print!(seq_file, "{}", data);
+        0
+    }
+
+    // Work around lack of generic const items.
+    pub(crate) trait DisplayFile: Display + Sized {
+        const VTABLE: bindings::file_operations = bindings::file_operations {
+            read: Some(bindings::seq_read),
+            llseek: Some(bindings::seq_lseek),
+            release: Some(bindings::single_release),
+            open: Some(display_open::<Self> as _),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+    }
+
+    impl<T: Display + Sized> DisplayFile for T {}
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+use helpers::*;

-- 
2.49.0.906.g1f30a19c02-goog

[PATCH v4 3/4] rust: debugfs: Support format hooks

[Matthew Maurer]

Rather than always using Display, allow hooking arbitrary functions to
arbitrary files. Display technically has the expressiveness to do this,
but requires a new type be declared for every different way to render
things, which can be very clumsy.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/debugfs.rs | 124 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 21b116abad864d303f11cc515fe6f86ce5d51cbf..cfdff63c10517f1aebf757c965289a49fed6ae85 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -7,6 +7,7 @@
 //! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
 
 use crate::str::CStr;
+use core::fmt;
 use core::fmt::Display;
 use core::marker::PhantomData;
 
@@ -123,9 +124,55 @@ pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b, true> {
     /// // "my_debugfs_dir/foo" now contains the number 200.
     /// ```
     pub fn display_file<'b, T: Display + Sized>(
-        &'a self,
+        &'b self,
         name: &CStr,
         data: &'static T,
+    ) -> File<'b> {
+        // SAFETY: As `data` lives for the static lifetime, it outlives the file.
+        unsafe { self.display_file_raw(name, data) }
+    }
+
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking `f`
+    /// on the provided reference.
+    ///
+    /// `f` must be a function item or a non-capturing closure, or this will fail to compile.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use core::sync::atomic::{AtomicU32, Ordering};
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let dir = Dir::new(c_str!("foo"));
+    /// static MY_ATOMIC: AtomicU32 = AtomicU32::new(3);
+    /// let file = dir.fmt_file(c_str!("bar"), &MY_ATOMIC, &|val, f| {
+    ///   let out = val.load(Ordering::Relaxed);
+    ///   writeln!(f, "{out:#010x}")
+    /// });
+    /// MY_ATOMIC.store(10, Ordering::Relaxed);
+    /// ```
+    pub fn fmt_file<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
+        &'b self,
+        name: &CStr,
+        data: &'static T,
+        f: &'static F,
+    ) -> File<'b> {
+        // SAFETY: As `data` lives for the static lifetime, it outlives the file
+        unsafe { self.fmt_file_raw(name, data, f) }
+    }
+
+    /// Creates a DebugFS file backed by the display implementation of the provided pointer.
+    ///
+    /// # Safety
+    ///
+    /// The pointee of `data` must outlive the accessibility of the `Dir` returned by this function.
+    /// This means that before `data` may become invalid, either:
+    /// * The refcount must go to zero.
+    /// * The file must be rendered inaccessible, e.g. via `debugfs_remove`.
+    unsafe fn display_file_raw<'b, T: Display + Sized>(
+        &'b self,
+        name: &CStr,
+        data: *const T,
     ) -> File<'b> {
         // SAFETY:
         // * `name` is a NUL-terminated C string, living across the call, by `CStr` invariant.
@@ -160,6 +207,32 @@ pub fn display_file<'b, T: Display + Sized>(
 
         File(dir)
     }
+
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking the
+    /// fomatter on the attached data.
+    ///
+    /// The attached function must be a ZST, and will cause a compilation error if it is not.
+    ///
+    /// # Safety
+    ///
+    /// `data` must outlive the resulting file's accessibility
+    unsafe fn fmt_file_raw<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
+        &'b self,
+        name: &CStr,
+        data: &T,
+        f: &'static F,
+    ) -> File<'b> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        let data_adapted = FormatAdapter::new(data, f);
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        let data_adapted = {
+            // Mark used
+            let (_, _) = (data, f);
+            &0
+        };
+        // SAFETY: data outlives the file's accessibility, so data_adapted does too
+        unsafe { self.display_file_raw(name, data_adapted) }
+    }
 }
 
 impl<'a> Dir<'a, false> {
@@ -234,7 +307,9 @@ pub fn remove(self) {
 mod helpers {
     use crate::seq_file::SeqFile;
     use crate::seq_print;
-    use core::fmt::Display;
+    use core::fmt;
+    use core::fmt::{Display, Formatter};
+    use core::marker::PhantomData;
     use core::ptr::addr_of;
 
     /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
@@ -292,6 +367,51 @@ pub(crate) trait DisplayFile: Display + Sized {
     }
 
     impl<T: Display + Sized> DisplayFile for T {}
+
+    /// Adapter to implement `Display` via a callback with the same representation as `T`.
+    ///
+    /// # Invariants
+    ///
+    /// If an instance for `FormatAdapter<_, F>` is constructed, `F` is inhabited.
+    #[repr(transparent)]
+    pub(crate) struct FormatAdapter<T, F> {
+        inner: T,
+        _formatter: PhantomData<F>,
+    }
+
+    impl<T, F> FormatAdapter<T, F> {
+        pub(crate) fn new<'a>(inner: &'a T, _f: &'static F) -> &'a Self {
+            // SAFETY: FormatAdapater is a repr(transparent) wrapper around T, so
+            // casting a reference is legal
+            // INVARIANT: We were passed a reference to F, so it is inhabited.
+            unsafe { core::mem::transmute(inner) }
+        }
+    }
+
+    impl<T, F> Display for FormatAdapter<T, F>
+    where
+        F: Fn(&T, &mut Formatter<'_>) -> fmt::Result + 'static,
+    {
+        fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
+            // SAFETY: FormatAdapter<_, F> can only be constructed if F is inhabited
+            let f: &F = unsafe { materialize_zst_fmt() };
+            f(&self.inner, fmt)
+        }
+    }
+
+    /// For types with a unique value, produce a static reference to it.
+    ///
+    /// # Safety
+    ///
+    /// The caller asserts that F is inhabited
+    unsafe fn materialize_zst_fmt<F>() -> &'static F {
+        const { assert!(core::mem::size_of::<F>() == 0) };
+        let zst_dangle: core::ptr::NonNull<F> = core::ptr::NonNull::dangling();
+        // SAFETY: While the pointer is dangling, it is a dangling pointer to a ZST, based on the
+        // assertion above. The type is also inhabited, by the caller's assertion. This means
+        // we can materialize it.
+        unsafe { zst_dangle.as_ref() }
+    }
 }
 
 #[cfg(CONFIG_DEBUG_FS)]

-- 
2.49.0.906.g1f30a19c02-goog

[PATCH v4 4/4] rust: samples: Add debugfs sample

[Matthew Maurer]

Provides an example of using the Rust DebugFS bindings.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 MAINTAINERS                  |  1 +
 samples/rust/Kconfig         | 11 +++++++++
 samples/rust/Makefile        |  1 +
 samples/rust/rust_debugfs.rs | 56 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 69 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a3b835e427b083a4ddd690d9e7739851f0af47ae..426bcdac025134e20911de8e2cf5c9efb0591814 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7278,6 +7278,7 @@ F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
 F:	rust/kernel/faux.rs
 F:	rust/kernel/platform.rs
+F:	samples/rust/rust_debugfs.rs
 F:	samples/rust/rust_driver_platform.rs
 F:	samples/rust/rust_driver_faux.rs
 
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 43cb72d72631bb2d6e06185e1d88778edff6ee13..6c42ed73f842cda26256039e6917bb443738d3f1 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -51,6 +51,17 @@ config SAMPLE_RUST_DMA
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DEBUGFS
+	tristate "DebugFS Test Driver"
+	depends on DEBUG_FS
+	help
+	  This option builds the Rust DebugFS Test driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_debugfs.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_DRIVER_PCI
 	tristate "PCI Driver"
 	depends on PCI
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 6a466afd2a21eba84a3b7b2be29f25dce44e9053..b1fc4677ed53fcf7d0f5a3dbf322f65851bc1784 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -4,6 +4,7 @@ ccflags-y += -I$(src)				# needed for trace events
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_DEBUGFS)		+= rust_debugfs.o
 obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
new file mode 100644
index 0000000000000000000000000000000000000000..2b1119b7281fd15109b542e6853d4206c2c80afc
--- /dev/null
+++ b/samples/rust/rust_debugfs.rs
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Sample DebugFS exporting module
+
+use core::sync::atomic::{AtomicU32, Ordering};
+use kernel::c_str;
+use kernel::debugfs::Dir;
+use kernel::prelude::*;
+
+module! {
+    type: RustDebugFs,
+    name: "rust_debugfs",
+    authors: ["Matthew Maurer"],
+    description: "Rust DebugFS usage sample",
+    license: "GPL",
+}
+
+struct RustDebugFs {
+    // As we only hold this for drop effect (to remove the directory) we have a leading underscore
+    // to indicate to the compiler that we don't expect to use this field directly.
+    _debugfs: Dir<'static>,
+}
+
+static EXAMPLE: AtomicU32 = AtomicU32::new(8);
+
+impl kernel::Module for RustDebugFs {
+    fn init(_this: &'static ThisModule) -> Result<Self> {
+        // Create a debugfs directory in the root of the filesystem called "sample_debugfs".
+        let debugfs = Dir::new(c_str!("sample_debugfs"));
+
+        {
+            // Create a subdirectory, so "sample_debugfs/subdir" now exists.
+            let sub = debugfs.subdir(c_str!("subdir"));
+
+            // Create a single file in the subdirectory called "example" that will read from the
+            // `EXAMPLE` atomic variable.
+            sub.fmt_file(c_str!("example"), &EXAMPLE, &|example, f| {
+                writeln!(f, "Reading atomic: {}", example.load(Ordering::Relaxed))
+            });
+            // Now, "sample_debugfs/subdir/example" will print "Reading atomic: 8\n" when read.
+        }
+
+        // Change the value in the variable displayed by the file. This is intended to demonstrate
+        // that the module can continue to change the value used by the file.
+        EXAMPLE.store(10, Ordering::Relaxed);
+        // Now, "sample_debugfs/subdir/example" will print "Reading atomic: 10\n" when read.
+
+        // Save the root debugfs directory we created to our module object. It will be
+        // automatically cleaned up when our module is unloaded because dropping the module object
+        // will drop the `Dir` handle. The base directory, the subdirectory, and the file will all
+        // continue to exist until the module is unloaded.
+        Ok(Self { _debugfs: debugfs })
+    }
+}

-- 
2.49.0.906.g1f30a19c02-goog

Re: [PATCH v4 1/4] rust: debugfs: Bind DebugFS directory creation

[Danilo Krummrich]

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.

Re: [PATCH v4 2/4] rust: debugfs: Bind file creation for long-lived Display

[Danilo Krummrich]

On Fri, May 02, 2025 at 07:49:31PM +0000, Matthew Maurer wrote:
> +/// Handle to a DebugFS file.
> +#[repr(transparent)]
> +pub struct File<'a>(Dir<'a, true>);

As mentioned in [1], please create a base type Entry. While it's an improvement
to not expose things like subdir() from File directly, it's still odd to base
File on a type that has invalid methods its purpose.

[1] https://lore.kernel.org/lkml/aBRrniLfCzWX7nbR@pollux/

> +impl<'a> File<'a> {
> +    /// Remove the file from DebugFS.
> +    ///
> +    /// # Examples
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// let dir = Dir::new(c_str!("foo"));
> +    /// let file = dir.display_file(c_str!("bar"), &0);
> +    /// // "foo/bar" is created.
> +    /// file.remove()
> +    /// // "foo/bar" is removed"
> +    pub fn remove(self) {
> +        drop(self.0.owning())
> +    }

Same as with Dir, please make it the other way around.

Re: [PATCH v4 1/4] rust: debugfs: Bind DebugFS directory creation

[Matthew Maurer]

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.

Re: [PATCH v4 1/4] rust: debugfs: Bind DebugFS directory creation

[Greg Kroah-Hartman]

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

Re: [PATCH v4 1/4] rust: debugfs: Bind DebugFS directory creation

[Matthew Maurer]

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

Re: [PATCH v4 1/4] rust: debugfs: Bind DebugFS directory creation

[Danilo Krummrich]

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.