linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/7] rust: DebugFS Bindings
@ 2025-08-19 22:53 Matthew Maurer
  2025-08-19 22:53 ` [PATCH v10 1/7] rust: debugfs: Add initial support for directories Matthew Maurer
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

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

Shortly after this is sent, you will see a real driver WIP using this
implenting the qcom socinfo driver.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
Changes in v10:
- Introduced Scoped to show how either a File or Dir can be bound to
  data
- Remove use of `use<>` for MSRV compatibility
- Added Write support
- Added more complex sample driver using scoped interface
- Updated original sample driver to use writes to drive mutation
- Added `FileOps<T>` (only for DebugFS, not a kernel-wide abstraction)
  to decrease needed `unsafe` for keeping vtables paired to types.
- Centralized `dentry` lifecycle management to `entry.rs`.
- Link to v9: https://lore.kernel.org/r/20250709-debugfs-rust-v9-0-92b9eab5a951@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 (7):
      rust: debugfs: Add initial support for directories
      rust: debugfs: Add support for read-only files
      rust: debugfs: Add support for writable files
      rust: debugfs: Add support for callback-based files
      samples: rust: Add debugfs sample driver
      rust: debugfs: Add support for scoped directories
      samples: rust: Add scoped debugfs sample driver

 MAINTAINERS                              |   4 +
 rust/bindings/bindings_helper.h          |   1 +
 rust/kernel/debugfs.rs                   | 613 +++++++++++++++++++++++++++++++
 rust/kernel/debugfs/callback_adapters.rs | 122 ++++++
 rust/kernel/debugfs/entry.rs             | 164 +++++++++
 rust/kernel/debugfs/file_ops.rs          | 246 +++++++++++++
 rust/kernel/debugfs/traits.rs            |  97 +++++
 rust/kernel/lib.rs                       |   1 +
 samples/rust/Kconfig                     |  22 ++
 samples/rust/Makefile                    |   2 +
 samples/rust/rust_debugfs.rs             | 151 ++++++++
 samples/rust/rust_scoped_debugfs.rs      | 134 +++++++
 12 files changed, 1557 insertions(+)
---
base-commit: f3c5631f70e434e318c44001e2417d4770f06cd0
change-id: 20250428-debugfs-rust-3cd5c97eb7d1

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


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

* [PATCH v10 1/7] rust: debugfs: Add initial support for directories
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
@ 2025-08-19 22:53 ` Matthew Maurer
  2025-08-26 15:39   ` Danilo Krummrich
  2025-08-19 22:53 ` [PATCH v10 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Adds a `debugfs::Dir` type that can be used to create and remove
DebugFS directories. The `Dir` handle automatically cleans up the
directory on `Drop`.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index daf520a13bdf6a991c0160a96620f40308c29ee0..8f2dbf71ca3f8f97e4d7619375279ed11d1261b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7472,6 +7472,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 84d60635e8a9baef1f1a1b2752dc0fa044f8542f..e847820dc807fdda2d682d496a3c6361bb944c10 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..893aee54b920bac80f77c2726567da76929b7244
--- /dev/null
+++ b/rust/kernel/debugfs.rs
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+//! DebugFS Abstraction
+//!
+//! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
+
+// When DebugFS is disabled, many parameters are dead. Linting for this isn't helpful.
+#![cfg_attr(not(CONFIG_DEBUG_FS), allow(unused_variables))]
+
+#[cfg(CONFIG_DEBUG_FS)]
+use crate::prelude::*;
+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 removed when this handle has been dropped *and* all children have been
+/// removed.
+// We hold a reference to our parent if it exists in the `Entry` to prevent the dentry we point
+// to from being cleaned up when our parent is removed.
+//
+// The `None` option indicates that the `Arc` could not be allocated, so our children would not be
+// able to refer to us. In this case, we need to silently fail. All future child directories/files
+// will silently fail as well.
+#[derive(Clone)]
+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.
+    fn create(name: &CStr, parent: Option<&Dir>) -> Self {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            let parent_entry = match parent {
+                // If the parent couldn't be allocated, just early-return
+                Some(Dir(None)) => return Self(None),
+                Some(Dir(Some(entry))) => Some(entry.clone()),
+                None => None,
+            };
+            Self(
+                // If Arc creation fails, the `Entry` will be dropped, so the directory will be
+                // cleaned up.
+                Arc::new(Entry::dynamic_dir(name, parent_entry), GFP_KERNEL).ok(),
+            )
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        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)
+    }
+
+    /// Creates a subdirectory within this directory.
+    ///
+    /// # 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))
+    }
+}
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
new file mode 100644
index 0000000000000000000000000000000000000000..d2fba0e65e20e954e2a33e776b872bac4adb12e8
--- /dev/null
+++ b/rust/kernel/debugfs/entry.rs
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+use crate::str::CStr;
+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,
+    // If we were created with an owning parent, this is the keep-alive
+    _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 C functions we call on the `dentry` pointer are threadsafe.
+unsafe impl Sync for Entry {}
+
+impl Entry {
+    pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
+        let parent_ptr = match &parent {
+            Some(entry) => entry.as_ptr(),
+            None => core::ptr::null_mut(),
+        };
+        // SAFETY: The invariants of this function's arguments ensure the safety of this call.
+        // * `name` is a valid C string by the invariants of `&CStr`.
+        // * `parent_ptr` is either `NULL` (if `parent` is `None`), or a pointer to a valid
+        //   `dentry` by our invariant. `debugfs_create_dir` handles `NULL` pointers correctly.
+        let entry = unsafe { bindings::debugfs_create_dir(name.as_char_ptr(), parent_ptr) };
+
+        Entry {
+            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. If it is live, it will remain live at least as
+    /// long as this entry lives.
+    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 ed53169e795c0badf548025a57f946fa18bc73e3..828620c8441566a638f31d03633fc1bf4c1bda85 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -76,6 +76,7 @@
 pub mod cpufreq;
 pub mod cpumask;
 pub mod cred;
+pub mod debugfs;
 pub mod device;
 pub mod device_id;
 pub mod devres;

-- 
2.51.0.rc1.167.g924127e9c0-goog


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

* [PATCH v10 2/7] rust: debugfs: Add support for read-only files
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
  2025-08-19 22:53 ` [PATCH v10 1/7] rust: debugfs: Add initial support for directories Matthew Maurer
@ 2025-08-19 22:53 ` Matthew Maurer
  2025-08-26 18:45   ` Danilo Krummrich
  2025-08-19 22:53 ` [PATCH v10 3/7] rust: debugfs: Add support for writable files Matthew Maurer
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Extends the `debugfs` API to support creating read-only files. This
is done via the `Dir::read_only_file` method, which takes a data object
that implements the `Render` trait.

The file's content is generated by the `Render` implementation, and the
file is automatically removed when the returned `File` handle is
dropped.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/debugfs.rs          | 142 +++++++++++++++++++++++++++++++++++++++-
 rust/kernel/debugfs/entry.rs    |  42 ++++++++++++
 rust/kernel/debugfs/file_ops.rs | 125 +++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/traits.rs   |  28 ++++++++
 4 files changed, 336 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 893aee54b920bac80f77c2726567da76929b7244..875d433fc3608cc9ffcf022d7c00cb207016f146 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -8,12 +8,18 @@
 // When DebugFS is disabled, many parameters are dead. Linting for this isn't helpful.
 #![cfg_attr(not(CONFIG_DEBUG_FS), allow(unused_variables))]
 
-#[cfg(CONFIG_DEBUG_FS)]
 use crate::prelude::*;
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
+use core::marker::PhantomPinned;
+use core::ops::Deref;
+
+mod traits;
+pub use traits::Render;
 
+mod file_ops;
+use file_ops::{FileOps, ReadFile};
 #[cfg(CONFIG_DEBUG_FS)]
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]
@@ -53,6 +59,31 @@ fn create(name: &CStr, parent: Option<&Dir>) -> Self {
         Self()
     }
 
+    /// Creates a DebugFS file which will own the data produced by the initializer provided in
+    /// `data`.
+    fn create_file<'a, T: Sync + 'static, E: 'a, TI: PinInit<T, E> + 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: TI,
+        file_ops: &'static FileOps<T>,
+    ) -> impl PinInit<File<T>, E> + 'a {
+        let scope = Scope::<T>::new(data, move |data| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            if let Some(parent) = &self.0 {
+                // SAFETY: Because data derives from a scope, and our entry will be dropped before
+                // the data is dropped, it is guaranteed to outlive the entry we return.
+                unsafe { Entry::dynamic_file(name, parent.clone(), data, file_ops) }
+            } else {
+                Entry::empty()
+            }
+        });
+        try_pin_init! {
+            File {
+                scope <- scope
+            } ? E
+        }
+    }
+
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples
@@ -79,4 +110,113 @@ pub fn new(name: &CStr) -> Self {
     pub fn subdir(&self, name: &CStr) -> Self {
         Dir::create(name, Some(self))
     }
+
+    /// Creates a read-only file in this directory.
+    ///
+    /// The file's contents are produced by invoking [`Render::render`] on the value initialized by
+    /// `data`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// # use kernel::prelude::*;
+    /// # let dir = Dir::new(c_str!("my_debugfs_dir"));
+    /// let file = KBox::pin_init(dir.read_only_file(c_str!("foo"), 200), GFP_KERNEL)?;
+    /// // "my_debugfs_dir/foo" now contains the number 200.
+    /// // The file is removed when `file` is dropped.
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn read_only_file<'a, T: Render + Send + Sync + 'static, E: 'a, TI: PinInit<T, E> + 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: TI,
+    ) -> impl PinInit<File<T>, E> + 'a {
+        let file_ops = &<T as ReadFile<_>>::FILE_OPS;
+        self.create_file(name, data, file_ops)
+    }
+}
+
+#[pin_data]
+/// Handle to a DebugFS scope, which allows a variety of DebugFS files/directories to hang off a
+/// single structure.
+pub struct Scope<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,
+}
+
+#[pin_data]
+/// Handle to a DebugFS file, owning its backing data.
+///
+/// When dropped, the DebugFS file will be removed and the attached data will be dropped.
+pub struct File<T> {
+    #[pin]
+    scope: Scope<T>,
+}
+
+#[cfg(not(CONFIG_DEBUG_FS))]
+impl<T> Scope<T> {
+    fn new<E, TI: PinInit<T, E>, F: for<'a> FnOnce(&'a T)>(
+        data: TI,
+        init: F,
+    ) -> impl PinInit<Self, E> {
+        try_pin_init! {
+            Self {
+                data <- data,
+                _pin: PhantomPinned
+            } ? E
+        }
+        .pin_chain(|scope| {
+            init(&scope.data);
+            Ok(())
+        })
+    }
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+impl<T> Scope<T> {
+    fn entry_mut(self: Pin<&mut Self>) -> &mut Entry {
+        // SAFETY: _entry is not structurally pinned
+        unsafe { &mut Pin::into_inner_unchecked(self)._entry }
+    }
+    fn new<'b, E: 'b, TI: PinInit<T, E> + 'b, F: for<'a> FnOnce(&'a T) -> Entry + 'b>(
+        data: TI,
+        init: F,
+    ) -> impl PinInit<Self, E> + 'b
+    where
+        T: 'b,
+    {
+        try_pin_init! {
+            Self {
+                _entry: Entry::empty(),
+                data <- data,
+                _pin: PhantomPinned
+            } ? E
+        }
+        .pin_chain(|scope| {
+            *scope.entry_mut() = init(&scope.data);
+            Ok(())
+        })
+    }
+}
+
+impl<T> Deref for Scope<T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.data
+    }
+}
+
+impl<T> Deref for File<T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.scope
+    }
 }
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
index d2fba0e65e20e954e2a33e776b872bac4adb12e8..227fa50b7a79aeab49779e54b8c4241f455777c3 100644
--- a/rust/kernel/debugfs/entry.rs
+++ b/rust/kernel/debugfs/entry.rs
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2025 Google LLC.
 
+use crate::debugfs::file_ops::FileOps;
+use crate::ffi::c_void;
 use crate::str::CStr;
 use crate::sync::Arc;
 
@@ -40,6 +42,46 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
         }
     }
 
+    /// # Safety
+    ///
+    /// * `data` must outlive the returned `Entry`.
+    pub(crate) unsafe fn dynamic_file<T>(
+        name: &CStr,
+        parent: Arc<Self>,
+        data: &T,
+        file_ops: &'static FileOps<T>,
+    ) -> Self {
+        // SAFETY: The invariants of this function's arguments ensure the safety of this call.
+        // * `name` is a valid C string by the invariants of `&CStr`.
+        // * `parent.as_ptr()` is a pointer to a valid `dentry` by invariant.
+        // * The caller guarantees that `data` will outlive the returned `Entry`.
+        // * The guarantees on `FileOps` assert the vtable will be compatible with the data we have
+        //   provided.
+        let entry = unsafe {
+            bindings::debugfs_create_file_full(
+                name.as_char_ptr(),
+                file_ops.mode(),
+                parent.as_ptr(),
+                core::ptr::from_ref(data) as *mut c_void,
+                core::ptr::null(),
+                &**file_ops,
+            )
+        };
+
+        Entry {
+            entry,
+            _parent: Some(parent),
+        }
+    }
+
+    /// 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
diff --git a/rust/kernel/debugfs/file_ops.rs b/rust/kernel/debugfs/file_ops.rs
new file mode 100644
index 0000000000000000000000000000000000000000..134ac26e80f2e5b9cae53ed5a00462af7ce1aa38
--- /dev/null
+++ b/rust/kernel/debugfs/file_ops.rs
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+use super::Render;
+use crate::prelude::*;
+use crate::seq_file::SeqFile;
+use crate::seq_print;
+use core::fmt::{Display, Formatter, Result};
+use core::marker::PhantomData;
+
+#[cfg(CONFIG_DEBUG_FS)]
+use core::ops::Deref;
+
+/// # Invariant
+///
+/// `FileOps<T>` will always contain an `operations` which is safe to use for a file backed
+/// off an inode which has a pointer to a `T` in its private data that is safe to convert
+/// into a reference.
+pub(super) struct FileOps<T> {
+    #[cfg(CONFIG_DEBUG_FS)]
+    operations: bindings::file_operations,
+    #[cfg(CONFIG_DEBUG_FS)]
+    mode: u16,
+    _phantom: PhantomData<T>,
+}
+
+impl<T> FileOps<T> {
+    /// # Safety
+    /// The caller asserts that the provided `operations` is safe to use for a file whose
+    /// inode has a pointer to `T` in its private data that is safe to convert into a reference.
+    const unsafe fn new(operations: bindings::file_operations, mode: u16) -> Self {
+        Self {
+            #[cfg(CONFIG_DEBUG_FS)]
+            operations,
+            #[cfg(CONFIG_DEBUG_FS)]
+            mode,
+            _phantom: PhantomData,
+        }
+    }
+    #[cfg(CONFIG_DEBUG_FS)]
+    pub(crate) const fn mode(&self) -> u16 {
+        self.mode
+    }
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+impl<T> Deref for FileOps<T> {
+    type Target = bindings::file_operations;
+    fn deref(&self) -> &Self::Target {
+        &self.operations
+    }
+}
+
+struct RenderAdapter<T>(T);
+
+impl<'a, T: Render> Display for RenderAdapter<&'a T> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+        self.0.render(f)
+    }
+}
+
+/// 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.
+unsafe extern "C" fn render_open<T: Render + Sync>(
+    inode: *mut bindings::inode,
+    file: *mut bindings::file,
+) -> c_int {
+    // SAFETY: The caller ensures that `inode` is a valid pointer.
+    let data = unsafe { (*inode).i_private };
+    // 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(render_act::<T>), data) }
+}
+
+/// 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.
+unsafe extern "C" fn render_act<T: Render + 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.cast::<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, "{}", RenderAdapter(data));
+    0
+}
+
+// Work around lack of generic const items.
+pub(crate) trait ReadFile<T> {
+    const FILE_OPS: FileOps<T>;
+}
+
+impl<T: Render + Sync> ReadFile<T> for T {
+    const FILE_OPS: FileOps<T> = {
+        let operations = bindings::file_operations {
+            read: Some(bindings::seq_read),
+            llseek: Some(bindings::seq_lseek),
+            release: Some(bindings::single_release),
+            open: Some(render_open::<Self>),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+        // SAFETY: `operations` is all stock `seq_file` implementations except for `render_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 matches the
+        // `FileOps` requirements.
+        unsafe { FileOps::new(operations, 0o400) }
+    };
+}
diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
new file mode 100644
index 0000000000000000000000000000000000000000..2939e18e3dda39571cd7255505e5f605f0e3d154
--- /dev/null
+++ b/rust/kernel/debugfs/traits.rs
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+//! Traits for rendering or updating values exported to DebugFS.
+
+use crate::sync::Mutex;
+use core::fmt::{self, Debug, Formatter};
+
+/// A trait for types that can be rendered into a string.
+///
+/// This works very similarly to `Debug`, and is automatically implemented if `Debug` is
+/// implemented for a type. It is also implemented for any renderable type inside a `Mutex`.
+pub trait Render {
+    /// Formats the value using the given formatter.
+    fn render(&self, f: &mut Formatter<'_>) -> fmt::Result;
+}
+
+impl<T: Render> Render for Mutex<T> {
+    fn render(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        self.lock().render(f)
+    }
+}
+
+impl<T: Debug> Render for T {
+    fn render(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        writeln!(f, "{self:?}")
+    }
+}

-- 
2.51.0.rc1.167.g924127e9c0-goog


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

* [PATCH v10 3/7] rust: debugfs: Add support for writable files
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
  2025-08-19 22:53 ` [PATCH v10 1/7] rust: debugfs: Add initial support for directories Matthew Maurer
  2025-08-19 22:53 ` [PATCH v10 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
@ 2025-08-19 22:53 ` Matthew Maurer
  2025-08-26 19:38   ` Danilo Krummrich
  2025-08-19 22:53 ` [PATCH v10 4/7] rust: debugfs: Add support for callback-based files Matthew Maurer
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Extends the `debugfs` API to support creating writable files. This
is done via the `Dir::write_only_file` and `Dir::read_write_file`
methods, which take a data object that implements the `UpdateFromSlice`
trait.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/debugfs.rs          |  41 +++++++++++++-
 rust/kernel/debugfs/file_ops.rs | 115 +++++++++++++++++++++++++++++++++++++++-
 rust/kernel/debugfs/traits.rs   |  69 ++++++++++++++++++++++++
 3 files changed, 222 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 875d433fc3608cc9ffcf022d7c00cb207016f146..62bc2b1d4e5a4b21441a09e03bff74c32c6781d2 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -16,10 +16,10 @@
 use core::ops::Deref;
 
 mod traits;
-pub use traits::Render;
+pub use traits::{Render, UpdateFromSlice};
 
 mod file_ops;
-use file_ops::{FileOps, ReadFile};
+use file_ops::{FileOps, ReadFile, ReadWriteFile, WriteFile};
 #[cfg(CONFIG_DEBUG_FS)]
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]
@@ -136,6 +136,43 @@ pub fn read_only_file<'a, T: Render + Send + Sync + 'static, E: 'a, TI: PinInit<
         let file_ops = &<T as ReadFile<_>>::FILE_OPS;
         self.create_file(name, data, file_ops)
     }
+
+    /// Creates a read-write file in this directory.
+    ///
+    /// Reading the file uses the [`Render`] implementation.
+    /// Writing to the file uses the [`UpdateFromSlice`] implementation.
+    pub fn read_write_file<
+        'a,
+        T: Render + UpdateFromSlice + Send + Sync + 'static,
+        E: 'a,
+        TI: PinInit<T, E> + 'a,
+    >(
+        &'a self,
+        name: &'a CStr,
+        data: TI,
+    ) -> impl PinInit<File<T>, E> + 'a {
+        let file_ops = &<T as ReadWriteFile<_>>::FILE_OPS;
+        self.create_file(name, data, file_ops)
+    }
+
+    /// Creates a write-only file in this directory.
+    ///
+    /// The file owns its backing data. Writing to the file uses the [`UpdateFromSlice`]
+    /// implementation.
+    ///
+    /// The file is removed when the returned [`File`] is dropped.
+    pub fn write_only_file<
+        'a,
+        T: UpdateFromSlice + Send + Sync + 'static,
+        E: 'a,
+        TI: PinInit<T, E> + 'a,
+    >(
+        &'a self,
+        name: &'a CStr,
+        data: TI,
+    ) -> impl PinInit<File<T>, E> + 'a {
+        self.create_file(name, data, &T::FILE_OPS)
+    }
 }
 
 #[pin_data]
diff --git a/rust/kernel/debugfs/file_ops.rs b/rust/kernel/debugfs/file_ops.rs
index 134ac26e80f2e5b9cae53ed5a00462af7ce1aa38..30f6a0532c7f5f4a2974edc8f1100f5485aa8da9 100644
--- a/rust/kernel/debugfs/file_ops.rs
+++ b/rust/kernel/debugfs/file_ops.rs
@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2025 Google LLC.
 
-use super::Render;
+use super::{Render, UpdateFromSlice};
 use crate::prelude::*;
 use crate::seq_file::SeqFile;
 use crate::seq_print;
+use crate::uaccess::UserSlice;
 use core::fmt::{Display, Formatter, Result};
 use core::marker::PhantomData;
 
@@ -123,3 +124,115 @@ impl<T: Render + Sync> ReadFile<T> for T {
         unsafe { FileOps::new(operations, 0o400) }
     };
 }
+
+fn update<T: UpdateFromSlice + Sync>(data: &T, buf: *const c_char, count: usize) -> isize {
+    let mut reader = UserSlice::new(UserPtr::from_ptr(buf as *mut c_void), count).reader();
+
+    if let Err(e) = data.update_from_slice(&mut reader) {
+        return e.to_errno() as isize;
+    }
+
+    count as isize
+}
+
+/// # Safety
+///
+/// `file` must be a valid pointer to a `file` struct.
+/// The `private_data` of the file must contain a valid pointer to a `seq_file` whose
+/// `private` data in turn points to a `T` that implements `UpdateFromSlice`.
+/// `buf` must be a valid user-space buffer.
+pub(crate) unsafe extern "C" fn write<T: UpdateFromSlice + Sync>(
+    file: *mut bindings::file,
+    buf: *const c_char,
+    count: usize,
+    _ppos: *mut bindings::loff_t,
+) -> isize {
+    // SAFETY: The file was opened with `single_open`, which sets `private_data` to a `seq_file`.
+    let seq = unsafe { &mut *((*file).private_data.cast::<bindings::seq_file>()) };
+    // SAFETY: By caller precondition, this pointer is live and points to a value of type `T`.
+    let data = unsafe { &*(seq.private as *const T) };
+    update(data, buf, count)
+}
+
+// A trait to get the file operations for a type.
+pub(crate) trait ReadWriteFile<T> {
+    const FILE_OPS: FileOps<T>;
+}
+
+impl<T: Render + UpdateFromSlice + Sync> ReadWriteFile<T> for T {
+    const FILE_OPS: FileOps<T> = {
+        let operations = bindings::file_operations {
+            open: Some(render_open::<T>),
+            read: Some(bindings::seq_read),
+            write: Some(write::<T>),
+            llseek: Some(bindings::seq_lseek),
+            release: Some(bindings::single_release),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+        // SAFETY: `operations` is all stock `seq_file` implementations except for `render_open`
+        // and `write`.
+        // `render_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 matches the
+        // `FileOps` requirements.
+        // `write` only requires that the file's private data pointer points to `seq_file`
+        // which points to a `T` that will outlive it, which matches what `render_open`
+        // provides.
+        unsafe { FileOps::new(operations, 0o600) }
+    };
+}
+
+/// # Safety
+///
+/// `inode` must be a valid pointer to an `inode` struct.
+/// `file` must be a valid pointer to a `file` struct.
+unsafe extern "C" fn write_only_open(
+    inode: *mut bindings::inode,
+    file: *mut bindings::file,
+) -> c_int {
+    // SAFETY: The caller ensures that `inode` and `file` are valid pointers.
+    unsafe {
+        (*file).private_data = (*inode).i_private;
+    }
+    0
+}
+
+/// # Safety
+///
+/// * `file` must be a valid pointer to a `file` struct.
+/// * The `private_data` of the file must contain a valid pointer to a `T` that implements
+///   `UpdateFromSlice`.
+/// * `buf` must be a valid user-space buffer.
+pub(crate) unsafe extern "C" fn write_only_write<T: UpdateFromSlice + Sync>(
+    file: *mut bindings::file,
+    buf: *const c_char,
+    count: usize,
+    _ppos: *mut bindings::loff_t,
+) -> isize {
+    // SAFETY: The caller ensures that `file` is a valid pointer and that `private_data` holds a
+    // valid pointer to `T`.
+    let data = unsafe { &*((*file).private_data as *const T) };
+    update(data, buf, count)
+}
+
+pub(crate) trait WriteFile<T> {
+    const FILE_OPS: FileOps<T>;
+}
+
+impl<T: UpdateFromSlice + Sync> WriteFile<T> for T {
+    const FILE_OPS: FileOps<T> = {
+        let operations = bindings::file_operations {
+            open: Some(write_only_open),
+            write: Some(write_only_write::<T>),
+            llseek: Some(bindings::noop_llseek),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+        // SAFETY:
+        // * `write_only_open` populates the file private data with the inode private data
+        // * `write_only_write`'s only requirement is that the private data of the file point to
+        //   a `T` and be legal to convert to a shared reference, which `write_only_open`
+        //   satisfies.
+        unsafe { FileOps::new(operations, 0o200) }
+    };
+}
diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
index 2939e18e3dda39571cd7255505e5f605f0e3d154..d64638898faaa1a6a9898c374b8c1114993376c9 100644
--- a/rust/kernel/debugfs/traits.rs
+++ b/rust/kernel/debugfs/traits.rs
@@ -3,8 +3,15 @@
 
 //! Traits for rendering or updating values exported to DebugFS.
 
+use crate::prelude::*;
 use crate::sync::Mutex;
+use crate::uaccess::UserSliceReader;
 use core::fmt::{self, Debug, Formatter};
+use core::str::FromStr;
+use core::sync::atomic::{
+    AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicU16, AtomicU32, AtomicU64,
+    AtomicU8, AtomicUsize, Ordering,
+};
 
 /// A trait for types that can be rendered into a string.
 ///
@@ -26,3 +33,65 @@ fn render(&self, f: &mut Formatter<'_>) -> fmt::Result {
         writeln!(f, "{self:?}")
     }
 }
+
+/// A trait for types that can be updated from a user slice.
+///
+/// This works similarly to `FromStr`, but operates on a `UserSliceReader` rather than a &str.
+///
+/// It is automatically implemented for all atomic integers, or any type that implements `FromStr`
+/// wrapped in a `Mutex`.
+pub trait UpdateFromSlice {
+    /// Updates the value from the given user slice.
+    fn update_from_slice(&self, reader: &mut UserSliceReader) -> Result<()>;
+}
+
+impl<T: FromStr> UpdateFromSlice for Mutex<T> {
+    fn update_from_slice(&self, reader: &mut UserSliceReader) -> Result<()> {
+        let mut buf = [0u8; 128];
+        if reader.len() > buf.len() {
+            return Err(EINVAL);
+        }
+        let n = reader.len();
+        reader.read_slice(&mut buf[..n])?;
+
+        let s = core::str::from_utf8(&buf[..n]).map_err(|_| EINVAL)?;
+        let val = s.trim().parse::<T>().map_err(|_| EINVAL)?;
+        *self.lock() = val;
+        Ok(())
+    }
+}
+
+macro_rules! impl_update_from_slice_for_atomic {
+    ($(($atomic_type:ty, $int_type:ty)),*) => {
+        $(
+            impl UpdateFromSlice for $atomic_type {
+                fn update_from_slice(&self, reader: &mut UserSliceReader) -> Result<()> {
+                    let mut buf = [0u8; 21]; // Enough for a 64-bit number.
+                    if reader.len() > buf.len() {
+                        return Err(EINVAL);
+                    }
+                    let n = reader.len();
+                    reader.read_slice(&mut buf[..n])?;
+
+                    let s = core::str::from_utf8(&buf[..n]).map_err(|_| EINVAL)?;
+                    let val = s.trim().parse::<$int_type>().map_err(|_| EINVAL)?;
+                    self.store(val, Ordering::Relaxed);
+                    Ok(())
+                }
+            }
+        )*
+    };
+}
+
+impl_update_from_slice_for_atomic!(
+    (AtomicI16, i16),
+    (AtomicI32, i32),
+    (AtomicI64, i64),
+    (AtomicI8, i8),
+    (AtomicIsize, isize),
+    (AtomicU16, u16),
+    (AtomicU32, u32),
+    (AtomicU64, u64),
+    (AtomicU8, u8),
+    (AtomicUsize, usize)
+);

-- 
2.51.0.rc1.167.g924127e9c0-goog


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

* [PATCH v10 4/7] rust: debugfs: Add support for callback-based files
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
                   ` (2 preceding siblings ...)
  2025-08-19 22:53 ` [PATCH v10 3/7] rust: debugfs: Add support for writable files Matthew Maurer
@ 2025-08-19 22:53 ` Matthew Maurer
  2025-08-19 22:53 ` [PATCH v10 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Extends the `debugfs` API to support creating files with content
generated and updated by callbacks. This is done via the
`read_callback_file`, `write_callback_file`, and
`read_write_callback_file` methods.

These methods allow for more flexible file definition, either because
the type already has a `Render` or `UpdateFromSlice` method that doesn't
do what you'd like, or because you cannot implement it (e.g. because
it's a type defined in another crate or a primitive type).

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/debugfs.rs                   |  95 ++++++++++++++++++++++++
 rust/kernel/debugfs/callback_adapters.rs | 122 +++++++++++++++++++++++++++++++
 rust/kernel/debugfs/file_ops.rs          |   8 ++
 3 files changed, 225 insertions(+)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 62bc2b1d4e5a4b21441a09e03bff74c32c6781d2..a843d01506a54d5f8626dab5223d006c9a363a91 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -12,12 +12,16 @@
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
+use crate::uaccess::UserSliceReader;
+use core::fmt;
 use core::marker::PhantomPinned;
 use core::ops::Deref;
 
 mod traits;
 pub use traits::{Render, UpdateFromSlice};
 
+mod callback_adapters;
+use callback_adapters::{FormatAdapter, NoRender, WritableAdapter};
 mod file_ops;
 use file_ops::{FileOps, ReadFile, ReadWriteFile, WriteFile};
 #[cfg(CONFIG_DEBUG_FS)]
@@ -137,6 +141,48 @@ pub fn read_only_file<'a, T: Render + Send + Sync + 'static, E: 'a, TI: PinInit<
         self.create_file(name, data, file_ops)
     }
 
+    /// Creates a read-only file in this directory, with contents from a callback.
+    ///
+    /// `f` must be a function item or a non-capturing closure.
+    /// This is statically asserted and not a safety requirement.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use core::sync::atomic::{AtomicU32, Ordering};
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// # use kernel::prelude::*;
+    /// # let dir = Dir::new(c_str!("foo"));
+    /// let file = KBox::pin_init(
+    ///     dir.read_callback_file(c_str!("bar"),
+    ///     AtomicU32::new(3),
+    ///     &|val, f| {
+    ///       let out = val.load(Ordering::Relaxed);
+    ///       writeln!(f, "{out:#010x}")
+    ///     }),
+    ///     GFP_KERNEL)?;
+    /// // Reading "foo/bar" will show "0x00000003".
+    /// file.store(10, Ordering::Relaxed);
+    /// // Reading "foo/bar" will now show "0x0000000a".
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn read_callback_file<
+        'a,
+        T: Send + Sync + 'static,
+        E: 'a,
+        TI: PinInit<T, E> + 'a,
+        F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+    >(
+        &'a self,
+        name: &'a CStr,
+        data: TI,
+        _f: &'static F,
+    ) -> impl PinInit<File<T>, E> + 'a {
+        let file_ops = <FormatAdapter<T, F>>::FILE_OPS.adapt();
+        self.create_file(name, data, file_ops)
+    }
+
     /// Creates a read-write file in this directory.
     ///
     /// Reading the file uses the [`Render`] implementation.
@@ -155,6 +201,33 @@ pub fn read_write_file<
         self.create_file(name, data, file_ops)
     }
 
+    /// Creates a read-write file in this directory, with logic from callbacks.
+    ///
+    /// Reading from the file is handled by `f`. Writing to the file is handled by `w`.
+    ///
+    /// `f` and `w` must be function items or non-capturing closures.
+    /// This is statically asserted and not a safety requirement.
+    pub fn read_write_callback_file<
+        'a,
+        T: Send + Sync + 'static,
+        E: 'a,
+        TI: PinInit<T, E> + 'a,
+        F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+        W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+    >(
+        &'a self,
+        name: &'a CStr,
+        data: TI,
+        _f: &'static F,
+        _w: &'static W,
+    ) -> impl PinInit<File<T>, E> + 'a {
+        let file_ops =
+            <WritableAdapter<FormatAdapter<T, F>, W> as file_ops::ReadWriteFile<_>>::FILE_OPS
+                .adapt()
+                .adapt();
+        self.create_file(name, data, file_ops)
+    }
+
     /// Creates a write-only file in this directory.
     ///
     /// The file owns its backing data. Writing to the file uses the [`UpdateFromSlice`]
@@ -173,6 +246,28 @@ pub fn write_only_file<
     ) -> impl PinInit<File<T>, E> + 'a {
         self.create_file(name, data, &T::FILE_OPS)
     }
+
+    /// Creates a write-only file in this directory, with write logic from a callback.
+    ///
+    /// `w` must be a function item or a non-capturing closure.
+    /// This is statically asserted and not a safety requirement.
+    pub fn write_callback_file<
+        'a,
+        T: Send + Sync + 'static,
+        E: 'a,
+        TI: PinInit<T, E> + 'a,
+        W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+    >(
+        &'a self,
+        name: &'a CStr,
+        data: TI,
+        _w: &'static W,
+    ) -> impl PinInit<File<T>, E> + 'a {
+        let file_ops = <WritableAdapter<NoRender<T>, W> as WriteFile<_>>::FILE_OPS
+            .adapt()
+            .adapt();
+        self.create_file(name, data, file_ops)
+    }
 }
 
 #[pin_data]
diff --git a/rust/kernel/debugfs/callback_adapters.rs b/rust/kernel/debugfs/callback_adapters.rs
new file mode 100644
index 0000000000000000000000000000000000000000..1b6001306bf4efabf144cb24aa793e07ea8ac2fb
--- /dev/null
+++ b/rust/kernel/debugfs/callback_adapters.rs
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+//! Adapters which allow the user to supply a render or update implementation as a value rather
+//! than a trait implementation. If provided, it will override the trait implementation.
+
+use super::{Render, UpdateFromSlice};
+use crate::prelude::*;
+use crate::uaccess::UserSliceReader;
+use core::fmt;
+use core::fmt::Formatter;
+use core::marker::PhantomData;
+use core::ops::Deref;
+
+/// # Safety
+///
+/// To implement this trait, it must be safe to cast a `&Self` to a `&Inner`.
+/// It is intended for use in unstacking adapters out of `FileOps` backings.
+pub(crate) unsafe trait Adapter {
+    type Inner;
+}
+
+/// Adapter to implement `UpdateFromSlice` via a callback with the same representation as `T`.
+///
+/// * Layer it on top of `RenderAdapter` if you want to add a custom callback for `render`.
+/// * Layer it on top of `NoRender` to pass through any support present on the underlying type.
+///
+/// # Invariants
+///
+/// If an instance for `WritableAdapter<_, W>` is constructed, `W` is inhabited.
+#[repr(transparent)]
+pub(crate) struct WritableAdapter<D, W> {
+    inner: D,
+    _writer: PhantomData<W>,
+}
+
+// SAFETY: Stripping off the adapter only removes constraints
+unsafe impl<D, W> Adapter for WritableAdapter<D, W> {
+    type Inner = D;
+}
+
+impl<D: Render, W> Render for WritableAdapter<D, W> {
+    fn render(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.inner.render(fmt)
+    }
+}
+
+impl<D: Deref, W> UpdateFromSlice for WritableAdapter<D, W>
+where
+    W: Fn(&D::Target, &mut UserSliceReader) -> Result + Send + Sync + 'static,
+{
+    fn update_from_slice(&self, reader: &mut UserSliceReader) -> Result {
+        // SAFETY: WritableAdapter<_, W> can only be constructed if W is inhabited
+        let w: &W = unsafe { materialize_zst() };
+        w(self.inner.deref(), reader)
+    }
+}
+
+/// Adapter to implement `Render` 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> Deref for FormatAdapter<D, F> {
+    type Target = D;
+    fn deref(&self) -> &D {
+        &self.inner
+    }
+}
+
+impl<D, F> Render for FormatAdapter<D, F>
+where
+    F: Fn(&D, &mut Formatter<'_>) -> fmt::Result + 'static,
+{
+    fn render(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
+        // SAFETY: FormatAdapter<_, F> can only be constructed if F is inhabited
+        let f: &F = unsafe { materialize_zst() };
+        f(&self.inner, fmt)
+    }
+}
+
+// SAFETY: Stripping off the adapter only removes constraints
+unsafe impl<D, F> Adapter for FormatAdapter<D, F> {
+    type Inner = D;
+}
+
+#[repr(transparent)]
+pub(crate) struct NoRender<D> {
+    inner: D,
+}
+
+// SAFETY: Stripping off the adapter only removes constraints
+unsafe impl<D> Adapter for NoRender<D> {
+    type Inner = D;
+}
+
+impl<D> Deref for NoRender<D> {
+    type Target = D;
+    fn deref(&self) -> &D {
+        &self.inner
+    }
+}
+
+/// For types with a unique value, produce a static reference to it.
+///
+/// # Safety
+///
+/// The caller asserts that F is inhabited
+unsafe fn materialize_zst<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() }
+}
diff --git a/rust/kernel/debugfs/file_ops.rs b/rust/kernel/debugfs/file_ops.rs
index 30f6a0532c7f5f4a2974edc8f1100f5485aa8da9..d11c09520d4464417003362b7468c9b9e1a2e1bf 100644
--- a/rust/kernel/debugfs/file_ops.rs
+++ b/rust/kernel/debugfs/file_ops.rs
@@ -2,6 +2,7 @@
 // Copyright (C) 2025 Google LLC.
 
 use super::{Render, UpdateFromSlice};
+use crate::debugfs::callback_adapters::Adapter;
 use crate::prelude::*;
 use crate::seq_file::SeqFile;
 use crate::seq_print;
@@ -44,6 +45,13 @@ pub(crate) const fn mode(&self) -> u16 {
     }
 }
 
+impl<T: Adapter> FileOps<T> {
+    pub(super) const fn adapt(&self) -> &FileOps<T::Inner> {
+        // SAFETY: `Adapter` asserts that `T` can be legally cast to `T::Inner`.
+        unsafe { core::mem::transmute(self) }
+    }
+}
+
 #[cfg(CONFIG_DEBUG_FS)]
 impl<T> Deref for FileOps<T> {
     type Target = bindings::file_operations;

-- 
2.51.0.rc1.167.g924127e9c0-goog


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

* [PATCH v10 5/7] samples: rust: Add debugfs sample driver
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
                   ` (3 preceding siblings ...)
  2025-08-19 22:53 ` [PATCH v10 4/7] rust: debugfs: Add support for callback-based files Matthew Maurer
@ 2025-08-19 22:53 ` Matthew Maurer
  2025-08-20  0:34   ` Danilo Krummrich
  2025-08-19 22:53 ` [PATCH v10 6/7] rust: debugfs: Add support for scoped directories Matthew Maurer
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Adds a new sample driver that demonstrates the debugfs APIs.

The driver creates a directory in debugfs and populates it with a few
files:
- A read-only file that displays a fwnode property.
- A read-write file that exposes an atomic counter.
- A read-write file that exposes a custom struct.

This sample serves as a basic example of how to use the `debugfs::Dir`
and `debugfs::File` APIs to create and manage debugfs entries.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 8f2dbf71ca3f8f97e4d7619375279ed11d1261b2..5b6db584a33dd7ee39de3fdd0085d2bd7b7bef0e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7481,6 +7481,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..8d394f06b37e69ea1c30a3b0d8444c80562cc5ab
--- /dev/null
+++ b/samples/rust/rust_debugfs.rs
@@ -0,0 +1,151 @@
+// 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::str::FromStr;
+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,
+}
+
+impl FromStr for Inner {
+    type Err = Error;
+    fn from_str(s: &str) -> Result<Self> {
+        let mut parts = s.split_whitespace();
+        let x = parts
+            .next()
+            .ok_or(EINVAL)?
+            .parse::<u32>()
+            .map_err(|_| EINVAL)?;
+        let y = parts
+            .next()
+            .ok_or(EINVAL)?
+            .parse::<u32>()
+            .map_err(|_| EINVAL)?;
+        if parts.next().is_some() {
+            return Err(EINVAL);
+        }
+        Ok(Inner { x, y })
+    }
+}
+
+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>> + '_ {
+        dir.read_write_file(c_str!("counter"), AtomicUsize::new(0))
+    }
+
+    fn build_inner(dir: &Dir) -> impl PinInit<File<Mutex<Inner>>> + '_ {
+        dir.read_write_file(c_str!("pair"), new_mutex!(Inner { x: 3, y: 10 }))
+    }
+
+    fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
+        let debugfs = Dir::new(c_str!("sample_debugfs"));
+        let dev = pdev.as_ref();
+
+        try_pin_init! {
+            Self {
+                _compatible <- debugfs.read_only_file(
+                    c_str!("compatible"),
+                    dev.fwnode()
+                        .ok_or(ENOENT)?
+                        .property_read::<CString>(c_str!("compatible"))
+                        .required_by(dev)?,
+                ),
+                counter <- Self::build_counter(&debugfs),
+                inner <- Self::build_inner(&debugfs),
+                _debugfs: debugfs,
+                pdev: pdev.into(),
+            }
+        }
+    }
+}

-- 
2.51.0.rc1.167.g924127e9c0-goog


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

* [PATCH v10 6/7] rust: debugfs: Add support for scoped directories
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
                   ` (4 preceding siblings ...)
  2025-08-19 22:53 ` [PATCH v10 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
@ 2025-08-19 22:53 ` Matthew Maurer
  2025-08-19 22:53 ` [PATCH v10 7/7] samples: rust: Add scoped debugfs sample driver Matthew Maurer
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Introduces the concept of a `ScopedDir`, which allows for the creation
of debugfs directories and files that are tied to the lifetime of a
particular data structure. This ensures that debugfs entries do not
outlive the data they refer to.

The new `Dir::scope` method creates a new directory that is owned by a
`Scope` handle. All files and subdirectories created within this scope
are automatically cleaned up when the `Scope` is dropped.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/debugfs.rs       | 267 ++++++++++++++++++++++++++++++++++++++++++-
 rust/kernel/debugfs/entry.rs |  73 +++++++++++-
 2 files changed, 330 insertions(+), 10 deletions(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index a843d01506a54d5f8626dab5223d006c9a363a91..5e2b60cc1ea3eff859dbad8d7dd7a84d7c08d766 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -14,7 +14,10 @@
 use crate::sync::Arc;
 use crate::uaccess::UserSliceReader;
 use core::fmt;
+use core::marker::PhantomData;
 use core::marker::PhantomPinned;
+#[cfg(CONFIG_DEBUG_FS)]
+use core::mem::ManuallyDrop;
 use core::ops::Deref;
 
 mod traits;
@@ -40,7 +43,7 @@
 // able to refer to us. In this case, we need to silently fail. All future child directories/files
 // will silently fail as well.
 #[derive(Clone)]
-pub struct Dir(#[cfg(CONFIG_DEBUG_FS)] Option<Arc<Entry>>);
+pub struct Dir(#[cfg(CONFIG_DEBUG_FS)] Option<Arc<Entry<'static>>>);
 
 impl Dir {
     /// Create a new directory in DebugFS. If `parent` is [`None`], it will be created at the root.
@@ -268,6 +271,54 @@ pub fn write_callback_file<
             .adapt();
         self.create_file(name, data, file_ops)
     }
+
+    // While this function is safe, it is intentionally not public because it's a bit of a
+    // footgun.
+    //
+    // Unless you also extract the `entry` later and schedule it for `Drop` at the appropriate
+    // time, a `ScopedDir` with a `Dir` parent will never be deleted.
+    fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            let parent_entry = match &self.0 {
+                None => return ScopedDir::empty(),
+                Some(entry) => entry.clone(),
+            };
+            ScopedDir {
+                entry: ManuallyDrop::new(Entry::dynamic_dir(name, Some(parent_entry))),
+                _phantom: PhantomData,
+            }
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        ScopedDir::empty()
+    }
+
+    /// Creates a new scope, which is a directory associated with some data `T`.
+    ///
+    /// The created directory will be a subdirectory of `self`. The `init` closure is called to
+    /// populate the directory with files and subdirectories. These files can reference the data
+    /// stored in the scope.
+    ///
+    /// The entire directory tree created within the scope will be removed when the returned
+    /// `Scope` handle is dropped.
+    pub fn scope<
+        'a,
+        T: 'a,
+        E: 'a,
+        TI: PinInit<T, E> + 'a,
+        F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
+    >(
+        &'a self,
+        data: TI,
+        name: &'a CStr,
+        init: F,
+    ) -> impl PinInit<Scope<T>, E> + 'a {
+        Scope::new(data, |data| {
+            let scoped = self.scoped_dir(name);
+            init(data, &scoped);
+            scoped.into_entry()
+        })
+    }
 }
 
 #[pin_data]
@@ -276,7 +327,7 @@ pub fn write_callback_file<
 pub struct Scope<T> {
     // This order is load-bearing for drops - `_entry` must be dropped before `data`.
     #[cfg(CONFIG_DEBUG_FS)]
-    _entry: Entry,
+    _entry: Entry<'static>,
     #[pin]
     data: T,
     // Even if `T` is `Unpin`, we still can't allow it to be moved.
@@ -314,11 +365,11 @@ fn new<E, TI: PinInit<T, E>, F: for<'a> FnOnce(&'a T)>(
 
 #[cfg(CONFIG_DEBUG_FS)]
 impl<T> Scope<T> {
-    fn entry_mut(self: Pin<&mut Self>) -> &mut Entry {
+    fn entry_mut(self: Pin<&mut Self>) -> &mut Entry<'static> {
         // SAFETY: _entry is not structurally pinned
         unsafe { &mut Pin::into_inner_unchecked(self)._entry }
     }
-    fn new<'b, E: 'b, TI: PinInit<T, E> + 'b, F: for<'a> FnOnce(&'a T) -> Entry + 'b>(
+    fn new<'b, E: 'b, TI: PinInit<T, E> + 'b, F: for<'a> FnOnce(&'a T) -> Entry<'static> + 'b>(
         data: TI,
         init: F,
     ) -> impl PinInit<Self, E> + 'b
@@ -339,6 +390,36 @@ fn new<'b, E: 'b, TI: PinInit<T, E> + 'b, F: for<'a> FnOnce(&'a T) -> Entry + 'b
     }
 }
 
+impl<T> Scope<T> {
+    /// Creates a new scope, which is a directory at the root of the debugfs filesystem,
+    /// associated with some data `T`.
+    ///
+    /// The `init` closure is called to populate the directory with files and subdirectories. These
+    /// files can reference the data stored in the scope.
+    ///
+    /// The entire directory tree created within the scope will be removed when the returned
+    /// `Scope` handle is dropped.
+    pub fn dir<
+        'a,
+        E: 'a,
+        TI: PinInit<T, E> + 'a,
+        F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
+    >(
+        data: TI,
+        name: &'a CStr,
+        init: F,
+    ) -> impl PinInit<Self, E> + 'a
+    where
+        T: 'a,
+    {
+        Scope::new(data, |data| {
+            let scoped = ScopedDir::new(name);
+            init(data, &scoped);
+            scoped.into_entry()
+        })
+    }
+}
+
 impl<T> Deref for Scope<T> {
     type Target = T;
     fn deref(&self) -> &T {
@@ -352,3 +433,181 @@ fn deref(&self) -> &T {
         &self.scope
     }
 }
+
+/// A handle to a directory which will live at most `'dir`, accessing data that will live for at
+/// least `'data`.
+///
+/// Dropping a ScopedDir will not delete or clean it up, this is expected to occur through dropping
+/// the `Scope` that created it.
+pub struct ScopedDir<'data, 'dir> {
+    #[cfg(CONFIG_DEBUG_FS)]
+    entry: ManuallyDrop<Entry<'dir>>,
+    _phantom: PhantomData<fn(&'data ()) -> &'dir ()>,
+}
+
+impl<'data, 'dir> ScopedDir<'data, 'dir> {
+    /// Creates a subdirectory inside this `ScopedDir`.
+    ///
+    /// The returned directory handle cannot outlive this one.
+    pub fn dir<'dir2>(&'dir2 self, name: &CStr) -> ScopedDir<'data, 'dir2> {
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        let _ = name;
+        ScopedDir {
+            #[cfg(CONFIG_DEBUG_FS)]
+            entry: ManuallyDrop::new(Entry::dir(name, Some(&*self.entry))),
+            _phantom: PhantomData,
+        }
+    }
+
+    fn create_file<T: Sync>(&self, name: &CStr, data: &'data T, vtable: &'static FileOps<T>) {
+        #[cfg(CONFIG_DEBUG_FS)]
+        core::mem::forget(Entry::file(name, &self.entry, data, vtable));
+    }
+
+    /// Creates a read-only file in this directory.
+    ///
+    /// The file's contents are produced by invoking [`Render::render`]`.
+    ///
+    /// This function does not produce an owning handle to the file. The created
+    /// file is removed when the [`Scope`] that this directory belongs
+    /// to is dropped.
+    pub fn read_only_file<T: Render + Send + Sync + 'static>(&self, name: &CStr, data: &'data T) {
+        self.create_file(name, data, &T::FILE_OPS)
+    }
+
+    /// Creates a read-only file in this directory, with contents from a callback.
+    ///
+    /// The file contents are generated by calling `f` with `data`.
+    ///
+    ///
+    /// `f` must be a function item or a non-capturing closure.
+    /// This is statically asserted and not a safety requirement.
+    ///
+    /// This function does not produce an owning handle to the file. The created
+    /// file is removed when the [`Scope`] that this directory belongs
+    /// to is dropped.
+    pub fn read_callback_file<
+        T: Send + Sync + 'static,
+        F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+    >(
+        &self,
+        name: &CStr,
+        data: &'data T,
+        _f: &'static F,
+    ) {
+        let vtable = <FormatAdapter<T, F> as ReadFile<_>>::FILE_OPS.adapt();
+        self.create_file(name, data, vtable)
+    }
+
+    /// Creates a read-write file in this directory.
+    ///
+    /// Reading the file uses the [`Render`] implementation on `data`. Writing to the file uses
+    /// the [`UpdateFromSlice`] implementation on `data`.
+    ///
+    /// This function does not produce an owning handle to the file. The created
+    /// file is removed when the [`Scope`] that this directory belongs
+    /// to is dropped.
+    pub fn read_write_file<T: Render + UpdateFromSlice + Send + Sync + 'static>(
+        &self,
+        name: &CStr,
+        data: &'data T,
+    ) {
+        let vtable = &<T as ReadWriteFile<_>>::FILE_OPS;
+        self.create_file(name, data, vtable)
+    }
+
+    /// Creates a read-write file in this directory, with logic from callbacks.
+    ///
+    /// Reading from the file is handled by `f`. Writing to the file is handled by `w`.
+    ///
+    /// `f` and `w` must be function items or non-capturing closures.
+    /// This is statically asserted and not a safety requirement.
+    ///
+    /// This function does not produce an owning handle to the file. The created
+    /// file is removed when the [`Scope`] that this directory belongs
+    /// to is dropped.
+    pub fn read_write_callback_file<
+        T: Send + Sync + 'static,
+        F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+        W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+    >(
+        &self,
+        name: &CStr,
+        data: &'data T,
+        _f: &'static F,
+        _w: &'static W,
+    ) {
+        let vtable = <WritableAdapter<FormatAdapter<T, F>, W> as ReadWriteFile<_>>::FILE_OPS
+            .adapt()
+            .adapt();
+        self.create_file(name, data, vtable)
+    }
+
+    /// Creates a write-only file in this directory.
+    ///
+    /// Writing to the file uses the [`UpdateFromSlice`] implementation on `data`.
+    ///
+    /// This function does not produce an owning handle to the file. The created
+    /// file is removed when the [`Scope`] that this directory belongs
+    /// to is dropped.
+    pub fn write_only_file<T: UpdateFromSlice + Send + Sync + 'static>(
+        &self,
+        name: &CStr,
+        data: &'data T,
+    ) {
+        let vtable = &<T as WriteFile<_>>::FILE_OPS;
+        self.create_file(name, data, vtable)
+    }
+
+    /// Creates a write-only file in this directory, with write logic from a callback.
+    ///
+    /// Writing to the file is handled by `w`.
+    ///
+    /// `w` must be a function item or a non-capturing closure.
+    /// This is statically asserted and not a safety requirement.
+    ///
+    /// This function does not produce an owning handle to the file. The created
+    /// file is removed when the [`Scope`] that this directory belongs
+    /// to is dropped.
+    pub fn write_only_callback_file<
+        T: Send + Sync + 'static,
+        W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+    >(
+        &self,
+        name: &CStr,
+        data: &'data T,
+        _w: &'static W,
+    ) {
+        let vtable = &<WritableAdapter<NoRender<T>, W> as WriteFile<_>>::FILE_OPS
+            .adapt()
+            .adapt();
+        self.create_file(name, data, vtable)
+    }
+
+    fn empty() -> Self {
+        ScopedDir {
+            #[cfg(CONFIG_DEBUG_FS)]
+            entry: ManuallyDrop::new(Entry::empty()),
+            _phantom: PhantomData,
+        }
+    }
+    #[cfg(CONFIG_DEBUG_FS)]
+    fn into_entry(self) -> Entry<'dir> {
+        ManuallyDrop::into_inner(self.entry)
+    }
+    #[cfg(not(CONFIG_DEBUG_FS))]
+    fn into_entry(self) {}
+}
+
+impl<'data> ScopedDir<'data, 'static> {
+    // This is safe, but intentionally not exported due to footgun status. A ScopedDir with no
+    // parent will never be released by default, and needs to have its entry extracted and used
+    // somewhere.
+    fn new(name: &CStr) -> ScopedDir<'data, 'static> {
+        ScopedDir {
+            #[cfg(CONFIG_DEBUG_FS)]
+            entry: ManuallyDrop::new(Entry::dir(name, None)),
+            _phantom: PhantomData,
+        }
+    }
+}
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
index 227fa50b7a79aeab49779e54b8c4241f455777c3..f99402cd3ba0ca12f62d3699e4d6e460d0085d26 100644
--- a/rust/kernel/debugfs/entry.rs
+++ b/rust/kernel/debugfs/entry.rs
@@ -5,26 +5,29 @@
 use crate::ffi::c_void;
 use crate::str::CStr;
 use crate::sync::Arc;
+use core::marker::PhantomData;
 
 /// 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 {
+pub(crate) struct Entry<'a> {
     entry: *mut bindings::dentry,
     // If we were created with an owning parent, this is the keep-alive
-    _parent: Option<Arc<Entry>>,
+    _parent: Option<Arc<Entry<'static>>>,
+    // If we were created with a non-owning parent, this prevents us from outliving it
+    _phantom: PhantomData<&'a ()>,
 }
 
 // SAFETY: [`Entry`] is just a `dentry` under the hood, which the API promises can be transferred
 // between threads.
-unsafe impl Send for Entry {}
+unsafe impl Send for Entry<'_> {}
 
 // SAFETY: All the C functions we call on the `dentry` pointer are threadsafe.
-unsafe impl Sync for Entry {}
+unsafe impl Sync for Entry<'_> {}
 
-impl Entry {
+impl Entry<'static> {
     pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
         let parent_ptr = match &parent {
             Some(entry) => entry.as_ptr(),
@@ -39,6 +42,7 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
         Entry {
             entry,
             _parent: parent,
+            _phantom: PhantomData,
         }
     }
 
@@ -71,14 +75,71 @@ pub(crate) unsafe fn dynamic_file<T>(
         Entry {
             entry,
             _parent: Some(parent),
+            _phantom: PhantomData,
         }
     }
+}
+
+impl<'a> Entry<'a> {
+    pub(crate) fn dir(name: &CStr, parent: Option<&'a Entry<'_>>) -> Self {
+        let parent_ptr = match &parent {
+            Some(entry) => entry.as_ptr(),
+            None => core::ptr::null_mut(),
+        };
+        // SAFETY: The invariants of this function's arguments ensure the safety of this call.
+        // * `name` is a valid C string by the invariants of `&CStr`.
+        // * `parent_ptr` is either `NULL` (if `parent` is `None`), or a pointer to a valid
+        //   `dentry` (because `parent` is a valid reference to an `Entry`). The lifetime `'a`
+        //   ensures that the parent outlives this entry.
+        let entry = unsafe { bindings::debugfs_create_dir(name.as_char_ptr(), parent_ptr) };
+
+        Entry {
+            entry,
+            _parent: None,
+            _phantom: PhantomData,
+        }
+    }
+
+    pub(crate) fn file<T>(
+        name: &CStr,
+        parent: &'a Entry<'_>,
+        data: &'a T,
+        file_ops: &FileOps<T>,
+    ) -> Self {
+        // SAFETY: The invariants of this function's arguments ensure the safety of this call.
+        // * `name` is a valid C string by the invariants of `&CStr`.
+        // * `parent.as_ptr()` is a pointer to a valid `dentry` because we have `&'a Entry`.
+        // * `data` is a valid pointer to `T` for lifetime `'a`.
+        // * The returned `Entry` has lifetime `'a`, so it cannot outlive `parent` or `data`.
+        // * The caller guarantees that `vtable` is compatible with `data`.
+        // * The guarantees on `FileOps` assert the vtable will be compatible with the data we have
+        //   provided.
+        let entry = unsafe {
+            bindings::debugfs_create_file_full(
+                name.as_char_ptr(),
+                file_ops.mode(),
+                parent.as_ptr(),
+                core::ptr::from_ref(data) as *mut c_void,
+                core::ptr::null(),
+                &**file_ops,
+            )
+        };
+
+        Entry {
+            entry,
+            _parent: None,
+            _phantom: PhantomData,
+        }
+    }
+}
 
+impl Entry<'_> {
     /// Constructs a placeholder DebugFS [`Entry`].
     pub(crate) fn empty() -> Self {
         Self {
             entry: core::ptr::null_mut(),
             _parent: None,
+            _phantom: PhantomData,
         }
     }
 
@@ -94,7 +155,7 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::dentry {
     }
 }
 
-impl Drop for 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.

-- 
2.51.0.rc1.167.g924127e9c0-goog


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

* [PATCH v10 7/7] samples: rust: Add scoped debugfs sample driver
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
                   ` (5 preceding siblings ...)
  2025-08-19 22:53 ` [PATCH v10 6/7] rust: debugfs: Add support for scoped directories Matthew Maurer
@ 2025-08-19 22:53 ` Matthew Maurer
  2025-08-19 23:14 ` [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
  2025-08-25 11:51 ` Dirk Behme
  8 siblings, 0 replies; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 22:53 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Adds a new sample driver `rust_scoped_debugfs` that demonstrates the
use of the scoped debugfs APIs.

The driver creates a `control` directory with two write-only files,
`create` and `remove`. Writing a name and a series of numbers to
`create` will create a new subdirectory under a `dynamic` directory.
This new subdirectory will contain files that expose the numbers as
atomic values.

Writing a name to `remove` will remove the corresponding subdirectory
from the `dynamic` directory.

This sample serves as an example of how to use the `debugfs::Scope`
and `debugfs::ScopedDir` APIs to create and manage debugfs entries
that are tied to the lifetime of a data structure.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 5b6db584a33dd7ee39de3fdd0085d2bd7b7bef0e..2cbe890085dbb6a652623b38dd0eadeeaa127a94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7482,6 +7482,7 @@ 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_scoped_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 01101db41ae31b08a86d048cdd27da8ef9bb23a2..3372935519d658529ee7ba25fb2c3fff6adae8c4 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -73,6 +73,17 @@ config SAMPLE_RUST_DEBUGFS
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_SCOPED_DEBUGFS
+	tristate "Scoped DebugFS Test Module"
+	depends on DEBUG_FS
+	help
+	  This option builds the Rust Scoped DebugFS Test module sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_scoped_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 61276222a99f8cc6d7f84c26d0533b30815ebadd..de10b7f4db3996dc57be813ceb076d050ad8f65a 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -5,6 +5,7 @@ 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_SCOPED_DEBUGFS)	+= rust_scoped_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_scoped_debugfs.rs b/samples/rust/rust_scoped_debugfs.rs
new file mode 100644
index 0000000000000000000000000000000000000000..7c34cab62a2753d1ede3a1334be1fb13ddce780c
--- /dev/null
+++ b/samples/rust/rust_scoped_debugfs.rs
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Sample DebugFS exporting platform driver that demonstrates the use of
+//! `Scope::dir` to create a variety of files without the need to separately
+//! track them all.
+
+use core::sync::atomic::AtomicUsize;
+use kernel::debugfs::{Dir, Scope};
+use kernel::prelude::*;
+use kernel::sync::Mutex;
+use kernel::{c_str, new_mutex, str::CString};
+
+module! {
+    type: RustScopedDebugFs,
+    name: "rust_scoped_debugfs",
+    authors: ["Matthew Maurer"],
+    description: "Rust Scoped DebugFS usage sample",
+    license: "GPL",
+}
+
+fn remove_file_write(
+    mod_data: &ModuleData,
+    reader: &mut kernel::uaccess::UserSliceReader,
+) -> Result<()> {
+    let mut buf = [0u8; 128];
+    if reader.len() >= buf.len() {
+        return Err(EINVAL);
+    }
+    let n = reader.len();
+    reader.read_slice(&mut buf[..n])?;
+
+    let s = core::str::from_utf8(&buf[..n]).map_err(|_| EINVAL)?.trim();
+    let nul_idx = s.len();
+    buf[nul_idx] = 0;
+    let to_remove = CStr::from_bytes_with_nul(&buf[..nul_idx + 1]).map_err(|_| EINVAL)?;
+    mod_data
+        .devices
+        .lock()
+        .retain(|device| device.name.as_bytes() != to_remove.as_bytes());
+    Ok(())
+}
+
+fn create_file_write(
+    mod_data: &ModuleData,
+    reader: &mut kernel::uaccess::UserSliceReader,
+) -> Result<()> {
+    let mut buf = [0u8; 128];
+    if reader.len() > buf.len() {
+        return Err(EINVAL);
+    }
+    let n = reader.len();
+    reader.read_slice(&mut buf[..n])?;
+
+    let mut nums = KVec::new();
+
+    let s = core::str::from_utf8(&buf[..n]).map_err(|_| EINVAL)?.trim();
+    let mut items = s.split_whitespace();
+    let name_str = items.next().ok_or(EINVAL)?;
+    let name = CString::try_from_fmt(fmt!("{name_str}"))?;
+    let file_name = CString::try_from_fmt(fmt!("{name_str}"))?;
+    for sub in items {
+        nums.push(
+            AtomicUsize::new(sub.parse().map_err(|_| EINVAL)?),
+            GFP_KERNEL,
+        )?;
+    }
+
+    let scope = KBox::pin_init(
+        mod_data
+            .device_dir
+            .scope(DeviceData { name, nums }, &file_name, |dev_data, dir| {
+                for (idx, val) in dev_data.nums.iter().enumerate() {
+                    let Ok(name) = CString::try_from_fmt(fmt!("{idx}")) else {
+                        return;
+                    };
+                    dir.read_write_file(&name, val);
+                }
+            }),
+        GFP_KERNEL,
+    )?;
+    (*mod_data.devices.lock()).push(scope, GFP_KERNEL)?;
+
+    Ok(())
+}
+
+struct RustScopedDebugFs {
+    _data: Pin<KBox<Scope<ModuleData>>>,
+}
+
+#[pin_data]
+struct ModuleData {
+    device_dir: Dir,
+    #[pin]
+    devices: Mutex<KVec<Pin<KBox<Scope<DeviceData>>>>>,
+}
+
+impl ModuleData {
+    fn init(device_dir: Dir) -> impl PinInit<Self> {
+        pin_init! {
+            Self {
+                device_dir: device_dir,
+                devices <- new_mutex!(KVec::new())
+            }
+        }
+    }
+}
+
+struct DeviceData {
+    name: CString,
+    nums: KVec<AtomicUsize>,
+}
+
+fn init_control(base_dir: &Dir, dyn_dirs: Dir) -> impl PinInit<Scope<ModuleData>> + '_ {
+    base_dir.scope(
+        ModuleData::init(dyn_dirs),
+        c_str!("control"),
+        |data, dir| {
+            dir.write_only_callback_file(c_str!("create"), data, &create_file_write);
+            dir.write_only_callback_file(c_str!("remove"), data, &remove_file_write);
+        },
+    )
+}
+
+impl kernel::Module for RustScopedDebugFs {
+    fn init(_module: &'static kernel::ThisModule) -> Result<Self, Error> {
+        let base_dir = Dir::new(c_str!("rust_scoped_debugfs"));
+        let dyn_dirs = base_dir.subdir(c_str!("dynamic"));
+        Ok(Self {
+            _data: KBox::pin_init(init_control(&base_dir, dyn_dirs), GFP_KERNEL)?,
+        })
+    }
+}

-- 
2.51.0.rc1.167.g924127e9c0-goog


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

* Re: [PATCH v10 0/7] rust: DebugFS Bindings
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
                   ` (6 preceding siblings ...)
  2025-08-19 22:53 ` [PATCH v10 7/7] samples: rust: Add scoped debugfs sample driver Matthew Maurer
@ 2025-08-19 23:14 ` Matthew Maurer
  2025-08-25 11:51 ` Dirk Behme
  8 siblings, 0 replies; 17+ messages in thread
From: Matthew Maurer @ 2025-08-19 23:14 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, Dirk Beheme
  Cc: linux-kernel, rust-for-linux

On Tue, Aug 19, 2025 at 3:53 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> This series provides safe DebugFS bindings for Rust, with sample
> modules using them.
>
> Shortly after this is sent, you will see a real driver WIP using this
> implenting the qcom socinfo driver.

Now mailed. [1]

[1] https://lore.kernel.org/all/20250819-qcom-socinfo-v1-0-e8d32cc81270@google.com/

>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
> Changes in v10:
> - Introduced Scoped to show how either a File or Dir can be bound to
>   data
> - Remove use of `use<>` for MSRV compatibility
> - Added Write support
> - Added more complex sample driver using scoped interface
> - Updated original sample driver to use writes to drive mutation
> - Added `FileOps<T>` (only for DebugFS, not a kernel-wide abstraction)
>   to decrease needed `unsafe` for keeping vtables paired to types.
> - Centralized `dentry` lifecycle management to `entry.rs`.
> - Link to v9: https://lore.kernel.org/r/20250709-debugfs-rust-v9-0-92b9eab5a951@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 (7):
>       rust: debugfs: Add initial support for directories
>       rust: debugfs: Add support for read-only files
>       rust: debugfs: Add support for writable files
>       rust: debugfs: Add support for callback-based files
>       samples: rust: Add debugfs sample driver
>       rust: debugfs: Add support for scoped directories
>       samples: rust: Add scoped debugfs sample driver
>
>  MAINTAINERS                              |   4 +
>  rust/bindings/bindings_helper.h          |   1 +
>  rust/kernel/debugfs.rs                   | 613 +++++++++++++++++++++++++++++++
>  rust/kernel/debugfs/callback_adapters.rs | 122 ++++++
>  rust/kernel/debugfs/entry.rs             | 164 +++++++++
>  rust/kernel/debugfs/file_ops.rs          | 246 +++++++++++++
>  rust/kernel/debugfs/traits.rs            |  97 +++++
>  rust/kernel/lib.rs                       |   1 +
>  samples/rust/Kconfig                     |  22 ++
>  samples/rust/Makefile                    |   2 +
>  samples/rust/rust_debugfs.rs             | 151 ++++++++
>  samples/rust/rust_scoped_debugfs.rs      | 134 +++++++
>  12 files changed, 1557 insertions(+)
> ---
> base-commit: f3c5631f70e434e318c44001e2417d4770f06cd0
> change-id: 20250428-debugfs-rust-3cd5c97eb7d1
>
> Best regards,
> --
> Matthew Maurer <mmaurer@google.com>
>

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

* Re: [PATCH v10 5/7] samples: rust: Add debugfs sample driver
  2025-08-19 22:53 ` [PATCH v10 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
@ 2025-08-20  0:34   ` Danilo Krummrich
  2025-08-20  0:40     ` Matthew Maurer
  0 siblings, 1 reply; 17+ messages in thread
From: Danilo Krummrich @ 2025-08-20  0:34 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, Dirk Beheme, linux-kernel, rust-for-linux

On Wed Aug 20, 2025 at 12:53 AM CEST, Matthew Maurer wrote:
> Adds a new sample driver that demonstrates the debugfs APIs.
>
> The driver creates a directory in debugfs and populates it with a few
> files:
> - A read-only file that displays a fwnode property.
> - A read-write file that exposes an atomic counter.
> - A read-write file that exposes a custom struct.
>
> This sample serves as a basic example of how to use the `debugfs::Dir`
> and `debugfs::File` APIs to create and manage debugfs entries.
>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>

This is a great example, thanks! I really like how the API turned out.

When it comes to the newly added Scope API - and I assume this does not come at
a surprise - I have some concerns.

But first, thanks a lot for posting the socinfo driver in both variants, with
and without the Scope API.

I had a brief look at both of those and I can see why you want this.

With the Scope thing you can indeed write things a bit more compressed (I think
in the patches the differences looks quite a bit bigger than it actually is,
because the scope-based one uses quite some code from the file-based one).

I think the downsides are mainly:

  - The degree of complexity added for a rather specific use-case, that is also
    perfectly representable with the file-based API.

  - It makes it convinient to expose multiple fields grouped under the same lock
    as separate files, which design wise we shouln't encourage for the reasons
    we discussed in v8.

I think for the sake of getting this series merged, which I would really love to
see, I think we should focus on the file-based API first. Once we got this
landed I think we can still revisit the Scope idea and have some more discussion
about it.

I will have a more detailed look tomorrow (at least for the patches 1-5).

Thanks again for working on this!

- Danilo

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

* Re: [PATCH v10 5/7] samples: rust: Add debugfs sample driver
  2025-08-20  0:34   ` Danilo Krummrich
@ 2025-08-20  0:40     ` Matthew Maurer
  2025-08-20  0:42       ` Matthew Maurer
  2025-08-20  7:46       ` Benno Lossin
  0 siblings, 2 replies; 17+ messages in thread
From: Matthew Maurer @ 2025-08-20  0:40 UTC (permalink / raw)
  To: Danilo Krummrich
  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, Dirk Beheme, linux-kernel, rust-for-linux

On Tue, Aug 19, 2025 at 5:34 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Aug 20, 2025 at 12:53 AM CEST, Matthew Maurer wrote:
> > Adds a new sample driver that demonstrates the debugfs APIs.
> >
> > The driver creates a directory in debugfs and populates it with a few
> > files:
> > - A read-only file that displays a fwnode property.
> > - A read-write file that exposes an atomic counter.
> > - A read-write file that exposes a custom struct.
> >
> > This sample serves as a basic example of how to use the `debugfs::Dir`
> > and `debugfs::File` APIs to create and manage debugfs entries.
> >
> > Signed-off-by: Matthew Maurer <mmaurer@google.com>
>
> This is a great example, thanks! I really like how the API turned out.
>
> When it comes to the newly added Scope API - and I assume this does not come at
> a surprise - I have some concerns.

Yes, I expected this to be the case, but inspired by some of the
comments about wanting to just create files off fields and forget
about them, I wanted to take one more crack at it.

>
> But first, thanks a lot for posting the socinfo driver in both variants, with
> and without the Scope API.
>
> I had a brief look at both of those and I can see why you want this.
>
> With the Scope thing you can indeed write things a bit more compressed (I think
> in the patches the differences looks quite a bit bigger than it actually is,
> because the scope-based one uses quite some code from the file-based one).
>
> I think the downsides are mainly:
>
>   - The degree of complexity added for a rather specific use-case, that is also
>     perfectly representable with the file-based API.
I don't *think* this is just for this use case - if I just wanted to
improve the DebugFS use case, I'd mostly be looking at additional code
for `pin-init` (adding an `Option` placement + a few ergonomic
improvements to `pin_init` would slim off a large chunk of the code).
The idea here was that a file might not always directly correspond to
a field in a data structure, and the `File` API forces it to be one.
We could decide that forcing every file to be a data structure field
is a good idea, but I'm not certain it is.
>
>   - It makes it convinient to expose multiple fields grouped under the same lock
>     as separate files, which design wise we shouln't encourage for the reasons
>     we discussed in v8.
It's still pretty convenient to do this with `File`. I don't know how
common it'll be in kernel code, but in userspace Rust, `Arc<Mutex<T>>`
is a very common primitive. I would be unsurprised to see someone use
this pattern to expose separate fields as separate files if we go with
the `File` API.
>
> I think for the sake of getting this series merged, which I would really love to
> see, I think we should focus on the file-based API first. Once we got this
> landed I think we can still revisit the Scope idea and have some more discussion
> about it.

This is why I put the scope API and sample as patches on the end chain
of the series - it is possible to merge only the `File`-based API if
that's what we want to do first, and consider the rest later.

>
> I will have a more detailed look tomorrow (at least for the patches 1-5).
>
> Thanks again for working on this!
>
> - Danilo

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

* Re: [PATCH v10 5/7] samples: rust: Add debugfs sample driver
  2025-08-20  0:40     ` Matthew Maurer
@ 2025-08-20  0:42       ` Matthew Maurer
  2025-08-20  7:46       ` Benno Lossin
  1 sibling, 0 replies; 17+ messages in thread
From: Matthew Maurer @ 2025-08-20  0:42 UTC (permalink / raw)
  To: Danilo Krummrich
  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, Dirk Beheme, linux-kernel, rust-for-linux

On Tue, Aug 19, 2025 at 5:40 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> On Tue, Aug 19, 2025 at 5:34 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Wed Aug 20, 2025 at 12:53 AM CEST, Matthew Maurer wrote:
> > > Adds a new sample driver that demonstrates the debugfs APIs.
> > >
> > > The driver creates a directory in debugfs and populates it with a few
> > > files:
> > > - A read-only file that displays a fwnode property.
> > > - A read-write file that exposes an atomic counter.
> > > - A read-write file that exposes a custom struct.
> > >
> > > This sample serves as a basic example of how to use the `debugfs::Dir`
> > > and `debugfs::File` APIs to create and manage debugfs entries.
> > >
> > > Signed-off-by: Matthew Maurer <mmaurer@google.com>
> >
> > This is a great example, thanks! I really like how the API turned out.
> >
> > When it comes to the newly added Scope API - and I assume this does not come at
> > a surprise - I have some concerns.
>
> Yes, I expected this to be the case, but inspired by some of the
> comments about wanting to just create files off fields and forget
> about them, I wanted to take one more crack at it.
>
> >
> > But first, thanks a lot for posting the socinfo driver in both variants, with
> > and without the Scope API.
> >
> > I had a brief look at both of those and I can see why you want this.
> >
> > With the Scope thing you can indeed write things a bit more compressed (I think
> > in the patches the differences looks quite a bit bigger than it actually is,
> > because the scope-based one uses quite some code from the file-based one).
> >
> > I think the downsides are mainly:
> >
> >   - The degree of complexity added for a rather specific use-case, that is also
> >     perfectly representable with the file-based API.
> I don't *think* this is just for this use case - if I just wanted to
> improve the DebugFS use case, I'd mostly be looking at additional code
Edit: Socinfo use case
> for `pin-init` (adding an `Option` placement + a few ergonomic
> improvements to `pin_init` would slim off a large chunk of the code).
> The idea here was that a file might not always directly correspond to
> a field in a data structure, and the `File` API forces it to be one.
> We could decide that forcing every file to be a data structure field
> is a good idea, but I'm not certain it is.
> >
> >   - It makes it convinient to expose multiple fields grouped under the same lock
> >     as separate files, which design wise we shouln't encourage for the reasons
> >     we discussed in v8.
> It's still pretty convenient to do this with `File`. I don't know how
> common it'll be in kernel code, but in userspace Rust, `Arc<Mutex<T>>`
> is a very common primitive. I would be unsurprised to see someone use
> this pattern to expose separate fields as separate files if we go with
> the `File` API.
> >
> > I think for the sake of getting this series merged, which I would really love to
> > see, I think we should focus on the file-based API first. Once we got this
> > landed I think we can still revisit the Scope idea and have some more discussion
> > about it.
>
> This is why I put the scope API and sample as patches on the end chain
> of the series - it is possible to merge only the `File`-based API if
> that's what we want to do first, and consider the rest later.
>
> >
> > I will have a more detailed look tomorrow (at least for the patches 1-5).
> >
> > Thanks again for working on this!
> >
> > - Danilo

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

* Re: [PATCH v10 5/7] samples: rust: Add debugfs sample driver
  2025-08-20  0:40     ` Matthew Maurer
  2025-08-20  0:42       ` Matthew Maurer
@ 2025-08-20  7:46       ` Benno Lossin
  1 sibling, 0 replies; 17+ messages in thread
From: Benno Lossin @ 2025-08-20  7:46 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich
  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,
	Dirk Beheme, linux-kernel, rust-for-linux

On Wed Aug 20, 2025 at 2:40 AM CEST, Matthew Maurer wrote:
> On Tue, Aug 19, 2025 at 5:34 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Wed Aug 20, 2025 at 12:53 AM CEST, Matthew Maurer wrote:
>> > Adds a new sample driver that demonstrates the debugfs APIs.
>> >
>> > The driver creates a directory in debugfs and populates it with a few
>> > files:
>> > - A read-only file that displays a fwnode property.
>> > - A read-write file that exposes an atomic counter.
>> > - A read-write file that exposes a custom struct.
>> >
>> > This sample serves as a basic example of how to use the `debugfs::Dir`
>> > and `debugfs::File` APIs to create and manage debugfs entries.
>> >
>> > Signed-off-by: Matthew Maurer <mmaurer@google.com>
>>
>> This is a great example, thanks! I really like how the API turned out.
>>
>> When it comes to the newly added Scope API - and I assume this does not come at
>> a surprise - I have some concerns.
>
> Yes, I expected this to be the case, but inspired by some of the
> comments about wanting to just create files off fields and forget
> about them, I wanted to take one more crack at it.
>
>>
>> But first, thanks a lot for posting the socinfo driver in both variants, with
>> and without the Scope API.
>>
>> I had a brief look at both of those and I can see why you want this.
>>
>> With the Scope thing you can indeed write things a bit more compressed (I think
>> in the patches the differences looks quite a bit bigger than it actually is,
>> because the scope-based one uses quite some code from the file-based one).
>>
>> I think the downsides are mainly:
>>
>>   - The degree of complexity added for a rather specific use-case, that is also
>>     perfectly representable with the file-based API.
> I don't *think* this is just for this use case - if I just wanted to
> improve the DebugFS use case, I'd mostly be looking at additional code
> for `pin-init` (adding an `Option` placement + a few ergonomic

`Option` is currently not possible to support, since we can't set solely
the discriminant (it must always be a write to the entire enum, thus
requiring a move), see [1].

[1]: https://github.com/Rust-for-Linux/pin-init/issues/59

> improvements to `pin_init` would slim off a large chunk of the code).

I'd be interested in what kinds of improvements you need, maybe they are
simple enough to just include :)

---
Cheers,
Benno

> The idea here was that a file might not always directly correspond to
> a field in a data structure, and the `File` API forces it to be one.
> We could decide that forcing every file to be a data structure field
> is a good idea, but I'm not certain it is.

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

* Re: [PATCH v10 0/7] rust: DebugFS Bindings
  2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
                   ` (7 preceding siblings ...)
  2025-08-19 23:14 ` [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
@ 2025-08-25 11:51 ` Dirk Behme
  8 siblings, 0 replies; 17+ messages in thread
From: Dirk Behme @ 2025-08-25 11:51 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 20/08/2025 00:53, Matthew Maurer wrote:
> This series provides safe DebugFS bindings for Rust, with sample
> modules using them.
> 
> Shortly after this is sent, you will see a real driver WIP using this
> implenting the qcom socinfo driver.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
On ARM64 on top of v6.17-rc1 I applied this series to use
read_only_file() to export some u64 proprietary values.

If you like feel free to add

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

Thanks!

Dirk

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

* Re: [PATCH v10 1/7] rust: debugfs: Add initial support for directories
  2025-08-19 22:53 ` [PATCH v10 1/7] rust: debugfs: Add initial support for directories Matthew Maurer
@ 2025-08-26 15:39   ` Danilo Krummrich
  0 siblings, 0 replies; 17+ messages in thread
From: Danilo Krummrich @ 2025-08-26 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, Dirk Beheme, linux-kernel, rust-for-linux

On Wed Aug 20, 2025 at 12:53 AM CEST, Matthew Maurer wrote:
> +/// Owning handle to a DebugFS directory.
> +///
> +/// This directory will be removed when this handle has been dropped *and* all children have been

I'd say "The directory in the filesystem represented by [`Dir`] will be
removed..." to be extra clear.

> +/// removed.
> +// We hold a reference to our parent if it exists in the `Entry` to prevent the dentry we point
> +// to from being cleaned up when our parent is removed.

Please check punctuation, I think this is a bit hard to parse.

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

* Re: [PATCH v10 2/7] rust: debugfs: Add support for read-only files
  2025-08-19 22:53 ` [PATCH v10 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
@ 2025-08-26 18:45   ` Danilo Krummrich
  0 siblings, 0 replies; 17+ messages in thread
From: Danilo Krummrich @ 2025-08-26 18:45 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, Dirk Beheme, linux-kernel, rust-for-linux

On Wed Aug 20, 2025 at 12:53 AM CEST, Matthew Maurer wrote:
> +    /// Creates a DebugFS file which will own the data produced by the initializer provided in
> +    /// `data`.
> +    fn create_file<'a, T: Sync + 'static, E: 'a, TI: PinInit<T, E> + 'a>(
> +        &'a self,
> +        name: &'a CStr,
> +        data: TI,

Can you please use `impl PinInit<T, E> + 'a` directly? It's a bit more in line
with how we usually write this and I also think it makes things quite a bit more
obvious compared to hiding it behind the `TI` generic.

Also, can you please use a where clause when the trait and lifetime bounds are
getting a bit lengthy? I think for this method this is already justified.

Obviously, this applies to the whole patch (and likely subsequent ones).

> +        file_ops: &'static FileOps<T>,
> +    ) -> impl PinInit<File<T>, E> + 'a {
> +        let scope = Scope::<T>::new(data, move |data| {
> +            #[cfg(CONFIG_DEBUG_FS)]
> +            if let Some(parent) = &self.0 {
> +                // SAFETY: Because data derives from a scope, and our entry will be dropped before
> +                // the data is dropped, it is guaranteed to outlive the entry we return.
> +                unsafe { Entry::dynamic_file(name, parent.clone(), data, file_ops) }
> +            } else {
> +                Entry::empty()
> +            }
> +        });
> +        try_pin_init! {
> +            File {
> +                scope <- scope
> +            } ? E
> +        }
> +    }
> +
>      /// Create a new directory in DebugFS at the root.
>      ///
>      /// # Examples
> @@ -79,4 +110,113 @@ pub fn new(name: &CStr) -> Self {
>      pub fn subdir(&self, name: &CStr) -> Self {
>          Dir::create(name, Some(self))
>      }
> +
> +    /// Creates a read-only file in this directory.
> +    ///
> +    /// The file's contents are produced by invoking [`Render::render`] on the value initialized by
> +    /// `data`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// # use kernel::prelude::*;
> +    /// # let dir = Dir::new(c_str!("my_debugfs_dir"));
> +    /// let file = KBox::pin_init(dir.read_only_file(c_str!("foo"), 200), GFP_KERNEL)?;
> +    /// // "my_debugfs_dir/foo" now contains the number 200.
> +    /// // The file is removed when `file` is dropped.
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    pub fn read_only_file<'a, T: Render + Send + Sync + 'static, E: 'a, TI: PinInit<T, E> + 'a>(
> +        &'a self,
> +        name: &'a CStr,
> +        data: TI,
> +    ) -> impl PinInit<File<T>, E> + 'a {
> +        let file_ops = &<T as ReadFile<_>>::FILE_OPS;
> +        self.create_file(name, data, file_ops)
> +    }
> +}
> +
> +#[pin_data]
> +/// Handle to a DebugFS scope, which allows a variety of DebugFS files/directories to hang off a
> +/// single structure.
> +pub struct Scope<T> {

I think this doesn't need the Scope indirection just yet, but fine for me to add
it right away.

However, please make sure to expand the documentation a bit. You probably
don't want to mention where this is exactly going just yet, since technically
those patches should work out standalone in case the more sophisticated scope
stuff doesn't land.

But, I think you can still describe a bit more in detail that a debugfs::Scope
handles the lifetime of some private data T for a debugfs::Entry.

Subsequently I would mention that this is used for file entries only.

> +    // 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,
> +}
> +
> +#[pin_data]
> +/// Handle to a DebugFS file, owning its backing data.
> +///
> +/// When dropped, the DebugFS file will be removed and the attached data will be dropped.
> +pub struct File<T> {
> +    #[pin]
> +    scope: Scope<T>,
> +}
> +
> +#[cfg(not(CONFIG_DEBUG_FS))]
> +impl<T> Scope<T> {
> +    fn new<E, TI: PinInit<T, E>, F: for<'a> FnOnce(&'a T)>(
> +        data: TI,
> +        init: F,
> +    ) -> impl PinInit<Self, E> {
> +        try_pin_init! {
> +            Self {
> +                data <- data,
> +                _pin: PhantomPinned
> +            } ? E
> +        }
> +        .pin_chain(|scope| {
> +            init(&scope.data);
> +            Ok(())
> +        })
> +    }
> +}
> +
> +#[cfg(CONFIG_DEBUG_FS)]
> +impl<T> Scope<T> {
> +    fn entry_mut(self: Pin<&mut Self>) -> &mut Entry {
> +        // SAFETY: _entry is not structurally pinned

Please end with a period.

> +        unsafe { &mut Pin::into_inner_unchecked(self)._entry }
> +    }

Missing empty line.

> +    fn new<'b, E: 'b, TI: PinInit<T, E> + 'b, F: for<'a> FnOnce(&'a T) -> Entry + 'b>(
> +        data: TI,
> +        init: F,
> +    ) -> impl PinInit<Self, E> + 'b
> +    where
> +        T: 'b,
> +    {
> +        try_pin_init! {
> +            Self {
> +                _entry: Entry::empty(),
> +                data <- data,
> +                _pin: PhantomPinned
> +            } ? E
> +        }
> +        .pin_chain(|scope| {
> +            *scope.entry_mut() = init(&scope.data);
> +            Ok(())
> +        })
> +    }
> +}

<snip>

> +/// # Invariant
> +///
> +/// `FileOps<T>` will always contain an `operations` which is safe to use for a file backed
> +/// off an inode which has a pointer to a `T` in its private data that is safe to convert
> +/// into a reference.
> +pub(super) struct FileOps<T> {
> +    #[cfg(CONFIG_DEBUG_FS)]
> +    operations: bindings::file_operations,
> +    #[cfg(CONFIG_DEBUG_FS)]
> +    mode: u16,
> +    _phantom: PhantomData<T>,
> +}
> +
> +impl<T> FileOps<T> {
> +    /// # Safety

Missing empty line.

> +    /// The caller asserts that the provided `operations` is safe to use for a file whose
> +    /// inode has a pointer to `T` in its private data that is safe to convert into a reference.
> +    const unsafe fn new(operations: bindings::file_operations, mode: u16) -> Self {
> +        Self {
> +            #[cfg(CONFIG_DEBUG_FS)]
> +            operations,
> +            #[cfg(CONFIG_DEBUG_FS)]
> +            mode,
> +            _phantom: PhantomData,
> +        }
> +    }

Same here...

> +    #[cfg(CONFIG_DEBUG_FS)]
> +    pub(crate) const fn mode(&self) -> u16 {
> +        self.mode
> +    }
> +}
> +
> +#[cfg(CONFIG_DEBUG_FS)]
> +impl<T> Deref for FileOps<T> {
> +    type Target = bindings::file_operations;

...and here.

> +    fn deref(&self) -> &Self::Target {
> +        &self.operations
> +    }
> +}
> +
> +struct RenderAdapter<T>(T);
> +
> +impl<'a, T: Render> Display for RenderAdapter<&'a T> {
> +    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
> +        self.0.render(f)
> +    }
> +}
> +
> +/// 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.
> +unsafe extern "C" fn render_open<T: Render + Sync>(
> +    inode: *mut bindings::inode,
> +    file: *mut bindings::file,
> +) -> c_int {
> +    // SAFETY: The caller ensures that `inode` is a valid pointer.
> +    let data = unsafe { (*inode).i_private };
> +    // 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(render_act::<T>), data) }
> +}
> +
> +/// 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

We usually say "valid".

> +/// not have any unique references alias it during the call.
> +unsafe extern "C" fn render_act<T: Render + 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.cast::<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, "{}", RenderAdapter(data));
> +    0
> +}
> +
> +// Work around lack of generic const items.
> +pub(crate) trait ReadFile<T> {
> +    const FILE_OPS: FileOps<T>;
> +}
> +
> +impl<T: Render + Sync> ReadFile<T> for T {
> +    const FILE_OPS: FileOps<T> = {
> +        let operations = bindings::file_operations {
> +            read: Some(bindings::seq_read),
> +            llseek: Some(bindings::seq_lseek),
> +            release: Some(bindings::single_release),
> +            open: Some(render_open::<Self>),
> +            // SAFETY: `file_operations` supports zeroes in all fields.
> +            ..unsafe { core::mem::zeroed() }
> +        };
> +        // SAFETY: `operations` is all stock `seq_file` implementations except for `render_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 matches the
> +        // `FileOps` requirements.
> +        unsafe { FileOps::new(operations, 0o400) }
> +    };
> +}
> diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..2939e18e3dda39571cd7255505e5f605f0e3d154
> --- /dev/null
> +++ b/rust/kernel/debugfs/traits.rs
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2025 Google LLC.
> +
> +//! Traits for rendering or updating values exported to DebugFS.
> +
> +use crate::sync::Mutex;
> +use core::fmt::{self, Debug, Formatter};
> +
> +/// A trait for types that can be rendered into a string.
> +///
> +/// This works very similarly to `Debug`, and is automatically implemented if `Debug` is
> +/// implemented for a type. It is also implemented for any renderable type inside a `Mutex`.

I think the blanket impls make sense, but we should probably add a note that the
contents derived via #[derive(Debug)] are not guaranteed to be stable [1].

[1] https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability

> +pub trait Render {

Should we really call this Render? Maybe it'd be better to call it
debugfs::Writer?

Yes, I know it's called in read(), but that's what the kernel does, it writes
on read(). :)

> +    /// Formats the value using the given formatter.
> +    fn render(&self, f: &mut Formatter<'_>) -> fmt::Result;
> +}
> +
> +impl<T: Render> Render for Mutex<T> {
> +    fn render(&self, f: &mut Formatter<'_>) -> fmt::Result {
> +        self.lock().render(f)
> +    }
> +}
> +
> +impl<T: Debug> Render for T {
> +    fn render(&self, f: &mut Formatter<'_>) -> fmt::Result {
> +        writeln!(f, "{self:?}")
> +    }
> +}

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

* Re: [PATCH v10 3/7] rust: debugfs: Add support for writable files
  2025-08-19 22:53 ` [PATCH v10 3/7] rust: debugfs: Add support for writable files Matthew Maurer
@ 2025-08-26 19:38   ` Danilo Krummrich
  0 siblings, 0 replies; 17+ messages in thread
From: Danilo Krummrich @ 2025-08-26 19:38 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, Dirk Beheme, linux-kernel, rust-for-linux

On Wed Aug 20, 2025 at 12:53 AM CEST, Matthew Maurer wrote:
> +    /// Creates a read-write file in this directory.
> +    ///
> +    /// Reading the file uses the [`Render`] implementation.
> +    /// Writing to the file uses the [`UpdateFromSlice`] implementation.
> +    pub fn read_write_file<
> +        'a,
> +        T: Render + UpdateFromSlice + Send + Sync + 'static,
> +        E: 'a,
> +        TI: PinInit<T, E> + 'a,

Same comments as in the previous patch, I think using a where clause is a bit
cleaner, even though with this formatting it'd be fine too, but this is not
guaranteed.

> +    >(
> +        &'a self,
> +        name: &'a CStr,
> +        data: TI,

impl PinInit<T, E> + 'a

> +    ) -> impl PinInit<File<T>, E> + 'a {
> +        let file_ops = &<T as ReadWriteFile<_>>::FILE_OPS;
> +        self.create_file(name, data, file_ops)
> +    }

> +fn update<T: UpdateFromSlice + Sync>(data: &T, buf: *const c_char, count: usize) -> isize {
> +    let mut reader = UserSlice::new(UserPtr::from_ptr(buf as *mut c_void), count).reader();

This naming is pretty close to what I was about to propose for the UpdateFromSlice
trait. :)

Given that I proposed debugfs::Writer instead of debugfs::Render, I think we
should just rename debugfs::UpdateFromSlice to debugfs::Reader.

> +
> +    if let Err(e) = data.update_from_slice(&mut reader) {
> +        return e.to_errno() as isize;
> +    }
> +
> +    count as isize
> +}

<snip>

> +/// # Safety
> +///
> +/// `inode` must be a valid pointer to an `inode` struct.
> +/// `file` must be a valid pointer to a `file` struct.
> +unsafe extern "C" fn write_only_open(
> +    inode: *mut bindings::inode,
> +    file: *mut bindings::file,
> +) -> c_int {
> +    // SAFETY: The caller ensures that `inode` and `file` are valid pointers.
> +    unsafe {
> +        (*file).private_data = (*inode).i_private;
> +    }

NIT: If you move the semicolon at the end of the unsafe block it goes into a
single line.

> +    0
> +}

<snip>

> +impl<T: UpdateFromSlice + Sync> WriteFile<T> for T {
> +    const FILE_OPS: FileOps<T> = {
> +        let operations = bindings::file_operations {
> +            open: Some(write_only_open),
> +            write: Some(write_only_write::<T>),
> +            llseek: Some(bindings::noop_llseek),
> +            // SAFETY: `file_operations` supports zeroes in all fields.
> +            ..unsafe { core::mem::zeroed() }
> +        };
> +        // SAFETY:
> +        // * `write_only_open` populates the file private data with the inode private data
> +        // * `write_only_write`'s only requirement is that the private data of the file point to
> +        //   a `T` and be legal to convert to a shared reference, which `write_only_open`
> +        //   satisfies.
> +        unsafe { FileOps::new(operations, 0o200) }

I think it'd be nice to have an abstraction for file modes, but this can be done
separately.

> +    };
> +}
> diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
> index 2939e18e3dda39571cd7255505e5f605f0e3d154..d64638898faaa1a6a9898c374b8c1114993376c9 100644
> --- a/rust/kernel/debugfs/traits.rs
> +++ b/rust/kernel/debugfs/traits.rs
> @@ -3,8 +3,15 @@
>  
>  //! Traits for rendering or updating values exported to DebugFS.
>  
> +use crate::prelude::*;
>  use crate::sync::Mutex;
> +use crate::uaccess::UserSliceReader;
>  use core::fmt::{self, Debug, Formatter};
> +use core::str::FromStr;
> +use core::sync::atomic::{
> +    AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicU16, AtomicU32, AtomicU64,
> +    AtomicU8, AtomicUsize, Ordering,
> +};
>  
>  /// A trait for types that can be rendered into a string.
>  ///
> @@ -26,3 +33,65 @@ fn render(&self, f: &mut Formatter<'_>) -> fmt::Result {
>          writeln!(f, "{self:?}")
>      }
>  }
> +
> +/// A trait for types that can be updated from a user slice.
> +///
> +/// This works similarly to `FromStr`, but operates on a `UserSliceReader` rather than a &str.
> +///
> +/// It is automatically implemented for all atomic integers, or any type that implements `FromStr`
> +/// wrapped in a `Mutex`.
> +pub trait UpdateFromSlice {

As mentioned above, I think we should name this Reader and the Render thing
Writer.

> +    /// Updates the value from the given user slice.
> +    fn update_from_slice(&self, reader: &mut UserSliceReader) -> Result<()>;

read_from_slice()?

> +}
> +
> +impl<T: FromStr> UpdateFromSlice for Mutex<T> {
> +    fn update_from_slice(&self, reader: &mut UserSliceReader) -> Result<()> {
> +        let mut buf = [0u8; 128];
> +        if reader.len() > buf.len() {
> +            return Err(EINVAL);
> +        }
> +        let n = reader.len();
> +        reader.read_slice(&mut buf[..n])?;
> +
> +        let s = core::str::from_utf8(&buf[..n]).map_err(|_| EINVAL)?;
> +        let val = s.trim().parse::<T>().map_err(|_| EINVAL)?;
> +        *self.lock() = val;
> +        Ok(())
> +    }
> +}
> +
> +macro_rules! impl_update_from_slice_for_atomic {
> +    ($(($atomic_type:ty, $int_type:ty)),*) => {
> +        $(
> +            impl UpdateFromSlice for $atomic_type {
> +                fn update_from_slice(&self, reader: &mut UserSliceReader) -> Result<()> {
> +                    let mut buf = [0u8; 21]; // Enough for a 64-bit number.
> +                    if reader.len() > buf.len() {
> +                        return Err(EINVAL);
> +                    }
> +                    let n = reader.len();
> +                    reader.read_slice(&mut buf[..n])?;
> +
> +                    let s = core::str::from_utf8(&buf[..n]).map_err(|_| EINVAL)?;
> +                    let val = s.trim().parse::<$int_type>().map_err(|_| EINVAL)?;
> +                    self.store(val, Ordering::Relaxed);
> +                    Ok(())
> +                }
> +            }
> +        )*
> +    };
> +}
> +
> +impl_update_from_slice_for_atomic!(
> +    (AtomicI16, i16),
> +    (AtomicI32, i32),
> +    (AtomicI64, i64),
> +    (AtomicI8, i8),
> +    (AtomicIsize, isize),
> +    (AtomicU16, u16),
> +    (AtomicU32, u32),
> +    (AtomicU64, u64),
> +    (AtomicU8, u8),
> +    (AtomicUsize, usize)
> +);
>
> -- 
> 2.51.0.rc1.167.g924127e9c0-goog


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

end of thread, other threads:[~2025-08-26 19:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-19 22:53 [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
2025-08-19 22:53 ` [PATCH v10 1/7] rust: debugfs: Add initial support for directories Matthew Maurer
2025-08-26 15:39   ` Danilo Krummrich
2025-08-19 22:53 ` [PATCH v10 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
2025-08-26 18:45   ` Danilo Krummrich
2025-08-19 22:53 ` [PATCH v10 3/7] rust: debugfs: Add support for writable files Matthew Maurer
2025-08-26 19:38   ` Danilo Krummrich
2025-08-19 22:53 ` [PATCH v10 4/7] rust: debugfs: Add support for callback-based files Matthew Maurer
2025-08-19 22:53 ` [PATCH v10 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
2025-08-20  0:34   ` Danilo Krummrich
2025-08-20  0:40     ` Matthew Maurer
2025-08-20  0:42       ` Matthew Maurer
2025-08-20  7:46       ` Benno Lossin
2025-08-19 22:53 ` [PATCH v10 6/7] rust: debugfs: Add support for scoped directories Matthew Maurer
2025-08-19 22:53 ` [PATCH v10 7/7] samples: rust: Add scoped debugfs sample driver Matthew Maurer
2025-08-19 23:14 ` [PATCH v10 0/7] rust: DebugFS Bindings Matthew Maurer
2025-08-25 11:51 ` 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).