[PATCH v9 0/5] rust: DebugFS Bindings

[PATCH v9 0/5] rust: DebugFS Bindings

[Matthew Maurer]

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

Example interaction with the sample driver:
~ # mount -t debugfs debugfs /debug
~ # cat /debug/sample_debugfs/*
"sample-debugfs"
0
0
Inner { x: 3, y: 10 }
~ # cat /debug/sample_debugfs/inc_counter
1
~ # cat /debug/sample_debugfs/inc_counter
2
~ # cat /debug/sample_debugfs/inc_counter
3
~ # cat /debug/sample_debugfs/*
"sample-debugfs"
4
4
Inner { x: 3, y: 10 }
~ # cat /debug/sample_control/swap
Swapped!
~ # cat /debug/sample_debugfs/*
"sample-debugfs"
5
5
Inner { x: 10, y: 3 }
~ # cat /debug/sample_control/add_counter 
Counter added!
~ # cat /debug/sample_debugfs/*
"sample-debugfs"
6
6
Inner { x: 16, y: 3 }
~ # 

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
Changes in v9:
- Switched to `PinInit` backing instead of `ForeignOwnable`
- Changed sample to be a platform driver
- Exported a static property
- Demonstrated runtime mutation in platform driver (`inc_counter`)
- Demonstrated how driver code would interact with data structures
  exported through DebugFS (`Wrapper`)
- Link to v8: https://lore.kernel.org/r/20250627-debugfs-rust-v8-0-c6526e413d40@google.com

Changes in v8:
- Switched from casts to `core::from_{ref, mut}` in type change
- Link to v7: https://lore.kernel.org/r/20250624-debugfs-rust-v7-0-9c8835a7a20f@google.com

Changes in v7:
- Rewrote `entry::Entry` -> `Entry`
- Use `c_int` and `c_void` from kernel prelude rather than core
- Removed unnecessary `display_open` cast
- Switched from `Deref` + an explicit box to `ForeignOwnable` for
  attaching owned data.
- Made `&'static` and `&'static mut` implement `ForeignOwnable`
- Swapped "driver" to "module" in sample code
- Link to v6: https://lore.kernel.org/r/20250618-debugfs-rust-v6-0-72cae211b133@google.com

Changes in v6:
- Replaced explicit lifetimes with children keeping their parents alive.
- Added support for attaching owned data.
- Removed recomendation to only keep root handles and handles you want
  to delete around.
- Refactored some code into separate files to improve clarity.
- Link to v5: https://lore.kernel.org/r/20250505-debugfs-rust-v5-0-3e93ce7bb76e@google.com

Changes in v5:
- Made Dir + File wrappers around Entry
- All functions return owning handles. To discard without drop, use
  `forget`. To keep a handle without drop, use `ManuallyDrop`.
- Fixed bugs around `not(CONFIG_DEBUG_FS)`
- Removed unnecessary `addr_of!`
- Link to v4: https://lore.kernel.org/r/20250502-debugfs-rust-v4-0-788a9c6c2e77@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 (5):
      rust: debugfs: Bind DebugFS directory creation
      rust: debugfs: Bind file creation for long-lived Display
      rust: debugfs: Support `PinInit` backing for `File`s.
      rust: debugfs: Support format hooks
      rust: samples: Add debugfs sample

 MAINTAINERS                         |   3 +
 rust/bindings/bindings_helper.h     |   1 +
 rust/kernel/debugfs.rs              | 286 ++++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/display_file.rs | 100 +++++++++++++
 rust/kernel/debugfs/entry.rs        |  66 +++++++++
 rust/kernel/lib.rs                  |   1 +
 samples/rust/Kconfig                |  11 ++
 samples/rust/Makefile               |   1 +
 samples/rust/rust_debugfs.rs        | 182 +++++++++++++++++++++++
 9 files changed, 651 insertions(+)
---
base-commit: 6d16cd5769bbb5eb62974e8eddb97fca830b49fd
change-id: 20250428-debugfs-rust-3cd5c97eb7d1

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

[PATCH v9 1/5] rust: debugfs: Bind DebugFS directory creation

[Matthew Maurer]

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

Directories persist until their handle and the handles of any child
objects have been dropped.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 MAINTAINERS                     |  2 +
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/debugfs.rs          | 90 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/entry.rs    | 58 ++++++++++++++++++++++++++
 rust/kernel/lib.rs              |  1 +
 5 files changed, 152 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f8ddeec3b177772caf6160de65ca9985912cac8..1427af9d9779b1f6463409f4392e2900438bdc2a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7367,6 +7367,8 @@ 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/debugfs/
 F:	rust/kernel/device.rs
 F:	rust/kernel/device/
 F:	rust/kernel/device_id.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7e8f2285064797d5bbac5583990ff823b76c6bdc..8600b361bce3f3b613d5189b7acd1704326b6284 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -46,6 +46,7 @@
 #include <linux/cpufreq.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..2359bd11cd664fb9f7206f8fe38f758dc43d2cb8
--- /dev/null
+++ b/rust/kernel/debugfs.rs
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+//! DebugFS Abstraction
+//!
+//! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
+
+#[cfg(CONFIG_DEBUG_FS)]
+use crate::prelude::GFP_KERNEL;
+use crate::str::CStr;
+#[cfg(CONFIG_DEBUG_FS)]
+use crate::sync::Arc;
+
+#[cfg(CONFIG_DEBUG_FS)]
+mod entry;
+#[cfg(CONFIG_DEBUG_FS)]
+use entry::Entry;
+
+/// Owning handle to a DebugFS directory.
+///
+/// This directory will be cleaned up when the handle and all child directory/file handles have
+/// been dropped.
+// We hold a reference to our parent if it exists to prevent the dentry we point to from being
+// cleaned up when our parent is removed.
+pub struct Dir(#[cfg(CONFIG_DEBUG_FS)] Option<Arc<Entry>>);
+
+impl Dir {
+    /// Create a new directory in DebugFS. If `parent` is [`None`], it will be created at the root.
+    #[cfg(CONFIG_DEBUG_FS)]
+    fn create(name: &CStr, parent: Option<&Dir>) -> Self {
+        let parent_ptr = match parent {
+            // If the parent couldn't be allocated, just early-return
+            Some(Dir(None)) => return Self(None),
+            Some(Dir(Some(entry))) => entry.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_ptr` is null to mean create at root.
+        // * If `parent` is `Some`, `parent_ptr` is a live dentry debugfs pointer.
+        let dir = unsafe { bindings::debugfs_create_dir(name.as_char_ptr(), parent_ptr) };
+
+        Self(
+            // If Arc creation fails, the `Entry` will be dropped, so the directory will be cleaned
+            // up.
+            Arc::new(
+                // SAFETY: `debugfs_create_dir` either returns an error code or a legal `dentry`
+                // pointer, and the parent is the same one passed to `debugfs_create_dir`
+                unsafe { Entry::new(dir, parent.and_then(|dir| dir.0.clone())) },
+                GFP_KERNEL,
+            )
+            .ok(),
+        )
+    }
+
+    #[cfg(not(CONFIG_DEBUG_FS))]
+    fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
+        Self()
+    }
+
+    /// Create a DebugFS subdirectory.
+    ///
+    /// 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(&self, name: &CStr) -> Self {
+        Dir::create(name, Some(self))
+    }
+
+    /// 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(name, None)
+    }
+}
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
new file mode 100644
index 0000000000000000000000000000000000000000..ae0e2c4e1d58e878ebb081a71e4ac0f4a7d99b91
--- /dev/null
+++ b/rust/kernel/debugfs/entry.rs
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+use crate::sync::Arc;
+
+/// Owning handle to a DebugFS entry.
+///
+/// # Invariants
+///
+/// The wrapped pointer will always be `NULL`, an error, or an owned DebugFS `dentry`.
+pub(crate) struct Entry {
+    entry: *mut bindings::dentry,
+    _parent: Option<Arc<Entry>>,
+}
+
+// SAFETY: [`Entry`] is just a `dentry` under the hood, which the API promises can be transferred
+// between threads.
+unsafe impl Send for Entry {}
+
+// SAFETY: All the native functions we re-export use interior locking, and the contents of the
+// struct are opaque to Rust.
+unsafe impl Sync for Entry {}
+
+impl Entry {
+    /// Constructs a new DebugFS [`Entry`] 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. If this is a child directory or file, `'a` must be less than the
+    /// lifetime of the parent directory.
+    ///
+    /// If the dentry has a parent, it must be provided as the parent argument.
+    pub(crate) unsafe fn new(entry: *mut bindings::dentry, parent: Option<Arc<Entry>>) -> Self {
+        Self {
+            entry,
+            _parent: parent,
+        }
+    }
+
+    /// 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, NULL, or a live DebugFS directory.
+    pub(crate) fn as_ptr(&self) -> *mut bindings::dentry {
+        self.entry
+    }
+}
+
+impl Drop for Entry {
+    fn drop(&mut self) {
+        // 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 5bbf3627212f0a26d34be0d6c160a370abf1e996..8b4d7c95bcc895cf15544d9688941f93f2780510 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -67,6 +67,7 @@
 pub mod cpufreq;
 pub mod cpumask;
 pub mod cred;
+pub mod debugfs;
 pub mod device;
 pub mod device_id;
 pub mod devres;

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v9 2/5] 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 `Display` implementation is used because `seq_printf` needs to route
through `%pA`, which in turn routes through Arguments.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/debugfs.rs              | 62 ++++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/display_file.rs | 63 +++++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/entry.rs        |  8 +++++
 3 files changed, 133 insertions(+)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 2359bd11cd664fb9f7206f8fe38f758dc43d2cb8..e5b6497d1deb67671d22ffd90cd5baa855bb9257 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -10,7 +10,10 @@
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
+use core::fmt::Display;
 
+#[cfg(CONFIG_DEBUG_FS)]
+mod display_file;
 #[cfg(CONFIG_DEBUG_FS)]
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]
@@ -59,6 +62,43 @@ fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
         Self()
     }
 
+    #[cfg(CONFIG_DEBUG_FS)]
+    fn create_file<T: Display + Sized + Sync>(&self, name: &CStr, data: &'static T) -> File {
+        let Some(parent) = &self.0 else {
+            return File {
+                _entry: Entry::empty(),
+            };
+        };
+        // 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.
+        let ptr = unsafe {
+            bindings::debugfs_create_file_full(
+                name.as_char_ptr(),
+                0o444,
+                parent.as_ptr(),
+                data as *const _ as *mut _,
+                core::ptr::null(),
+                &<T as display_file::DisplayFile>::VTABLE,
+            )
+        };
+
+        // SAFETY: `debugfs_create_file_full` either returns an error code or a legal
+        // dentry pointer, so `Entry::new` is safe to call here.
+        let entry = unsafe { Entry::new(ptr, Some(parent.clone())) };
+
+        File { _entry: entry }
+    }
+
+    #[cfg(not(CONFIG_DEBUG_FS))]
+    fn create_file<T: Display + Sized + Sync>(&self, _name: &CStr, _data: &'static T) -> File {
+        File {}
+    }
+
     /// Create a DebugFS subdirectory.
     ///
     /// Subdirectory handles cannot outlive the directory handle they were created from.
@@ -75,6 +115,22 @@ pub fn subdir(&self, name: &CStr) -> Self {
         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<T: Display + Sized + Sync>(&self, name: &CStr, data: &'static T) -> File {
+        self.create_file(name, data)
+    }
+
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples
@@ -88,3 +144,9 @@ pub fn new(name: &CStr) -> Self {
         Dir::create(name, None)
     }
 }
+
+/// Handle to a DebugFS file.
+pub struct File {
+    #[cfg(CONFIG_DEBUG_FS)]
+    _entry: Entry,
+}
diff --git a/rust/kernel/debugfs/display_file.rs b/rust/kernel/debugfs/display_file.rs
new file mode 100644
index 0000000000000000000000000000000000000000..2a58ca2685258b050089e4cfd62188885f7f5f04
--- /dev/null
+++ b/rust/kernel/debugfs/display_file.rs
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+use crate::prelude::*;
+use crate::seq_file::SeqFile;
+use crate::seq_print;
+use core::fmt::Display;
+
+/// 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 have any unique references alias it during the call.
+/// * `file` must point to a live, not-yet-initialized file object.
+pub(crate) unsafe extern "C" fn display_open<T: Display + Sync>(
+    inode: *mut bindings::inode,
+    file: *mut bindings::file,
+) -> c_int {
+    // 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 may
+/// not have any unique references alias it during the call.
+pub(crate) unsafe extern "C" fn display_act<T: Display + Sync>(
+    seq: *mut bindings::seq_file,
+    _: *mut c_void,
+) -> c_int {
+    // SAFETY: By caller precondition, this pointer is live, points to a value of type `T`, and
+    // there are not and will not be any unique references until we are done.
+    let data = unsafe { &*((*seq).private 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 {
+    const VTABLE: bindings::file_operations;
+}
+
+impl<T: Display + Sync> DisplayFile for T {
+    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>),
+        // SAFETY: `file_operations` supports zeroes in all fields.
+        ..unsafe { core::mem::zeroed() }
+    };
+}
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
index ae0e2c4e1d58e878ebb081a71e4ac0f4a7d99b91..2baaf31c326c3071b92b5bc37552907fa1102246 100644
--- a/rust/kernel/debugfs/entry.rs
+++ b/rust/kernel/debugfs/entry.rs
@@ -38,6 +38,14 @@ pub(crate) unsafe fn new(entry: *mut bindings::dentry, parent: Option<Arc<Entry>
         }
     }
 
+    /// Constructs a placeholder DebugFS [`Entry`].
+    pub(crate) fn empty() -> Self {
+        Self {
+            entry: core::ptr::null_mut(),
+            _parent: None,
+        }
+    }
+
     /// Returns the pointer representation of the DebugFS directory.
     ///
     /// # Guarantees

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v9 3/5] rust: debugfs: Support `PinInit` backing for `File`s.

[Matthew Maurer]

This allows `File`s to own their data, allowing DebugFS files to be
managed in sync with the data that backs them. Because DebugFS files are
intended to actually own data and provide access, `File`s still maintain
the same lifecycle for provided data when `CONFIG_DEBUG_FS` is disabled.

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

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index e5b6497d1deb67671d22ffd90cd5baa855bb9257..a1a84dd309216f455ae8fe3d3c0fd00f957f82a9 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -5,12 +5,13 @@
 //!
 //! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
 
-#[cfg(CONFIG_DEBUG_FS)]
-use crate::prelude::GFP_KERNEL;
+use crate::prelude::*;
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
 use core::fmt::Display;
+use core::marker::PhantomPinned;
+use core::ops::Deref;
 
 #[cfg(CONFIG_DEBUG_FS)]
 mod display_file;
@@ -63,40 +64,78 @@ fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
     }
 
     #[cfg(CONFIG_DEBUG_FS)]
-    fn create_file<T: Display + Sized + Sync>(&self, name: &CStr, data: &'static T) -> File {
-        let Some(parent) = &self.0 else {
-            return File {
+    /// Creates a DebugFS file which will own the data produced by the initializer provided in
+    /// `data`.
+    ///
+    /// # Safety
+    ///
+    /// The provided vtable must be appropriate for implementing a seq_file if provided
+    /// with a private data pointer which provides shared access to a `T`.
+    unsafe fn create_file<'a, T: Sync, E, TI: PinInit<T, E>>(
+        &self,
+        name: &'a CStr,
+        data: TI,
+        vtable: &'static bindings::file_operations,
+    ) -> impl PinInit<File<T>, E> + use<'_, 'a, T, E, TI> {
+        try_pin_init! {
+            File {
                 _entry: Entry::empty(),
+                data <- data,
+                _pin: PhantomPinned,
+            } ? E
+        }
+        .pin_chain(|file| {
+            let Some(parent) = &self.0 else {
+                return Ok(());
             };
-        };
-        // 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.
-        let ptr = unsafe {
-            bindings::debugfs_create_file_full(
-                name.as_char_ptr(),
-                0o444,
-                parent.as_ptr(),
-                data as *const _ as *mut _,
-                core::ptr::null(),
-                &<T as display_file::DisplayFile>::VTABLE,
-            )
-        };
 
-        // SAFETY: `debugfs_create_file_full` either returns an error code or a legal
-        // dentry pointer, so `Entry::new` is safe to call here.
-        let entry = unsafe { Entry::new(ptr, Some(parent.clone())) };
+            // 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.
+            // * Since the file owns the `T` and it is pinned, we can safely assume the pointer
+            //   lives and is valid as long as we are.
+            // * Since the `Entry` will live in the `File`, it will be dropped before the pointer
+            //   is invalidated. Dropping the `Entry` will remove the DebugFS file and avoid
+            //   further access.
+            let ptr = unsafe {
+                bindings::debugfs_create_file_full(
+                    name.as_char_ptr(),
+                    0o444,
+                    parent.as_ptr(),
+                    &file.data as *const _ as *mut c_void,
+                    core::ptr::null(),
+                    vtable,
+                )
+            };
+
+            // SAFETY: `debugfs_create_file_full` either returns an error code or a legal
+            // dentry pointer, so `Entry::new` is safe to call here.
+            *file.entry_mut() = unsafe { Entry::new(ptr, Some(parent.clone())) };
 
-        File { _entry: entry }
+            Ok(())
+        })
     }
 
     #[cfg(not(CONFIG_DEBUG_FS))]
-    fn create_file<T: Display + Sized + Sync>(&self, _name: &CStr, _data: &'static T) -> File {
-        File {}
+    /// Creates a DebugFS file which will own the data produced by the initializer provided in
+    /// `data`.
+    ///
+    /// # Safety
+    ///
+    /// As DebugFS is disabled, this is actually entirely safe. It is marked unsafe for code
+    /// compatibility with the DebugFS-enabled variant.
+    unsafe fn create_file<'a, T: Sync, E, TI: PinInit<T, E>>(
+        &self,
+        _name: &'a CStr,
+        data: TI,
+        _vtable: (),
+    ) -> impl PinInit<File<T>, E> + use<'_, 'a, T, E, TI> {
+        try_pin_init! {
+            File {
+                data <- data,
+                _pin: PhantomPinned,
+            } ? E
+        }
     }
 
     /// Create a DebugFS subdirectory.
@@ -127,8 +166,32 @@ pub fn subdir(&self, name: &CStr) -> Self {
     /// dir.display_file(c_str!("foo"), &200);
     /// // "my_debugfs_dir/foo" now contains the number 200.
     /// ```
-    pub fn display_file<T: Display + Sized + Sync>(&self, name: &CStr, data: &'static T) -> File {
-        self.create_file(name, data)
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// # use kernel::prelude::*;
+    /// let val = KBox::new(300, GFP_KERNEL)?;
+    /// let dir = Dir::new(c_str!("my_debugfs_dir"));
+    /// dir.display_file(c_str!("foo"), val);
+    /// // "my_debugfs_dir/foo" now contains the number 300.
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn display_file<'b, T: Display + Send + Sync, E, TI: PinInit<T, E>>(
+        &self,
+        name: &'b CStr,
+        data: TI,
+    ) -> impl PinInit<File<T>, E> + use<'_, 'b, T, E, TI> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        let vtable = &<T as display_file::DisplayFile>::VTABLE;
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        let vtable = ();
+
+        // SAFETY: `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 is provided by
+        // `create_file`'s safety requirements.
+        unsafe { self.create_file(name, data, vtable) }
     }
 
     /// Create a new directory in DebugFS at the root.
@@ -146,7 +209,29 @@ pub fn new(name: &CStr) -> Self {
 }
 
 /// Handle to a DebugFS file.
-pub struct File {
+#[pin_data]
+pub struct File<T> {
+    // This order is load-bearing for drops - `_entry` must be dropped before `data`.
     #[cfg(CONFIG_DEBUG_FS)]
     _entry: Entry,
+    #[pin]
+    data: T,
+    // Even if `T` is `Unpin`, we still can't allow it to be moved.
+    #[pin]
+    _pin: PhantomPinned,
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+impl<T> File<T> {
+    fn entry_mut(self: Pin<&mut Self>) -> &mut Entry {
+        // SAFETY: _entry is not structurally pinned
+        unsafe { &mut Pin::into_inner_unchecked(self)._entry }
+    }
+}
+
+impl<T> Deref for File<T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.data
+    }
 }

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v9 4/5] 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              | 49 +++++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/display_file.rs | 39 ++++++++++++++++++++++++++++-
 2 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index a1a84dd309216f455ae8fe3d3c0fd00f957f82a9..083c49007cd7ae5b3d7954bf859c24b7eb62d557 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -9,6 +9,7 @@
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
+use core::fmt;
 use core::fmt::Display;
 use core::marker::PhantomPinned;
 use core::ops::Deref;
@@ -194,6 +195,54 @@ pub fn display_file<'b, T: Display + Send + Sync, E, TI: PinInit<T, E>>(
         unsafe { self.create_file(name, data, vtable) }
     }
 
+    /// 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: Send + Sync,
+        E,
+        TI: PinInit<T, E>,
+        F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+    >(
+        &self,
+        name: &'b CStr,
+        data: TI,
+        _f: &'static F,
+    ) -> impl PinInit<File<T>, E> + use<'_, 'b, T, TI, E, F> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        let vtable = &<display_file::FormatAdapter<T, F> as display_file::DisplayFile>::VTABLE;
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        let vtable = ();
+
+        // SAFETY: `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 `FormatAdapter<T, F>` that will outlive it.
+        // `create_file`'s safety requirements provide the lifetime aspect of this, but we are
+        // using a private `T` pointer. This is legal because:
+        // 1. `FormatAdapter<T, F>` is a `#[repr(transparent)]` wrapper around `T`, so the
+        //    implicit transmute is legal.
+        // 2. The invariant in `FormatAdapter` that `F` is inhabited is upheld because we have
+        //    `_f`, so constructing a `FormatAdapter<T, F> is legal.
+        unsafe { self.create_file(name, data, vtable) }
+    }
+
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples
diff --git a/rust/kernel/debugfs/display_file.rs b/rust/kernel/debugfs/display_file.rs
index 2a58ca2685258b050089e4cfd62188885f7f5f04..6275283b9dabd8dae84a9335c8832e7943707d56 100644
--- a/rust/kernel/debugfs/display_file.rs
+++ b/rust/kernel/debugfs/display_file.rs
@@ -4,7 +4,8 @@
 use crate::prelude::*;
 use crate::seq_file::SeqFile;
 use crate::seq_print;
-use core::fmt::Display;
+use core::fmt::{Display, Formatter, Result};
+use core::marker::PhantomData;
 
 /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
 ///
@@ -61,3 +62,39 @@ impl<T: Display + Sync> DisplayFile for T {
         ..unsafe { core::mem::zeroed() }
     };
 }
+
+/// 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<D, F> {
+    inner: D,
+    _formatter: PhantomData<F>,
+}
+
+impl<D, F> Display for FormatAdapter<D, F>
+where
+    F: Fn(&D, &mut Formatter<'_>) -> Result + 'static,
+{
+    fn fmt(&self, fmt: &mut Formatter<'_>) -> 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() }
+}

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v9 5/5] 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 | 182 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 195 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1427af9d9779b1f6463409f4392e2900438bdc2a..0d9cb77b54b4608a0e1006ae45761ed7440495ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7376,6 +7376,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 7f7371a004ee0a8f67dca99c836596709a70c4fa..01101db41ae31b08a86d048cdd27da8ef9bb23a2 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -62,6 +62,17 @@ config SAMPLE_RUST_DMA
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DEBUGFS
+	tristate "DebugFS Test Module"
+	depends on DEBUG_FS
+	help
+	  This option builds the Rust DebugFS Test module 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 bd2faad63b4f3befe7d1ed5139fe25c7a8b6d7f6..61276222a99f8cc6d7f84c26d0533b30815ebadd 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..21fbf26f7ec07fabad782915046da3cdf52b03b3
--- /dev/null
+++ b/samples/rust/rust_debugfs.rs
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Sample DebugFS exporting platform driver
+//!
+//! To successfully probe this driver with ACPI, use an ssdt that looks like
+//!
+//! ```dsl
+//! DefinitionBlock ("", "SSDT", 2, "TEST", "VIRTACPI", 0x00000001)
+//!{
+//!    Scope (\_SB)
+//!    {
+//!        Device (T432)
+//!        {
+//!            Name (_HID, "LNUXDEBF")  // ACPI hardware ID to match
+//!            Name (_UID, 1)
+//!            Name (_STA, 0x0F)        // Device present, enabled
+//!            Name (_DSD, Package () { // Sample attribute
+//!                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+//!                Package() {
+//!                    Package(2) {"compatible", "sample-debugfs"}
+//!                }
+//!            })
+//!            Name (_CRS, ResourceTemplate ()
+//!            {
+//!                Memory32Fixed (ReadWrite, 0xFED00000, 0x1000)
+//!            })
+//!        }
+//!    }
+//!}
+//! ```
+
+use core::sync::atomic::AtomicUsize;
+use core::sync::atomic::Ordering;
+use kernel::c_str;
+use kernel::debugfs::{Dir, File};
+use kernel::new_mutex;
+use kernel::prelude::*;
+use kernel::sync::Mutex;
+
+use kernel::{acpi, device::Core, of, platform, str::CString, types::ARef};
+
+kernel::module_platform_driver! {
+    type: Wrapper,
+    name: "rust_debugfs",
+    authors: ["Matthew Maurer"],
+    description: "Rust DebugFS usage sample",
+    license: "GPL",
+}
+
+// This data structure would be unlikely to be there in a real driver - it's to hook up mutation
+// that would normally be driven by whatever the driver was actually servicing and show how that
+// would work. We're assuming here that those methods would have access to a `&RustDebugFs`.
+#[pin_data]
+struct Wrapper {
+    _dir: Dir,
+    #[pin]
+    _wrapped: File<File<RustDebugFs>>,
+}
+
+#[pin_data]
+struct RustDebugFs {
+    pdev: ARef<platform::Device>,
+    // As we only hold these for drop effect (to remove the directory/files) we have a leading
+    // underscore to indicate to the compiler that we don't expect to use this field directly.
+    _debugfs: Dir,
+    #[pin]
+    _compatible: File<CString>,
+    #[pin]
+    counter: File<File<AtomicUsize>>,
+    #[pin]
+    inner: File<Mutex<Inner>>,
+}
+
+#[derive(Debug)]
+struct Inner {
+    x: u32,
+    y: u32,
+}
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <Wrapper as platform::Driver>::IdInfo,
+    [(of::DeviceId::new(c_str!("test,rust-debugfs-device")), ())]
+);
+
+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <Wrapper as platform::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]
+);
+
+impl platform::Driver for Wrapper {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
+
+    fn probe(
+        pdev: &platform::Device<Core>,
+        _info: Option<&Self::IdInfo>,
+    ) -> Result<Pin<KBox<Self>>> {
+        KBox::try_pin_init(Wrapper::new(RustDebugFs::new(pdev)), GFP_KERNEL)
+    }
+}
+
+impl Wrapper {
+    /// This builds two debugfs files that would be unusual to exist in the real world to emulate
+    /// actions taken servicing the device. They trigger their action when the debugfs file is
+    /// opened.
+    fn build_control<I: PinInit<RustDebugFs, Error>>(
+        dir: &Dir,
+        init: I,
+    ) -> impl PinInit<File<File<RustDebugFs>>, Error> + use<'_, I> {
+        let swap = dir.fmt_file(c_str!("swap"), init, &|sample, fmt| {
+            let mut guard = sample.inner.lock();
+            let x = guard.x;
+            guard.x = guard.y;
+            guard.y = x;
+            writeln!(fmt, "Swapped!")
+        });
+
+        dir.fmt_file(c_str!("add_counter"), swap, &|sample, fmt| {
+            let mut inner = sample.inner.lock();
+            inner.x += sample.counter.load(Ordering::Relaxed) as u32;
+            writeln!(fmt, "Counter added!")
+        })
+    }
+
+    fn new<I: PinInit<RustDebugFs, Error>>(init: I) -> impl PinInit<Self, Error> + use<I> {
+        let dir = Dir::new(c_str!("sample_control"));
+        try_pin_init! {
+            Self {
+                _wrapped <- Wrapper::build_control(&dir, init),
+                _dir: dir,
+            } ? Error
+        }
+    }
+}
+
+impl RustDebugFs {
+    fn build_counter(dir: &Dir) -> impl PinInit<File<File<AtomicUsize>>> + use<'_> {
+        let counter = dir.fmt_file(c_str!("counter"), AtomicUsize::new(0), &|counter, fmt| {
+            writeln!(fmt, "{}", counter.load(Ordering::Relaxed))
+        });
+        dir.fmt_file(c_str!("inc_counter"), counter, &|counter, fmt| {
+            writeln!(fmt, "{}", counter.fetch_add(1, Ordering::Relaxed))
+        })
+    }
+
+    fn build_inner(dir: &Dir) -> impl PinInit<File<Mutex<Inner>>> + use<'_> {
+        dir.fmt_file(
+            c_str!("pair"),
+            new_mutex!(Inner { x: 3, y: 10 }),
+            &|i, fmt| writeln!(fmt, "{:?}", *i.lock()),
+        )
+    }
+
+    fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + use<'_> {
+        let debugfs = Dir::new(c_str!("sample_debugfs"));
+        let dev = pdev.as_ref();
+
+        try_pin_init! {
+            Self {
+                _compatible <- debugfs.fmt_file(
+                    c_str!("compatible"),
+                    dev.fwnode()
+                        .ok_or(ENOENT)?
+                        .property_read::<CString>(c_str!("compatible"))
+                        .required_by(dev)?,
+                    &|cs, w| writeln!(w, "{cs:?}"),
+                ),
+                counter <- Self::build_counter(&debugfs),
+                inner <- Self::build_inner(&debugfs),
+                _debugfs: debugfs,
+                pdev: pdev.into(),
+            }
+        }
+    }
+}

-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Danilo Krummrich]

On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> This series provides safe DebugFS bindings for Rust, with a sample
> module using them.
>
> Example interaction with the sample driver:

I understand what you're trying to do here, i.e. showcase that values exported
via debugfs can be altered.

The problem is that the current abstractions only implement read(), but not
write().

So, to work around this you create multiple files for a single value: one that
only prints the value on read() and one that modifies the value on read().

For instance, you have a value counter (AtomicUsize), which is exported as:

	$(DEBUGFS_ROOT)/counter
	$(DEBUGFS_ROOT)/inc_counter

where 

	$ cat $(DEBUGFS_ROOT)/counter

prints the value and

	$ cat $(DEBUGFS_ROOT)/inc_counter

increments the counter.

While this is technically not wrong it's providing bad guidance to people.

Instead this should be something along the lines of:

	$ cat $(DEBUGFS_ROOT)/counter
	0

	$ echo "++" > $(DEBUGFS_ROOT)/counter
	$ cat $(DEBUGFS_ROOT)/counter
	1

	$ echo "42" > $(DEBUGFS_ROOT)/counter
	$ cat $(DEBUGFS_ROOT)/counter
	42

Given that the abstraction doesn't support write() yet, just don't try to work
around it.

If you really want to showcase changing values, you can, for instance, create a
workqueue inside the sample driver and modify the counter periodically.

We really should not teach people to modify values by read() instead of write().
Also, without this workaround there shouldn't be a reason to export the exact
same value twice, i.e. no need for File<File<AtomicUsize>>.

- Danilo

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Matthew Maurer]

On Wed, Jul 9, 2025 at 2:47 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> > This series provides safe DebugFS bindings for Rust, with a sample
> > module using them.
> >
> > Example interaction with the sample driver:
>
> I understand what you're trying to do here, i.e. showcase that values exported
> via debugfs can be altered.
>
> The problem is that the current abstractions only implement read(), but not
> write().

I was trying to keep the initial bindings simple. Adding `write` is
definitely something we could do, but I thought maybe that could be in
a subsequent patch.

> If you really want to showcase changing values, you can, for instance, create a
> workqueue inside the sample driver and modify the counter periodically.

This is supposed to be sample code, so ideally it should be as narrow
as is reasonable in what subsystems it touches, no? If people would
really prefer the sample schedule a ticking counter I can do that, but
it already felt weird to be registering a platform driver in a debugfs
sample.

>
> We really should not teach people to modify values by read() instead of write().
> Also, without this workaround there shouldn't be a reason to export the exact
> same value twice, i.e. no need for File<File<AtomicUsize>>.
>
> - Danilo

How do you feel about the `Wrapper` struct, intended to simulate the
driver doing its actual job and show how that would look? Is that
similarly verboten, even though there's a comment on it saying this
isn't how one should do things?

Re: [PATCH v9 5/5] rust: samples: Add debugfs sample

[Danilo Krummrich]

On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> +// This data structure would be unlikely to be there in a real driver - it's to hook up mutation
> +// that would normally be driven by whatever the driver was actually servicing and show how that
> +// would work. We're assuming here that those methods would have access to a `&RustDebugFs`.

Please see also [1]. I think you're making this too complicated, and due to the
missing write() support you have to create workarounds because of that, which
serve as a bad reference.

Keep it simple, Create some driver private data in probe() and export a couple
of fields of this driver private data through debugfs.

If you really want to showcase that those values can change, queue some work and
modify the counter and / or the Inner type that is protected by a mutex.

[1] https://lore.kernel.org/lkml/DB7US8G7ISG0.20430M3P7I0K0@kernel.org/

> +#[pin_data]
> +struct Wrapper {
> +    _dir: Dir,
> +    #[pin]
> +    _wrapped: File<File<RustDebugFs>>,
> +}
> +
> +#[pin_data]
> +struct RustDebugFs {
> +    pdev: ARef<platform::Device>,
> +    // As we only hold these for drop effect (to remove the directory/files) we have a leading
> +    // underscore to indicate to the compiler that we don't expect to use this field directly.
> +    _debugfs: Dir,
> +    #[pin]
> +    _compatible: File<CString>,
> +    #[pin]
> +    counter: File<File<AtomicUsize>>,
> +    #[pin]
> +    inner: File<Mutex<Inner>>,
> +}
> +
> +#[derive(Debug)]
> +struct Inner {
> +    x: u32,
> +    y: u32,
> +}
> +
> +kernel::of_device_table!(
> +    OF_TABLE,
> +    MODULE_OF_TABLE,
> +    <Wrapper as platform::Driver>::IdInfo,
> +    [(of::DeviceId::new(c_str!("test,rust-debugfs-device")), ())]
> +);

I don't think we need both, ACPI should be much simpler with QEMU.

> +kernel::acpi_device_table!(
> +    ACPI_TABLE,
> +    MODULE_ACPI_TABLE,
> +    <Wrapper as platform::Driver>::IdInfo,
> +    [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]
> +);
> +
> +impl platform::Driver for Wrapper {
> +    type IdInfo = ();
> +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> +    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
> +
> +    fn probe(
> +        pdev: &platform::Device<Core>,
> +        _info: Option<&Self::IdInfo>,
> +    ) -> Result<Pin<KBox<Self>>> {
> +        KBox::try_pin_init(Wrapper::new(RustDebugFs::new(pdev)), GFP_KERNEL)
> +    }
> +}

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Danilo Krummrich]

On Wed Jul 9, 2025 at 11:53 PM CEST, Matthew Maurer wrote:
> On Wed, Jul 9, 2025 at 2:47 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
>> > This series provides safe DebugFS bindings for Rust, with a sample
>> > module using them.
>> >
>> > Example interaction with the sample driver:
>>
>> I understand what you're trying to do here, i.e. showcase that values exported
>> via debugfs can be altered.
>>
>> The problem is that the current abstractions only implement read(), but not
>> write().
>
> I was trying to keep the initial bindings simple. Adding `write` is
> definitely something we could do, but I thought maybe that could be in
> a subsequent patch.

Absolutely, yes! I didn't mean to ask to add it now. :)

>> If you really want to showcase changing values, you can, for instance, create a
>> workqueue inside the sample driver and modify the counter periodically.
>
> This is supposed to be sample code, so ideally it should be as narrow
> as is reasonable in what subsystems it touches, no? If people would
> really prefer the sample schedule a ticking counter I can do that, but
> it already felt weird to be registering a platform driver in a debugfs
> sample.

I'm not asking to do that. If the values don't change for now, because
there's no write() yet, that's perfectly fine with me. :)

>>
>> We really should not teach people to modify values by read() instead of write().
>> Also, without this workaround there shouldn't be a reason to export the exact
>> same value twice, i.e. no need for File<File<AtomicUsize>>.
>>
>> - Danilo
>
> How do you feel about the `Wrapper` struct, intended to simulate the
> driver doing its actual job and show how that would look? Is that
> similarly verboten, even though there's a comment on it saying this
> isn't how one should do things?

Yeah, let's not do that -- don't give people ideas. :)

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Matthew Maurer]

On Wed, Jul 9, 2025 at 2:59 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Jul 9, 2025 at 11:53 PM CEST, Matthew Maurer wrote:
> > On Wed, Jul 9, 2025 at 2:47 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> >> > This series provides safe DebugFS bindings for Rust, with a sample
> >> > module using them.
> >> >
> >> > Example interaction with the sample driver:
> >>
> >> I understand what you're trying to do here, i.e. showcase that values exported
> >> via debugfs can be altered.
> >>
> >> The problem is that the current abstractions only implement read(), but not
> >> write().
> >
> > I was trying to keep the initial bindings simple. Adding `write` is
> > definitely something we could do, but I thought maybe that could be in
> > a subsequent patch.
>
> Absolutely, yes! I didn't mean to ask to add it now. :)
>
> >> If you really want to showcase changing values, you can, for instance, create a
> >> workqueue inside the sample driver and modify the counter periodically.
> >
> > This is supposed to be sample code, so ideally it should be as narrow
> > as is reasonable in what subsystems it touches, no? If people would
> > really prefer the sample schedule a ticking counter I can do that, but
> > it already felt weird to be registering a platform driver in a debugfs
> > sample.
>
> I'm not asking to do that. If the values don't change for now, because
> there's no write() yet, that's perfectly fine with me. :)

Potentially I misinterpreted Greg[1], I thought he wanted to see how
mutation would work.
If we don't need mutation, I'm fine simplifying the driver to not have
any mutation triggers and just export a few random things.

[1]: https://lore.kernel.org/all/2025070349-tricky-arguable-5362@gregkh/

>
> >>
> >> We really should not teach people to modify values by read() instead of write().
> >> Also, without this workaround there shouldn't be a reason to export the exact
> >> same value twice, i.e. no need for File<File<AtomicUsize>>.
> >>
> >> - Danilo
> >
> > How do you feel about the `Wrapper` struct, intended to simulate the
> > driver doing its actual job and show how that would look? Is that
> > similarly verboten, even though there's a comment on it saying this
> > isn't how one should do things?
>
> Yeah, let's not do that -- don't give people ideas. :)

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Danilo Krummrich]

On Wed, Jul 09, 2025 at 03:04:51PM -0700, Matthew Maurer wrote:
> On Wed, Jul 9, 2025 at 2:59 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Wed Jul 9, 2025 at 11:53 PM CEST, Matthew Maurer wrote:
> > > On Wed, Jul 9, 2025 at 2:47 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > >>
> > >> On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> > >> > This series provides safe DebugFS bindings for Rust, with a sample
> > >> > module using them.
> > >> >
> > >> > Example interaction with the sample driver:
> > >>
> > >> I understand what you're trying to do here, i.e. showcase that values exported
> > >> via debugfs can be altered.
> > >>
> > >> The problem is that the current abstractions only implement read(), but not
> > >> write().
> > >
> > > I was trying to keep the initial bindings simple. Adding `write` is
> > > definitely something we could do, but I thought maybe that could be in
> > > a subsequent patch.
> >
> > Absolutely, yes! I didn't mean to ask to add it now. :)
> >
> > >> If you really want to showcase changing values, you can, for instance, create a
> > >> workqueue inside the sample driver and modify the counter periodically.
> > >
> > > This is supposed to be sample code, so ideally it should be as narrow
> > > as is reasonable in what subsystems it touches, no? If people would
> > > really prefer the sample schedule a ticking counter I can do that, but
> > > it already felt weird to be registering a platform driver in a debugfs
> > > sample.
> >
> > I'm not asking to do that. If the values don't change for now, because
> > there's no write() yet, that's perfectly fine with me. :)
> 
> Potentially I misinterpreted Greg[1], I thought he wanted to see how
> mutation would work.
> If we don't need mutation, I'm fine simplifying the driver to not have
> any mutation triggers and just export a few random things.

I mean, the most simple way would be to create the debugfs entries in probe()
and mutate them - still in probe() - right afterwards once. I think we should
do in any case. And AFAICT, this also covers [1].

> [1]: https://lore.kernel.org/all/2025070349-tricky-arguable-5362@gregkh/
> 
> >
> > >>
> > >> We really should not teach people to modify values by read() instead of write().
> > >> Also, without this workaround there shouldn't be a reason to export the exact
> > >> same value twice, i.e. no need for File<File<AtomicUsize>>.
> > >>
> > >> - Danilo
> > >
> > > How do you feel about the `Wrapper` struct, intended to simulate the
> > > driver doing its actual job and show how that would look? Is that
> > > similarly verboten, even though there's a comment on it saying this
> > > isn't how one should do things?
> >
> > Yeah, let's not do that -- don't give people ideas. :)

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Matthew Maurer]

On Wed, Jul 9, 2025 at 3:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed, Jul 09, 2025 at 03:04:51PM -0700, Matthew Maurer wrote:
> > On Wed, Jul 9, 2025 at 2:59 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > >
> > > On Wed Jul 9, 2025 at 11:53 PM CEST, Matthew Maurer wrote:
> > > > On Wed, Jul 9, 2025 at 2:47 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > > >>
> > > >> On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> > > >> > This series provides safe DebugFS bindings for Rust, with a sample
> > > >> > module using them.
> > > >> >
> > > >> > Example interaction with the sample driver:
> > > >>
> > > >> I understand what you're trying to do here, i.e. showcase that values exported
> > > >> via debugfs can be altered.
> > > >>
> > > >> The problem is that the current abstractions only implement read(), but not
> > > >> write().
> > > >
> > > > I was trying to keep the initial bindings simple. Adding `write` is
> > > > definitely something we could do, but I thought maybe that could be in
> > > > a subsequent patch.
> > >
> > > Absolutely, yes! I didn't mean to ask to add it now. :)
> > >
> > > >> If you really want to showcase changing values, you can, for instance, create a
> > > >> workqueue inside the sample driver and modify the counter periodically.
> > > >
> > > > This is supposed to be sample code, so ideally it should be as narrow
> > > > as is reasonable in what subsystems it touches, no? If people would
> > > > really prefer the sample schedule a ticking counter I can do that, but
> > > > it already felt weird to be registering a platform driver in a debugfs
> > > > sample.
> > >
> > > I'm not asking to do that. If the values don't change for now, because
> > > there's no write() yet, that's perfectly fine with me. :)
> >
> > Potentially I misinterpreted Greg[1], I thought he wanted to see how
> > mutation would work.
> > If we don't need mutation, I'm fine simplifying the driver to not have
> > any mutation triggers and just export a few random things.
>
> I mean, the most simple way would be to create the debugfs entries in probe()
> and mutate them - still in probe() - right afterwards once. I think we should
> do in any case. And AFAICT, this also covers [1].

That's what I did with my `InPlaceModule` before and it evidently
didn't count? I don't see how the constructor being `probe` rather
than `init` would have been the issue - the only change that causes is
calling `KBox::pin_init` on the value you would have returned.

>
> > [1]: https://lore.kernel.org/all/2025070349-tricky-arguable-5362@gregkh/
> >
> > >
> > > >>
> > > >> We really should not teach people to modify values by read() instead of write().
> > > >> Also, without this workaround there shouldn't be a reason to export the exact
> > > >> same value twice, i.e. no need for File<File<AtomicUsize>>.
> > > >>
> > > >> - Danilo
> > > >
> > > > How do you feel about the `Wrapper` struct, intended to simulate the
> > > > driver doing its actual job and show how that would look? Is that
> > > > similarly verboten, even though there's a comment on it saying this
> > > > isn't how one should do things?
> > >
> > > Yeah, let's not do that -- don't give people ideas. :)

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Danilo Krummrich]

On Thu Jul 10, 2025 at 12:21 AM CEST, Matthew Maurer wrote:
> On Wed, Jul 9, 2025 at 3:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Wed, Jul 09, 2025 at 03:04:51PM -0700, Matthew Maurer wrote:
>> > On Wed, Jul 9, 2025 at 2:59 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> > >
>> > > On Wed Jul 9, 2025 at 11:53 PM CEST, Matthew Maurer wrote:
>> > > > On Wed, Jul 9, 2025 at 2:47 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> > > >>
>> > > >> On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
>> > > >> > This series provides safe DebugFS bindings for Rust, with a sample
>> > > >> > module using them.
>> > > >> >
>> > > >> > Example interaction with the sample driver:
>> > > >>
>> > > >> I understand what you're trying to do here, i.e. showcase that values exported
>> > > >> via debugfs can be altered.
>> > > >>
>> > > >> The problem is that the current abstractions only implement read(), but not
>> > > >> write().
>> > > >
>> > > > I was trying to keep the initial bindings simple. Adding `write` is
>> > > > definitely something we could do, but I thought maybe that could be in
>> > > > a subsequent patch.
>> > >
>> > > Absolutely, yes! I didn't mean to ask to add it now. :)
>> > >
>> > > >> If you really want to showcase changing values, you can, for instance, create a
>> > > >> workqueue inside the sample driver and modify the counter periodically.
>> > > >
>> > > > This is supposed to be sample code, so ideally it should be as narrow
>> > > > as is reasonable in what subsystems it touches, no? If people would
>> > > > really prefer the sample schedule a ticking counter I can do that, but
>> > > > it already felt weird to be registering a platform driver in a debugfs
>> > > > sample.
>> > >
>> > > I'm not asking to do that. If the values don't change for now, because
>> > > there's no write() yet, that's perfectly fine with me. :)
>> >
>> > Potentially I misinterpreted Greg[1], I thought he wanted to see how
>> > mutation would work.
>> > If we don't need mutation, I'm fine simplifying the driver to not have
>> > any mutation triggers and just export a few random things.
>>
>> I mean, the most simple way would be to create the debugfs entries in probe()
>> and mutate them - still in probe() - right afterwards once. I think we should
>> do in any case. And AFAICT, this also covers [1].
>
> That's what I did with my `InPlaceModule` before and it evidently
> didn't count? I don't see how the constructor being `probe` rather
> than `init` would have been the issue - the only change that causes is
> calling `KBox::pin_init` on the value you would have returned.

Who said this didn't count? It makes no difference from where you mutate it, the
importent part is that you show that you can, and that is clearly covered.

The discussion you're linking to in [1] has a different context. It was about
exporting multiple values that are behind a single lock individually. And we
concluded that for the reasons mentioned in [2] it's not desirable.

[2] https://lore.kernel.org/lkml/aGZ3q0PEmZ7lV4I-@pollux/

>> > [1]: https://lore.kernel.org/all/2025070349-tricky-arguable-5362@gregkh/
>> >
>> > >
>> > > >>
>> > > >> We really should not teach people to modify values by read() instead of write().
>> > > >> Also, without this workaround there shouldn't be a reason to export the exact
>> > > >> same value twice, i.e. no need for File<File<AtomicUsize>>.
>> > > >>
>> > > >> - Danilo
>> > > >
>> > > > How do you feel about the `Wrapper` struct, intended to simulate the
>> > > > driver doing its actual job and show how that would look? Is that
>> > > > similarly verboten, even though there's a comment on it saying this
>> > > > isn't how one should do things?
>> > >
>> > > Yeah, let's not do that -- don't give people ideas. :)

Re: [PATCH v9 5/5] rust: samples: Add debugfs sample

[Matthew Maurer]

> Keep it simple, Create some driver private data in probe() and export a couple
> of fields of this driver private data through debugfs.
>
> If you really want to showcase that those values can change, queue some work and
> modify the counter and / or the Inner type that is protected by a mutex.

I've included an example of what a simplified sample of the sort you
asked for might look like below. I'll include it in the updated series
unless someone wants the original functionality.

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 | 136 +++++++++++++++++++++++++++++++++++
 4 files changed, 149 insertions(+)
 create mode 100644 samples/rust/rust_debugfs.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 1427af9d9779..0d9cb77b54b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7376,6 +7376,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 7f7371a004ee..01101db41ae3 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -62,6 +62,17 @@ config SAMPLE_RUST_DMA

          If unsure, say N.

+config SAMPLE_RUST_DEBUGFS
+       tristate "DebugFS Test Module"
+       depends on DEBUG_FS
+       help
+         This option builds the Rust DebugFS Test module 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 bd2faad63b4f..61276222a99f 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 000000000000..47b337f2e960
--- /dev/null
+++ b/samples/rust/rust_debugfs.rs
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Sample DebugFS exporting platform driver
+//!
+//! To successfully probe this driver with ACPI, use an ssdt that looks like
+//!
+//! ```dsl
+//! DefinitionBlock ("", "SSDT", 2, "TEST", "VIRTACPI", 0x00000001)
+//! {
+//!    Scope (\_SB)
+//!    {
+//!        Device (T432)
+//!        {
+//!            Name (_HID, "LNUXDEBF")  // ACPI hardware ID to match
+//!            Name (_UID, 1)
+//!            Name (_STA, 0x0F)        // Device present, enabled
+//!            Name (_DSD, Package () { // Sample attribute
+//!                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+//!                Package() {
+//!                    Package(2) {"compatible", "sample-debugfs"}
+//!                }
+//!            })
+//!            Name (_CRS, ResourceTemplate ()
+//!            {
+//!                Memory32Fixed (ReadWrite, 0xFED00000, 0x1000)
+//!            })
+//!        }
+//!    }
+//! }
+//! ```
+
+use core::sync::atomic::AtomicUsize;
+use core::sync::atomic::Ordering;
+use kernel::c_str;
+use kernel::debugfs::{Dir, File};
+use kernel::new_mutex;
+use kernel::prelude::*;
+use kernel::sync::Mutex;
+
+use kernel::{acpi, device::Core, of, platform, str::CString, types::ARef};
+
+kernel::module_platform_driver! {
+    type: RustDebugFs,
+    name: "rust_debugfs",
+    authors: ["Matthew Maurer"],
+    description: "Rust DebugFS usage sample",
+    license: "GPL",
+}
+
+#[pin_data]
+struct RustDebugFs {
+    pdev: ARef<platform::Device>,
+    // As we only hold these for drop effect (to remove the
directory/files) we have a leading
+    // underscore to indicate to the compiler that we don't expect to
use this field directly.
+    _debugfs: Dir,
+    #[pin]
+    _compatible: File<CString>,
+    #[pin]
+    counter: File<AtomicUsize>,
+    #[pin]
+    inner: File<Mutex<Inner>>,
+}
+
+#[derive(Debug)]
+struct Inner {
+    x: u32,
+    y: u32,
+}
+
+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <RustDebugFs as platform::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]
+);
+
+impl platform::Driver for RustDebugFs {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> =
Some(&ACPI_TABLE);
+
+    fn probe(
+        pdev: &platform::Device<Core>,
+        _info: Option<&Self::IdInfo>,
+    ) -> Result<Pin<KBox<Self>>> {
+        let result = KBox::try_pin_init(RustDebugFs::new(pdev), GFP_KERNEL)?;
+        // We can still mutate fields through the files which are
atomic or mutexed:
+        result.counter.store(91, Ordering::Relaxed);
+        {
+            let mut guard = result.inner.lock();
+            guard.x = guard.y;
+            guard.y = 42;
+        }
+        Ok(result)
+    }
+}
+
+impl RustDebugFs {
+    fn build_counter(dir: &Dir) -> impl PinInit<File<AtomicUsize>> + use<'_> {
+        dir.fmt_file(c_str!("counter"), AtomicUsize::new(0), &|counter, fmt| {
+            writeln!(fmt, "{}", counter.load(Ordering::Relaxed))
+        })
+    }
+
+    fn build_inner(dir: &Dir) -> impl PinInit<File<Mutex<Inner>>> + use<'_> {
+        dir.fmt_file(
+            c_str!("pair"),
+            new_mutex!(Inner { x: 3, y: 10 }),
+            &|i, fmt| writeln!(fmt, "{:?}", *i.lock()),
+        )
+    }
+
+    fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self,
Error> + use<'_> {
+        let debugfs = Dir::new(c_str!("sample_debugfs"));
+        let dev = pdev.as_ref();
+
+        try_pin_init! {
+            Self {
+                _compatible <- debugfs.fmt_file(
+                    c_str!("compatible"),
+                    dev.fwnode()
+                        .ok_or(ENOENT)?
+                        .property_read::<CString>(c_str!("compatible"))
+                        .required_by(dev)?,
+                    &|cs, w| writeln!(w, "{cs:?}"),
+                ),
+                counter <- Self::build_counter(&debugfs),
+                inner <- Self::build_inner(&debugfs),
+                _debugfs: debugfs,
+                pdev: pdev.into(),
+            }
+        }
+    }
+}
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Greg Kroah-Hartman]

On Thu, Jul 10, 2025 at 12:12:15AM +0200, Danilo Krummrich wrote:
> On Wed, Jul 09, 2025 at 03:04:51PM -0700, Matthew Maurer wrote:
> > On Wed, Jul 9, 2025 at 2:59 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > >
> > > On Wed Jul 9, 2025 at 11:53 PM CEST, Matthew Maurer wrote:
> > > > On Wed, Jul 9, 2025 at 2:47 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > > >>
> > > >> On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> > > >> > This series provides safe DebugFS bindings for Rust, with a sample
> > > >> > module using them.
> > > >> >
> > > >> > Example interaction with the sample driver:
> > > >>
> > > >> I understand what you're trying to do here, i.e. showcase that values exported
> > > >> via debugfs can be altered.
> > > >>
> > > >> The problem is that the current abstractions only implement read(), but not
> > > >> write().
> > > >
> > > > I was trying to keep the initial bindings simple. Adding `write` is
> > > > definitely something we could do, but I thought maybe that could be in
> > > > a subsequent patch.
> > >
> > > Absolutely, yes! I didn't mean to ask to add it now. :)
> > >
> > > >> If you really want to showcase changing values, you can, for instance, create a
> > > >> workqueue inside the sample driver and modify the counter periodically.
> > > >
> > > > This is supposed to be sample code, so ideally it should be as narrow
> > > > as is reasonable in what subsystems it touches, no? If people would
> > > > really prefer the sample schedule a ticking counter I can do that, but
> > > > it already felt weird to be registering a platform driver in a debugfs
> > > > sample.
> > >
> > > I'm not asking to do that. If the values don't change for now, because
> > > there's no write() yet, that's perfectly fine with me. :)
> > 
> > Potentially I misinterpreted Greg[1], I thought he wanted to see how
> > mutation would work.
> > If we don't need mutation, I'm fine simplifying the driver to not have
> > any mutation triggers and just export a few random things.
> 
> I mean, the most simple way would be to create the debugfs entries in probe()
> and mutate them - still in probe() - right afterwards once. I think we should
> do in any case. And AFAICT, this also covers [1].
> 
> > [1]: https://lore.kernel.org/all/2025070349-tricky-arguable-5362@gregkh/

Ugh.

Yes we need write.  And read, and custom file-ops, and the like as
that's what debugfs is doing today for C code!  We need this to be as
simple as, or almost as simple as, what we have today in C or no one is
going to use this stuff and go off and attempt to write their own mess.

While I would love to have something as simple as:
	void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
like we do today.  I understand that this makes all sorts of
"assumptions" that Rust really doesn't like (i.e. lifetime of *value and
the like), BUT we MUST have something like this for Rust users, as
that's going to ensure that people actually use this api.

Look at an in-kernel function today, like ath9k_init_debug() that
creates a metric-ton of debugfs files and binds them to different
variables that are owned by a structure and more complex data structures
and memory dumps and other random file interactions.  We need, in Rust,
a way to do everything that that function can do today, in a SIMPLE
manner that reads just as easily as ath9k_init_debug() does.

So no "we will add write support later" stuff, sorry, real drivers
require write support in debugfs.

thanks,

greg k-h

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Danilo Krummrich]

On Thu Jul 10, 2025 at 7:27 AM CEST, Greg Kroah-Hartman wrote:
> Ugh.
>
> Yes we need write.  And read, and custom file-ops, and the like as
> that's what debugfs is doing today for C code!  We need this to be as
> simple as, or almost as simple as, what we have today in C or no one is
> going to use this stuff and go off and attempt to write their own mess.

I agree, we really want the helpers you're referring to below. I think we
discussed this in previous iterations already.

> While I would love to have something as simple as:
> 	void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
> like we do today.  I understand that this makes all sorts of
> "assumptions" that Rust really doesn't like (i.e. lifetime of *value and
> the like), BUT we MUST have something like this for Rust users, as
> that's going to ensure that people actually use this api.

I think it can be as simple as

	void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value);

in Rust as well. Declaring this in a structure looks like this.

	struct Data {
	   counter: File<u8>,
	}

Given that we have some Dir instance, this can be as simple as:

	dir.create_file_u8(...);

Which uses default callbacks for read(), write(), etc.

> Look at an in-kernel function today, like ath9k_init_debug() that
> creates a metric-ton of debugfs files and binds them to different
> variables that are owned by a structure and more complex data structures
> and memory dumps and other random file interactions.  We need, in Rust,
> a way to do everything that that function can do today, in a SIMPLE
> manner that reads just as easily as ath9k_init_debug() does.

That's possible with the current design and code, it misses the helpers, such as
create_file_u8() above, to reduce the boilerplate though. With that, it should
look pretty similar.

> So no "we will add write support later" stuff, sorry, real drivers
> require write support in debugfs.

Adding the write callback seems rather simple, so it should also be fine to add
it right away.

From a design point of view the things above basically come down to different
variants of create_file().

So, it should mostly be sufficient to add subsequent patches to this series
implementing those.

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Benno Lossin]

On Thu Jul 10, 2025 at 11:36 AM CEST, Danilo Krummrich wrote:
> On Thu Jul 10, 2025 at 7:27 AM CEST, Greg Kroah-Hartman wrote:
>> Ugh.
>>
>> Yes we need write.  And read, and custom file-ops, and the like as
>> that's what debugfs is doing today for C code!  We need this to be as
>> simple as, or almost as simple as, what we have today in C or no one is
>> going to use this stuff and go off and attempt to write their own mess.
>
> I agree, we really want the helpers you're referring to below. I think we
> discussed this in previous iterations already.
>
>> While I would love to have something as simple as:
>> 	void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
>> like we do today.  I understand that this makes all sorts of
>> "assumptions" that Rust really doesn't like (i.e. lifetime of *value and
>> the like), BUT we MUST have something like this for Rust users, as
>> that's going to ensure that people actually use this api.
>
> I think it can be as simple as
>
> 	void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
>
> in Rust as well. Declaring this in a structure looks like this.
>
> 	struct Data {
> 	   counter: File<u8>,
> 	}
>
> Given that we have some Dir instance, this can be as simple as:
>
> 	dir.create_file_u8(...);
>
> Which uses default callbacks for read(), write(), etc.
>
>> Look at an in-kernel function today, like ath9k_init_debug() that
>> creates a metric-ton of debugfs files and binds them to different
>> variables that are owned by a structure and more complex data structures
>> and memory dumps and other random file interactions.  We need, in Rust,
>> a way to do everything that that function can do today, in a SIMPLE
>> manner that reads just as easily as ath9k_init_debug() does.
>
> That's possible with the current design and code, it misses the helpers, such as
> create_file_u8() above, to reduce the boilerplate though. With that, it should
> look pretty similar.

Can't you just implement the traits directly on `u8` and then just call
`create_file`?

---
Cheers,
Benno

Re: [PATCH v9 0/5] rust: DebugFS Bindings

[Benno Lossin]

On Thu Jul 10, 2025 at 1:09 PM CEST, Benno Lossin wrote:
> On Thu Jul 10, 2025 at 11:36 AM CEST, Danilo Krummrich wrote:
>> On Thu Jul 10, 2025 at 7:27 AM CEST, Greg Kroah-Hartman wrote:
>>> Ugh.
>>>
>>> Yes we need write.  And read, and custom file-ops, and the like as
>>> that's what debugfs is doing today for C code!  We need this to be as
>>> simple as, or almost as simple as, what we have today in C or no one is
>>> going to use this stuff and go off and attempt to write their own mess.
>>
>> I agree, we really want the helpers you're referring to below. I think we
>> discussed this in previous iterations already.
>>
>>> While I would love to have something as simple as:
>>> 	void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
>>> like we do today.  I understand that this makes all sorts of
>>> "assumptions" that Rust really doesn't like (i.e. lifetime of *value and
>>> the like), BUT we MUST have something like this for Rust users, as
>>> that's going to ensure that people actually use this api.
>>
>> I think it can be as simple as
>>
>> 	void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value);
>>
>> in Rust as well. Declaring this in a structure looks like this.
>>
>> 	struct Data {
>> 	   counter: File<u8>,
>> 	}
>>
>> Given that we have some Dir instance, this can be as simple as:
>>
>> 	dir.create_file_u8(...);
>>
>> Which uses default callbacks for read(), write(), etc.
>>
>>> Look at an in-kernel function today, like ath9k_init_debug() that
>>> creates a metric-ton of debugfs files and binds them to different
>>> variables that are owned by a structure and more complex data structures
>>> and memory dumps and other random file interactions.  We need, in Rust,
>>> a way to do everything that that function can do today, in a SIMPLE
>>> manner that reads just as easily as ath9k_init_debug() does.
>>
>> That's possible with the current design and code, it misses the helpers, such as
>> create_file_u8() above, to reduce the boilerplate though. With that, it should
>> look pretty similar.
>
> Can't you just implement the traits directly on `u8` and then just call
> `create_file`?

Ah I guess for write support you need `Atomic<u8>` and that doesn't
implement `Display`...

Maybe `Display` is the wrong trait for this...

---
Cheers,
Benno

Re: [PATCH v9 3/5] rust: debugfs: Support `PinInit` backing for `File`s.

[Dirk Behme]

Hi Matthew,

On 09/07/2025 21:09, Matthew Maurer wrote:
> This allows `File`s to own their data, allowing DebugFS files to be
> managed in sync with the data that backs them. Because DebugFS files are
> intended to actually own data and provide access, `File`s still maintain
> the same lifecycle for provided data when `CONFIG_DEBUG_FS` is disabled.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
>  rust/kernel/debugfs.rs | 149 ++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 117 insertions(+), 32 deletions(-)
> 
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index e5b6497d1deb67671d22ffd90cd5baa855bb9257..a1a84dd309216f455ae8fe3d3c0fd00f957f82a9 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -5,12 +5,13 @@
>  //!
>  //! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
>  
> -#[cfg(CONFIG_DEBUG_FS)]
> -use crate::prelude::GFP_KERNEL;
> +use crate::prelude::*;
>  use crate::str::CStr;
>  #[cfg(CONFIG_DEBUG_FS)]
>  use crate::sync::Arc;
>  use core::fmt::Display;
> +use core::marker::PhantomPinned;
> +use core::ops::Deref;
>  
>  #[cfg(CONFIG_DEBUG_FS)]
>  mod display_file;
> @@ -63,40 +64,78 @@ fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
>      }
>  
>      #[cfg(CONFIG_DEBUG_FS)]
> -    fn create_file<T: Display + Sized + Sync>(&self, name: &CStr, data: &'static T) -> File {
> -        let Some(parent) = &self.0 else {
> -            return File {
> +    /// Creates a DebugFS file which will own the data produced by the initializer provided in
> +    /// `data`.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The provided vtable must be appropriate for implementing a seq_file if provided
> +    /// with a private data pointer which provides shared access to a `T`.
> +    unsafe fn create_file<'a, T: Sync, E, TI: PinInit<T, E>>(
> +        &self,
> +        name: &'a CStr,
> +        data: TI,
> +        vtable: &'static bindings::file_operations,
> +    ) -> impl PinInit<File<T>, E> + use<'_, 'a, T, E, TI> {

Rebasing my test code from an older version of this series to this v9
(this is the most recent one?) here in rust/kernel/debugfs.rs and in
samples/rust/rust_debugfs.rs I get errors for each place where
'use<...>' is used:

error[E0658]: precise captures on `impl Trait` are experimental
  --> rust/kernel/debugfs.rs:81:37
   |
81 |     ) -> impl PinInit<File<T>, E> + use<'_, 'a, T, E, TI> {
   |                                     ^^^
   |
   = note: see issue #123432
<https://github.com/rust-lang/rust/issues/123432> for more information
   = help: add `#![feature(precise_capturing)]` to the crate attributes
to enable
   = note: this compiler was built on 2024-09-04; consider upgrading it
if it is out of date

rustc is

rustc 1.81.0 (eeb90cda1 2024-09-04)

I tried to work around this by adding

--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,9 @@
 //
 // To be determined.
 #![feature(used_with_arg)]
+
+#![feature(precise_capturing)]
+

This seems to help for rust/kernel/debugfs.rs but not for
samples/rust/rust_debugfs.rs.

Any hint?

Best regards

Dirk

Re: [PATCH v9 3/5] rust: debugfs: Support `PinInit` backing for `File`s.

[Matthew Maurer]

On Mon, Aug 18, 2025 at 10:51 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Hi Matthew,
>
> On 09/07/2025 21:09, Matthew Maurer wrote:
> > This allows `File`s to own their data, allowing DebugFS files to be
> > managed in sync with the data that backs them. Because DebugFS files are
> > intended to actually own data and provide access, `File`s still maintain
> > the same lifecycle for provided data when `CONFIG_DEBUG_FS` is disabled.
> >
> > Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > ---
> >  rust/kernel/debugfs.rs | 149 ++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 117 insertions(+), 32 deletions(-)
> >
> > diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> > index e5b6497d1deb67671d22ffd90cd5baa855bb9257..a1a84dd309216f455ae8fe3d3c0fd00f957f82a9 100644
> > --- a/rust/kernel/debugfs.rs
> > +++ b/rust/kernel/debugfs.rs
> > @@ -5,12 +5,13 @@
> >  //!
> >  //! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
> >
> > -#[cfg(CONFIG_DEBUG_FS)]
> > -use crate::prelude::GFP_KERNEL;
> > +use crate::prelude::*;
> >  use crate::str::CStr;
> >  #[cfg(CONFIG_DEBUG_FS)]
> >  use crate::sync::Arc;
> >  use core::fmt::Display;
> > +use core::marker::PhantomPinned;
> > +use core::ops::Deref;
> >
> >  #[cfg(CONFIG_DEBUG_FS)]
> >  mod display_file;
> > @@ -63,40 +64,78 @@ fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
> >      }
> >
> >      #[cfg(CONFIG_DEBUG_FS)]
> > -    fn create_file<T: Display + Sized + Sync>(&self, name: &CStr, data: &'static T) -> File {
> > -        let Some(parent) = &self.0 else {
> > -            return File {
> > +    /// Creates a DebugFS file which will own the data produced by the initializer provided in
> > +    /// `data`.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// The provided vtable must be appropriate for implementing a seq_file if provided
> > +    /// with a private data pointer which provides shared access to a `T`.
> > +    unsafe fn create_file<'a, T: Sync, E, TI: PinInit<T, E>>(
> > +        &self,
> > +        name: &'a CStr,
> > +        data: TI,
> > +        vtable: &'static bindings::file_operations,
> > +    ) -> impl PinInit<File<T>, E> + use<'_, 'a, T, E, TI> {
>
> Rebasing my test code from an older version of this series to this v9
> (this is the most recent one?) here in rust/kernel/debugfs.rs and in
> samples/rust/rust_debugfs.rs I get errors for each place where
> 'use<...>' is used:
>
> error[E0658]: precise captures on `impl Trait` are experimental
>   --> rust/kernel/debugfs.rs:81:37
>    |
> 81 |     ) -> impl PinInit<File<T>, E> + use<'_, 'a, T, E, TI> {
>    |                                     ^^^
>    |
>    = note: see issue #123432
> <https://github.com/rust-lang/rust/issues/123432> for more information
>    = help: add `#![feature(precise_capturing)]` to the crate attributes
> to enable
>    = note: this compiler was built on 2024-09-04; consider upgrading it
> if it is out of date
>
> rustc is
>
> rustc 1.81.0 (eeb90cda1 2024-09-04)
>
The easy way to resolve this is to use `rustc` 1.82.0 or newer (you're
only one revision behind!), but that's past the MSRV for the kernel at
the moment of 1.79.0. I intend to send up a new patch today, so I
suppose I'll need to put that behind a flag similar to
`RUSTC_HAS_DERIVE_COERCE_POINTEE`.
>
> I tried to work around this by adding
>
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -36,6 +36,9 @@
>  //
>  // To be determined.
>  #![feature(used_with_arg)]
> +
> +#![feature(precise_capturing)]
> +
>
> This seems to help for rust/kernel/debugfs.rs but not for
> samples/rust/rust_debugfs.rs.
Did you add `#![feature(precise_capturing)]` to
`samples/rust/rust_debugfs.rs` as well? It is its own crate, so it has
its own set of features.
>
>
> Any hint?
>
> Best regards
>
> Dirk
>

Re: [PATCH v9 3/5] rust: debugfs: Support `PinInit` backing for `File`s.

[Miguel Ojeda]

On Tue, Aug 19, 2025 at 4:33 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> The easy way to resolve this is to use `rustc` 1.82.0 or newer (you're
> only one revision behind!), but that's past the MSRV for the kernel at
> the moment of 1.79.0. I intend to send up a new patch today, so I
> suppose I'll need to put that behind a flag similar to
> `RUSTC_HAS_DERIVE_COERCE_POINTEE`.

The syntax changed after our minimum version IIRC -- does this case
require the precise syntax or can we use one of the workarounds?

> Did you add `#![feature(precise_capturing)]` to
> `samples/rust/rust_debugfs.rs` as well? It is its own crate, so it has
> its own set of features.

This would need to go into the `allowed_features` list, assuming it
works, but please check the above to see if the syntax is allowed
across the versions.

Thanks!

Cheers,
Miguel

Re: [PATCH v9 3/5] rust: debugfs: Support `PinInit` backing for `File`s.

[Matthew Maurer]

This is resolved in V10 [1], just sent.

[1] https://lore.kernel.org/all/CAGSQo03RGzmP2diL-vvLDZHduu=d4oFy8X46Fc8vg0SzE-XfDw@mail.gmail.com/