rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/5] rust: DebugFS Bindings
@ 2025-06-18  2:28 Matthew Maurer
  2025-06-18  2:28 ` [PATCH v6 1/5] rust: debugfs: Bind DebugFS directory creation Matthew Maurer
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Matthew Maurer @ 2025-06-18  2:28 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

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

Signed-off-by: Matthew Maurer <mmaurer@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 arbitrary owned backing for File
      rust: debugfs: Support format hooks
      rust: samples: Add debugfs sample

 MAINTAINERS                         |   3 +
 rust/bindings/bindings_helper.h     |   1 +
 rust/kernel/debugfs.rs              | 218 ++++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/display_file.rs | 115 +++++++++++++++++++
 rust/kernel/debugfs/entry.rs        |  66 +++++++++++
 rust/kernel/lib.rs                  |   1 +
 samples/rust/Kconfig                |  11 ++
 samples/rust/Makefile               |   1 +
 samples/rust/rust_debugfs.rs        |  76 +++++++++++++
 9 files changed, 492 insertions(+)
---
base-commit: 6a7635cd013da3b41bf1e66bbdb9ae4a570d7449
change-id: 20250428-debugfs-rust-3cd5c97eb7d1

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v6 1/5] rust: debugfs: Bind DebugFS directory creation
  2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
@ 2025-06-18  2:28 ` Matthew Maurer
  2025-06-18 10:04   ` Danilo Krummrich
  2025-06-18  2:28 ` [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display Matthew Maurer
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Matthew Maurer @ 2025-06-18  2:28 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin
  Cc: linux-kernel, rust-for-linux, 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          | 88 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/entry.rs    | 58 +++++++++++++++++++++++++++
 rust/kernel/lib.rs              |  1 +
 5 files changed, 150 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ec49e19734256f0c0c0960b722293edeb851562a..fda422023e30fe8821e4cd79b503693ecc6eb48c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7373,6 +7373,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 8cbb660e2ec218021d16e6e0144acf6f4d7cca13..655d88b8e7fe717190ccfb7b8173e84213bf8331 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -45,6 +45,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..e390278b8b7384f1d9e2b3d7a4f5f365f6447d0f
--- /dev/null
+++ b/rust/kernel/debugfs.rs
@@ -0,0 +1,88 @@
+// 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;
+
+/// 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::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::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 6b4774b2b1c37f4da1866e993be6230bc6715841..43f40b1baa9717ea71e4586042e9e6979491ad37 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -66,6 +66,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.rc2.696.g1fc2a0284f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display
  2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
  2025-06-18  2:28 ` [PATCH v6 1/5] rust: debugfs: Bind DebugFS directory creation Matthew Maurer
@ 2025-06-18  2:28 ` Matthew Maurer
  2025-06-18 15:39   ` Danilo Krummrich
  2025-06-18  2:28 ` [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File Matthew Maurer
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Matthew Maurer @ 2025-06-18  2:28 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin
  Cc: linux-kernel, rust-for-linux, 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.

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

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index e390278b8b7384f1d9e2b3d7a4f5f365f6447d0f..6a89557d8cf49327d2984d15741ffb6640defd70 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;
 
@@ -57,6 +60,43 @@ fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
         Self()
     }
 
+    #[cfg(CONFIG_DEBUG_FS)]
+    fn create_file<T: Display + Sized>(&self, name: &CStr, data: &'static T) -> File {
+        let Some(parent) = &self.0 else {
+            return File {
+                _entry: 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::Entry::new(ptr, Some(parent.clone())) };
+
+        File { _entry: entry }
+    }
+
+    #[cfg(not(CONFIG_DEBUG_FS))]
+    fn create_file<T: Display + Sized>(&self, _name: &CStr, _data: &'static T) -> File {
+        File {}
+    }
+
     /// Create a DebugFS subdirectory.
     ///
     /// Subdirectory handles cannot outlive the directory handle they were created from.
@@ -73,6 +113,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>(&self, name: &CStr, data: &'static T) -> File {
+        self.create_file(name, data)
+    }
+
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples
@@ -86,3 +142,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::Entry,
+}
diff --git a/rust/kernel/debugfs/display_file.rs b/rust/kernel/debugfs/display_file.rs
new file mode 100644
index 0000000000000000000000000000000000000000..65e37e34d7b587482492dc296637a3bc517b9fe3
--- /dev/null
+++ b/rust/kernel/debugfs/display_file.rs
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+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 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, this pointer is live, points to a value of type `T`, and
+    // is not being mutated.
+    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: 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 {}
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.rc2.696.g1fc2a0284f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File
  2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
  2025-06-18  2:28 ` [PATCH v6 1/5] rust: debugfs: Bind DebugFS directory creation Matthew Maurer
  2025-06-18  2:28 ` [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display Matthew Maurer
@ 2025-06-18  2:28 ` Matthew Maurer
  2025-06-18  8:19   ` Alice Ryhl
  2025-06-18  2:28 ` [PATCH v6 4/5] rust: debugfs: Support format hooks Matthew Maurer
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Matthew Maurer @ 2025-06-18  2:28 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

This allows `File`s to be backed by `Deref<Target=T>` rather than just
`&'static T`. This means that dynamically allocated objects can be
attached to `File`s without needing to take extra steps to create a
pinned reference that's guaranteed to live long enough.

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

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 6a89557d8cf49327d2984d15741ffb6640defd70..cd83f21cf2818f406575941ebbc6c426575643e4 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::alloc::KBox;
 use crate::prelude::GFP_KERNEL;
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
 use core::fmt::Display;
+use core::ops::Deref;
 
 #[cfg(CONFIG_DEBUG_FS)]
 mod display_file;
@@ -61,40 +62,59 @@ fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
     }
 
     #[cfg(CONFIG_DEBUG_FS)]
-    fn create_file<T: Display + Sized>(&self, name: &CStr, data: &'static T) -> File {
+    fn create_file<D: Deref<Target = T> + 'static + Send + Sync, T: Display>(
+        &self,
+        name: &CStr,
+        data: D,
+    ) -> File {
+        let mut file = File {
+            _entry: entry::Entry::empty(),
+            _data: None,
+        };
+        let Some(data) = KBox::new(data, GFP_KERNEL).ok() else {
+            return file;
+        };
+
         let Some(parent) = &self.0 else {
-            return File {
-                _entry: entry::Entry::empty(),
-            };
+            return file;
         };
+
         // 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.
+        //   we have an owning `D` in the `File`, and we tear down the file during `Drop`.
         let ptr = unsafe {
             bindings::debugfs_create_file_full(
                 name.as_char_ptr(),
                 0o444,
                 parent.as_ptr(),
-                data as *const _ as *mut _,
+                data.deref() as *const _ as *mut _,
                 core::ptr::null(),
                 &<T as display_file::DisplayFile>::VTABLE,
             )
         };
 
+        file._data = Some(data);
+
         // 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::Entry::new(ptr, Some(parent.clone())) };
+        file._entry = unsafe { entry::Entry::new(ptr, Some(parent.clone())) };
 
-        File { _entry: entry }
+        file
     }
 
     #[cfg(not(CONFIG_DEBUG_FS))]
-    fn create_file<T: Display + Sized>(&self, _name: &CStr, _data: &'static T) -> File {
-        File {}
+    fn create_file<D: Deref<Target = T> + 'static + Send + Sync, T: Display>(
+        &self,
+        _name: &CStr,
+        data: D,
+    ) -> File {
+        File {
+            _data: KBox::new(data, GFP_KERNEL).ok().map(|x| x as _),
+        }
     }
 
     /// Create a DebugFS subdirectory.
@@ -125,7 +145,11 @@ 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>(&self, name: &CStr, data: &'static T) -> File {
+    pub fn display_file<D: Deref<Target = T> + 'static + Send + Sync, T: Display>(
+        &self,
+        name: &CStr,
+        data: D,
+    ) -> File {
         self.create_file(name, data)
     }
 
@@ -147,4 +171,7 @@ pub fn new(name: &CStr) -> Self {
 pub struct File {
     #[cfg(CONFIG_DEBUG_FS)]
     _entry: entry::Entry,
+    // The data needs to be kept in a `Box` to prevent it from moving when the file does, as
+    // this might invalidate the pointer that's been passed to debugfs.
+    _data: Option<KBox<dyn Send + Sync>>,
 }

-- 
2.50.0.rc2.696.g1fc2a0284f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v6 4/5] rust: debugfs: Support format hooks
  2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
                   ` (2 preceding siblings ...)
  2025-06-18  2:28 ` [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File Matthew Maurer
@ 2025-06-18  2:28 ` Matthew Maurer
  2025-06-18  2:28 ` [PATCH v6 5/5] rust: samples: Add debugfs sample Matthew Maurer
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Matthew Maurer @ 2025-06-18  2:28 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin
  Cc: linux-kernel, rust-for-linux, 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              | 41 ++++++++++++++++++++++++++
 rust/kernel/debugfs/display_file.rs | 57 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index cd83f21cf2818f406575941ebbc6c426575643e4..78544653b46d959c1520b1210170d22a0e973989 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -10,6 +10,7 @@
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
+use core::fmt;
 use core::fmt::Display;
 use core::ops::Deref;
 
@@ -153,6 +154,46 @@ pub fn display_file<D: Deref<Target = T> + 'static + Send + Sync, T: Display>(
         self.create_file(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<
+        D: Deref<Target = T> + 'static + Send + Sync,
+        T: Sync,
+        F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+    >(
+        &self,
+        name: &CStr,
+        data: D,
+        f: &'static F,
+    ) -> File {
+        #[cfg(CONFIG_DEBUG_FS)]
+        let data_adapted = display_file::FormatAdapter::new(data, f);
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        let data_adapted = {
+            // Mark used
+            let (_, _) = (data, f);
+            &0
+        };
+        self.display_file(name, data_adapted)
+    }
+
     /// 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 65e37e34d7b587482492dc296637a3bc517b9fe3..75f2e1c560840324117bc1bc73aa89e3823aaf0e 100644
--- a/rust/kernel/debugfs/display_file.rs
+++ b/rust/kernel/debugfs/display_file.rs
@@ -3,7 +3,9 @@
 
 use crate::seq_file::SeqFile;
 use crate::seq_print;
-use core::fmt::Display;
+use core::fmt::{Display, Formatter, Result};
+use core::marker::PhantomData;
+use core::ops::Deref;
 
 /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
 ///
@@ -58,3 +60,56 @@ 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<D, F> {
+    inner: D,
+    _formatter: PhantomData<F>,
+}
+
+impl<D, F> FormatAdapter<D, F> {
+    pub(crate) fn new(inner: D, _f: &'static F) -> Self {
+        // INVARIANT: We were passed a reference to F, so it is inhabited.
+        FormatAdapter {
+            inner,
+            _formatter: PhantomData,
+        }
+    }
+}
+
+impl<D: Deref<Target = T>, T, F> Display for FormatAdapter<D, F>
+where
+    F: Fn(&T, &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)
+    }
+}
+
+impl<D, F> Deref for FormatAdapter<D, F> {
+    type Target = Self;
+    fn deref(&self) -> &Self {
+        self
+    }
+}
+
+/// 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.rc2.696.g1fc2a0284f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v6 5/5] rust: samples: Add debugfs sample
  2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
                   ` (3 preceding siblings ...)
  2025-06-18  2:28 ` [PATCH v6 4/5] rust: debugfs: Support format hooks Matthew Maurer
@ 2025-06-18  2:28 ` Matthew Maurer
  2025-06-18 15:52   ` Danilo Krummrich
  2025-06-18  8:21 ` [PATCH v6 0/5] rust: DebugFS Bindings Alice Ryhl
  2025-06-24 11:37 ` Dirk Behme
  6 siblings, 1 reply; 15+ messages in thread
From: Matthew Maurer @ 2025-06-18  2:28 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin
  Cc: linux-kernel, rust-for-linux, 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 | 76 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fda422023e30fe8821e4cd79b503693ecc6eb48c..7faa088cc08514acfec371b043d324bb52d2e7ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7382,6 +7382,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..8eb2088b66b0646c7a1fba9cdccc9a6bb328defb 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 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 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..6af9177c711a07764f0323e03a72d115287f1205
--- /dev/null
+++ b/samples/rust/rust_debugfs.rs
@@ -0,0 +1,76 @@
+// 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, File};
+use kernel::prelude::*;
+use kernel::sync::{new_mutex, Arc};
+
+module! {
+    type: RustDebugFs,
+    name: "rust_debugfs",
+    authors: ["Matthew Maurer"],
+    description: "Rust DebugFS usage sample",
+    license: "GPL",
+}
+
+struct RustDebugFs {
+    // 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,
+    _subdir: Dir,
+    _file: File,
+    _file_2: File,
+}
+
+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.
+        // We `forget` the result to avoid deleting the file at the end of the scope.
+        let file = 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.
+
+        // In addition to globals, we can also attach any kind of owned data. Most commonly, this
+        // will look like an `Arc<MyObject>` as those can be shared with the rest of the module.
+        let my_arc = Arc::pin_init(new_mutex!(10), GFP_KERNEL)?;
+        // An `Arc<Mutex<usize>>` doesn't implement display, so let's give explicit instructions on
+        // how to print it
+        let file_2 = sub.fmt_file(c_str!("arc_backed"), my_arc.clone(), &|val, f| {
+            writeln!(f, "locked value: {:#010x}", *val.lock())
+        });
+
+        // Since it's an `Arc` and we cloned it, we continue to have access to `my_arc`. If this
+        // were real, we'd probably stash it in our module struct and do something with it when
+        // handling real calls.
+        *my_arc.lock() = 99;
+
+        // Save the handles we want to preserve to our module object. They will be automatically
+        // cleaned up when our module is unloaded.
+        Ok(Self {
+            _debugfs: debugfs,
+            _subdir: sub,
+            _file: file,
+            _file_2: file_2,
+        })
+    }
+}

-- 
2.50.0.rc2.696.g1fc2a0284f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File
  2025-06-18  2:28 ` [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File Matthew Maurer
@ 2025-06-18  8:19   ` Alice Ryhl
  2025-06-18 15:00     ` Matthew Maurer
  0 siblings, 1 reply; 15+ messages in thread
From: Alice Ryhl @ 2025-06-18  8:19 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin, linux-kernel,
	rust-for-linux

On Wed, Jun 18, 2025 at 02:28:15AM +0000, Matthew Maurer wrote:
> This allows `File`s to be backed by `Deref<Target=T>` rather than just
> `&'static T`. This means that dynamically allocated objects can be
> attached to `File`s without needing to take extra steps to create a
> pinned reference that's guaranteed to live long enough.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
>  rust/kernel/debugfs.rs | 51 ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index 6a89557d8cf49327d2984d15741ffb6640defd70..cd83f21cf2818f406575941ebbc6c426575643e4 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::alloc::KBox;
>  use crate::prelude::GFP_KERNEL;
>  use crate::str::CStr;
>  #[cfg(CONFIG_DEBUG_FS)]
>  use crate::sync::Arc;
>  use core::fmt::Display;
> +use core::ops::Deref;
>  
>  #[cfg(CONFIG_DEBUG_FS)]
>  mod display_file;
> @@ -61,40 +62,59 @@ fn create(_name: &CStr, _parent: Option<&Dir>) -> Self {
>      }
>  
>      #[cfg(CONFIG_DEBUG_FS)]
> -    fn create_file<T: Display + Sized>(&self, name: &CStr, data: &'static T) -> File {
> +    fn create_file<D: Deref<Target = T> + 'static + Send + Sync, T: Display>(
> +        &self,
> +        name: &CStr,
> +        data: D,
> +    ) -> File {
> +        let mut file = File {
> +            _entry: entry::Entry::empty(),
> +            _data: None,
> +        };
> +        let Some(data) = KBox::new(data, GFP_KERNEL).ok() else {
> +            return file;
> +        };

We may want to consider using the ForeignOwnable trait here instead. The
trait is implemented by anything that can be converted to/from a void
pointer, so you can:

* When creating the file, convert it to a void pointer that you store in
  File and pass to debugfs_create_file_full.
* When displaying the file, create a borrowed version of the void
  pointer and display that.
* When freeing the File, convert the void pointer back into an owned
  value and drop it.

For cases where a box really is necessary, the user can create a box and
pass it themselves. But if the user already has a pointer type (e.g. and
Arc<T> or &'static T) then they can pass that pointer directly and the
pointer is stored as a void pointer without the Box indirection.

Alice

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 0/5] rust: DebugFS Bindings
  2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
                   ` (4 preceding siblings ...)
  2025-06-18  2:28 ` [PATCH v6 5/5] rust: samples: Add debugfs sample Matthew Maurer
@ 2025-06-18  8:21 ` Alice Ryhl
  2025-06-24 11:37 ` Dirk Behme
  6 siblings, 0 replies; 15+ messages in thread
From: Alice Ryhl @ 2025-06-18  8:21 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin, linux-kernel,
	rust-for-linux

On Wed, Jun 18, 2025 at 02:28:12AM +0000, Matthew Maurer wrote:
> This series provides safe DebugFS bindings for Rust, with a sample
> driver using them.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>

Overall I think this series looks great. I left one comment, but overall
LGTM.

Alice

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 1/5] rust: debugfs: Bind DebugFS directory creation
  2025-06-18  2:28 ` [PATCH v6 1/5] rust: debugfs: Bind DebugFS directory creation Matthew Maurer
@ 2025-06-18 10:04   ` Danilo Krummrich
  0 siblings, 0 replies; 15+ messages in thread
From: Danilo Krummrich @ 2025-06-18 10:04 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
	Benno Lossin, linux-kernel, rust-for-linux

On Wed, Jun 18, 2025 at 02:28:13AM +0000, Matthew Maurer wrote:
> +#[cfg(CONFIG_DEBUG_FS)]
> +mod 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::Entry>>);

NIT: Can we import Entry, such that we don't have to refer to it as entry::Entry
all the time?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File
  2025-06-18  8:19   ` Alice Ryhl
@ 2025-06-18 15:00     ` Matthew Maurer
  2025-06-18 15:32       ` Danilo Krummrich
  0 siblings, 1 reply; 15+ messages in thread
From: Matthew Maurer @ 2025-06-18 15:00 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin, linux-kernel,
	rust-for-linux

> We may want to consider using the ForeignOwnable trait here instead. The

I was considering trying to switch over to `StableDeref`-like trait
[1] in a follow-up patchset. The core property I need is that moving
the `D` cannot result in the pointer it would `deref` to changing.

The problem with `ForeignOwnable` is that it forbids the user from
passing in a `Box<dyn Foo>`, because that doesn't fit in a `void*` A
`StableDeref` version would not have this issue. I agree that
`ForeignOwnable` would be a strict upgrade to what I have now, since a
user can still pass in a `Box<Box<dyn Foo>>` and have it work with
`ForeignOwnable`, and if we ever added `StableDeref`, then
`ForeignOwnable` would have a blanket impl for it.

I'll send a new version using `ForeignOwnable`, and we can consider
the `StableDeref` version in the future.

[1]: https://docs.rs/gimli/latest/gimli/trait.StableDeref.html


> trait is implemented by anything that can be converted to/from a void
> pointer, so you can:
>
> * When creating the file, convert it to a void pointer that you store in
>   File and pass to debugfs_create_file_full.
> * When displaying the file, create a borrowed version of the void
>   pointer and display that.
> * When freeing the File, convert the void pointer back into an owned
>   value and drop it.
>
> For cases where a box really is necessary, the user can create a box and
> pass it themselves. But if the user already has a pointer type (e.g. and
> Arc<T> or &'static T) then they can pass that pointer directly and the
> pointer is stored as a void pointer without the Box indirection.
>
> Alice

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File
  2025-06-18 15:00     ` Matthew Maurer
@ 2025-06-18 15:32       ` Danilo Krummrich
  0 siblings, 0 replies; 15+ messages in thread
From: Danilo Krummrich @ 2025-06-18 15:32 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
	Benno Lossin, linux-kernel, rust-for-linux

On Wed, Jun 18, 2025 at 08:00:51AM -0700, Matthew Maurer wrote:
> > We may want to consider using the ForeignOwnable trait here instead. The
> 
> I was considering trying to switch over to `StableDeref`-like trait
> [1] in a follow-up patchset. The core property I need is that moving
> the `D` cannot result in the pointer it would `deref` to changing.
> 
> The problem with `ForeignOwnable` is that it forbids the user from
> passing in a `Box<dyn Foo>`, because that doesn't fit in a `void*` A
> `StableDeref` version would not have this issue. I agree that
> `ForeignOwnable` would be a strict upgrade to what I have now, since a
> user can still pass in a `Box<Box<dyn Foo>>` and have it work with
> `ForeignOwnable`, and if we ever added `StableDeref`, then
> `ForeignOwnable` would have a blanket impl for it.
> 
> I'll send a new version using `ForeignOwnable`, and we can consider
> the `StableDeref` version in the future.

Yes, please do that for now. It's rather common case that drivers want to expose
reference counted data, i.e. an Arc, through debugfs and having to go through
the indirection with an additional Box isn't quite nice.

> [1]: https://docs.rs/gimli/latest/gimli/trait.StableDeref.html
> 
> 
> > trait is implemented by anything that can be converted to/from a void
> > pointer, so you can:
> >
> > * When creating the file, convert it to a void pointer that you store in
> >   File and pass to debugfs_create_file_full.
> > * When displaying the file, create a borrowed version of the void
> >   pointer and display that.
> > * When freeing the File, convert the void pointer back into an owned
> >   value and drop it.
> >
> > For cases where a box really is necessary, the user can create a box and
> > pass it themselves. But if the user already has a pointer type (e.g. and
> > Arc<T> or &'static T) then they can pass that pointer directly and the
> > pointer is stored as a void pointer without the Box indirection.
> >
> > Alice

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display
  2025-06-18  2:28 ` [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display Matthew Maurer
@ 2025-06-18 15:39   ` Danilo Krummrich
  2025-06-18 15:56     ` Alice Ryhl
  0 siblings, 1 reply; 15+ messages in thread
From: Danilo Krummrich @ 2025-06-18 15:39 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
	Benno Lossin, linux-kernel, rust-for-linux

On Wed, Jun 18, 2025 at 02:28:14AM +0000, Matthew Maurer wrote:
> diff --git a/rust/kernel/debugfs/display_file.rs b/rust/kernel/debugfs/display_file.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..65e37e34d7b587482492dc296637a3bc517b9fe3
> --- /dev/null
> +++ b/rust/kernel/debugfs/display_file.rs
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2025 Google LLC.
> +
> +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 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 {

Please use kernel::ffi::c_int instead.

> +    // 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 {

kernel::ffi::c_int

> +    // SAFETY: By caller precondition, this pointer is live, points to a value of type `T`, and
> +    // is not being mutated.
> +    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: 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 _),

Why the cast? That shouldn't be needed.

> +        // SAFETY: `file_operations` supports zeroes in all fields.
> +        ..unsafe { core::mem::zeroed() }
> +    };
> +}

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 5/5] rust: samples: Add debugfs sample
  2025-06-18  2:28 ` [PATCH v6 5/5] rust: samples: Add debugfs sample Matthew Maurer
@ 2025-06-18 15:52   ` Danilo Krummrich
  0 siblings, 0 replies; 15+ messages in thread
From: Danilo Krummrich @ 2025-06-18 15:52 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
	Benno Lossin, linux-kernel, rust-for-linux

On Wed, Jun 18, 2025 at 02:28:17AM +0000, Matthew Maurer wrote:
> +config SAMPLE_RUST_DEBUGFS
> +	tristate "DebugFS Test Driver"

s/driver/module/

> +	depends on DEBUG_FS
> +	help
> +	  This option builds the Rust DebugFS Test driver sample.

s/driver/module/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display
  2025-06-18 15:39   ` Danilo Krummrich
@ 2025-06-18 15:56     ` Alice Ryhl
  0 siblings, 0 replies; 15+ messages in thread
From: Alice Ryhl @ 2025-06-18 15:56 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
	Benno Lossin, linux-kernel, rust-for-linux

On Wed, Jun 18, 2025 at 5:39 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed, Jun 18, 2025 at 02:28:14AM +0000, Matthew Maurer wrote:
> > diff --git a/rust/kernel/debugfs/display_file.rs b/rust/kernel/debugfs/display_file.rs
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..65e37e34d7b587482492dc296637a3bc517b9fe3
> > --- /dev/null
> > +++ b/rust/kernel/debugfs/display_file.rs
> > @@ -0,0 +1,60 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (C) 2025 Google LLC.
> > +
> > +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 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 {
>
> Please use kernel::ffi::c_int instead.

And please use it without the fully qualified path. The c_int typedef
is available in the prelude, so if you import kernel::prelude::* it
should be available.

Alice

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v6 0/5] rust: DebugFS Bindings
  2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
                   ` (5 preceding siblings ...)
  2025-06-18  8:21 ` [PATCH v6 0/5] rust: DebugFS Bindings Alice Ryhl
@ 2025-06-24 11:37 ` Dirk Behme
  6 siblings, 0 replies; 15+ messages in thread
From: Dirk Behme @ 2025-06-24 11:37 UTC (permalink / raw)
  To: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Sami Tolvanen, Timur Tabi, Benno Lossin
  Cc: linux-kernel, rust-for-linux

On 18/06/2025 04:28, Matthew Maurer wrote:
> This series provides safe DebugFS bindings for Rust, with a sample
> driver using them.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>


Having this series applied on top of v6.16-rc1 and adapting a
proprietary driver exposing some simple u64 values via debugfs on ARM64
looks good.

Tested-by: Dirk Behme <dirk.behme@de.bosch.com>

Thanks!

Dirk


> ---
> 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 arbitrary owned backing for File
>       rust: debugfs: Support format hooks
>       rust: samples: Add debugfs sample
> 
>  MAINTAINERS                         |   3 +
>  rust/bindings/bindings_helper.h     |   1 +
>  rust/kernel/debugfs.rs              | 218 ++++++++++++++++++++++++++++++++++++
>  rust/kernel/debugfs/display_file.rs | 115 +++++++++++++++++++
>  rust/kernel/debugfs/entry.rs        |  66 +++++++++++
>  rust/kernel/lib.rs                  |   1 +
>  samples/rust/Kconfig                |  11 ++
>  samples/rust/Makefile               |   1 +
>  samples/rust/rust_debugfs.rs        |  76 +++++++++++++
>  9 files changed, 492 insertions(+)
> ---
> base-commit: 6a7635cd013da3b41bf1e66bbdb9ae4a570d7449
> change-id: 20250428-debugfs-rust-3cd5c97eb7d1
> 
> Best regards,


-- 
======================================================================
Dirk Behme                      Robert Bosch Car Multimedia GmbH
                                CM/ESO2
Phone: +49 5121 49-3274         Dirk Behme
Fax:   +49 711 811 5053274      PO Box 77 77 77
mailto:dirk.behme@de.bosch.com  D-31132 Hildesheim - Germany

Bosch Group, Car Multimedia (CM)
             Engineering SW Operating Systems 2 (ESO2)

Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe
Sitz: Hildesheim
Registergericht: Amtsgericht Hildesheim HRB 201334
Aufsichtsratsvorsitzender: Dr. Dirk Hoheisel
Geschäftsführung: Dr. Steffen Berns;
                  Dr. Sven Ost, Jörg Pollak, Dr. Walter Schirm
======================================================================

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-06-24 11:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-18  2:28 [PATCH v6 0/5] rust: DebugFS Bindings Matthew Maurer
2025-06-18  2:28 ` [PATCH v6 1/5] rust: debugfs: Bind DebugFS directory creation Matthew Maurer
2025-06-18 10:04   ` Danilo Krummrich
2025-06-18  2:28 ` [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display Matthew Maurer
2025-06-18 15:39   ` Danilo Krummrich
2025-06-18 15:56     ` Alice Ryhl
2025-06-18  2:28 ` [PATCH v6 3/5] rust: debugfs: Support arbitrary owned backing for File Matthew Maurer
2025-06-18  8:19   ` Alice Ryhl
2025-06-18 15:00     ` Matthew Maurer
2025-06-18 15:32       ` Danilo Krummrich
2025-06-18  2:28 ` [PATCH v6 4/5] rust: debugfs: Support format hooks Matthew Maurer
2025-06-18  2:28 ` [PATCH v6 5/5] rust: samples: Add debugfs sample Matthew Maurer
2025-06-18 15:52   ` Danilo Krummrich
2025-06-18  8:21 ` [PATCH v6 0/5] rust: DebugFS Bindings Alice Ryhl
2025-06-24 11:37 ` Dirk Behme

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).