* [PATCH v11 1/7] rust: debugfs: Add initial support for directories
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
@ 2025-09-04 21:13 ` Matthew Maurer
2025-09-04 21:13 ` [PATCH v11 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
` (6 subsequent siblings)
7 siblings, 0 replies; 38+ messages in thread
From: Matthew Maurer @ 2025-09-04 21:13 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..65be71600b8eda83c0d313f3d205d0713e8e9510
--- /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.
+///
+/// The directory in the filesystem represented by [`Dir`] will be removed when handle has been
+/// dropped *and* all children have been removed.
+// If we have a parent, we hold a reference to it in the `Entry`. This prevents the `dentry`
+// we point to from being cleaned up if our parent `Dir`/`Entry` is dropped before us.
+//
+// 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.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
2025-09-04 21:13 ` [PATCH v11 1/7] rust: debugfs: Add initial support for directories Matthew Maurer
@ 2025-09-04 21:13 ` Matthew Maurer
2025-09-08 10:17 ` Greg Kroah-Hartman
2025-09-09 7:29 ` Dirk Behme
2025-09-04 21:13 ` [PATCH v11 3/7] rust: debugfs: Add support for writable files Matthew Maurer
` (5 subsequent siblings)
7 siblings, 2 replies; 38+ messages in thread
From: Matthew Maurer @ 2025-09-04 21:13 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 `Writer` trait.
The file's content is generated by the `Writer` 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 | 148 +++++++++++++++++++++++++++++++++++++++-
rust/kernel/debugfs/entry.rs | 42 ++++++++++++
rust/kernel/debugfs/file_ops.rs | 128 ++++++++++++++++++++++++++++++++++
rust/kernel/debugfs/traits.rs | 33 +++++++++
4 files changed, 350 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 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::Writer;
+mod file_ops;
+use file_ops::{FileOps, ReadFile};
#[cfg(CONFIG_DEBUG_FS)]
mod entry;
#[cfg(CONFIG_DEBUG_FS)]
@@ -53,6 +59,34 @@ 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, E: 'a>(
+ &'a self,
+ name: &'a CStr,
+ data: impl PinInit<T, E> + 'a,
+ file_ops: &'static FileOps<T>,
+ ) -> impl PinInit<File<T>, E> + 'a
+ where
+ T: Sync + 'static,
+ {
+ 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 +113,116 @@ 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 [`Writer::write`] 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, E: 'a>(
+ &'a self,
+ name: &'a CStr,
+ data: impl PinInit<T, E> + 'a,
+ ) -> impl PinInit<File<T>, E> + 'a
+ where
+ T: Writer + Send + Sync + 'static,
+ {
+ let file_ops = &<T as ReadFile<_>>::FILE_OPS;
+ self.create_file(name, data, file_ops)
+ }
+}
+
+#[pin_data]
+/// Handle to a DebugFS scope, which ensures that attached `data` will outlive the provided
+/// [`Entry`] without moving.
+/// Currently, this is used to back [`File`] so that its `read` and/or `write` implementations
+/// can assume that their backing data is still alive.
+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<'b, T: 'b> Scope<T> {
+ fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+ where
+ F: for<'a> FnOnce(&'a T) -> Entry + 'b,
+ {
+ try_pin_init! {
+ Self {
+ data <- data,
+ _pin: PhantomPinned
+ } ? E
+ }
+ .pin_chain(|scope| {
+ init(&scope.data);
+ Ok(())
+ })
+ }
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+impl<'b, T: 'b> 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<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+ where
+ F: for<'a> FnOnce(&'a T) -> Entry + '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..c2fbef96580eaa2fab7cc8c1ba559c3284d12e1b
--- /dev/null
+++ b/rust/kernel/debugfs/file_ops.rs
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+use super::Writer;
+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 WriterAdapter<T>(T);
+
+impl<'a, T: Writer> Display for WriterAdapter<&'a T> {
+ fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+ self.0.write(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 writer_open<T: Writer + 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(writer_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 valid pointer to a `T` which may
+/// not have any unique references alias it during the call.
+unsafe extern "C" fn writer_act<T: Writer + Sync>(
+ seq: *mut bindings::seq_file,
+ _: *mut c_void,
+) -> c_int {
+ // SAFETY: By caller precondition, this pointer is valid pointer to a `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, "{}", WriterAdapter(data));
+ 0
+}
+
+// Work around lack of generic const items.
+pub(crate) trait ReadFile<T> {
+ const FILE_OPS: FileOps<T>;
+}
+
+impl<T: Writer + 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(writer_open::<Self>),
+ // SAFETY: `file_operations` supports zeroes in all fields.
+ ..unsafe { core::mem::zeroed() }
+ };
+ // SAFETY: `operations` is all stock `seq_file` implementations except for `writer_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..0e6e461324de42a3d80b692264d50e78a48f561d
--- /dev/null
+++ b/rust/kernel/debugfs/traits.rs
@@ -0,0 +1,33 @@
+// 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 written 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 writable type inside a `Mutex`.
+///
+/// The derived implementation of `Debug` [may
+/// change](https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability)
+/// between Rust versions, so if stability is key for your use case, please implement `Writer`
+/// explicitly instead.
+pub trait Writer {
+ /// Formats the value using the given formatter.
+ fn write(&self, f: &mut Formatter<'_>) -> fmt::Result;
+}
+
+impl<T: Writer> Writer for Mutex<T> {
+ fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ self.lock().write(f)
+ }
+}
+
+impl<T: Debug> Writer for T {
+ fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ writeln!(f, "{self:?}")
+ }
+}
--
2.51.0.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-04 21:13 ` [PATCH v11 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
@ 2025-09-08 10:17 ` Greg Kroah-Hartman
2025-09-08 10:54 ` Danilo Krummrich
2025-09-09 7:29 ` Dirk Behme
1 sibling, 1 reply; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-08 10:17 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,
Danilo Krummrich, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, Dirk Beheme, linux-kernel, rust-for-linux
On Thu, Sep 04, 2025 at 09:13:53PM +0000, Matthew Maurer wrote:
> 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 `Writer` trait.
>
> The file's content is generated by the `Writer` 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 | 148 +++++++++++++++++++++++++++++++++++++++-
> rust/kernel/debugfs/entry.rs | 42 ++++++++++++
> rust/kernel/debugfs/file_ops.rs | 128 ++++++++++++++++++++++++++++++++++
> rust/kernel/debugfs/traits.rs | 33 +++++++++
> 4 files changed, 350 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 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::Writer;
>
> +mod file_ops;
> +use file_ops::{FileOps, ReadFile};
> #[cfg(CONFIG_DEBUG_FS)]
> mod entry;
> #[cfg(CONFIG_DEBUG_FS)]
> @@ -53,6 +59,34 @@ 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, E: 'a>(
> + &'a self,
> + name: &'a CStr,
> + data: impl PinInit<T, E> + 'a,
> + file_ops: &'static FileOps<T>,
> + ) -> impl PinInit<File<T>, E> + 'a
> + where
> + T: Sync + 'static,
> + {
> + 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 +113,116 @@ 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 [`Writer::write`] 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, E: 'a>(
> + &'a self,
> + name: &'a CStr,
> + data: impl PinInit<T, E> + 'a,
> + ) -> impl PinInit<File<T>, E> + 'a
> + where
> + T: Writer + Send + Sync + 'static,
> + {
> + let file_ops = &<T as ReadFile<_>>::FILE_OPS;
> + self.create_file(name, data, file_ops)
> + }
> +}
> +
> +#[pin_data]
> +/// Handle to a DebugFS scope, which ensures that attached `data` will outlive the provided
> +/// [`Entry`] without moving.
> +/// Currently, this is used to back [`File`] so that its `read` and/or `write` implementations
> +/// can assume that their backing data is still alive.
> +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<'b, T: 'b> Scope<T> {
> + fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
> + where
> + F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> + {
> + try_pin_init! {
> + Self {
> + data <- data,
> + _pin: PhantomPinned
> + } ? E
> + }
> + .pin_chain(|scope| {
> + init(&scope.data);
> + Ok(())
> + })
> + }
> +}
> +
> +#[cfg(CONFIG_DEBUG_FS)]
> +impl<'b, T: 'b> 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<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
> + where
> + F: for<'a> FnOnce(&'a T) -> Entry + '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..c2fbef96580eaa2fab7cc8c1ba559c3284d12e1b
> --- /dev/null
> +++ b/rust/kernel/debugfs/file_ops.rs
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2025 Google LLC.
> +
> +use super::Writer;
> +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 WriterAdapter<T>(T);
> +
> +impl<'a, T: Writer> Display for WriterAdapter<&'a T> {
> + fn fmt(&self, f: &mut Formatter<'_>) -> Result {
> + self.0.write(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 writer_open<T: Writer + 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(writer_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 valid pointer to a `T` which may
> +/// not have any unique references alias it during the call.
> +unsafe extern "C" fn writer_act<T: Writer + Sync>(
> + seq: *mut bindings::seq_file,
> + _: *mut c_void,
> +) -> c_int {
> + // SAFETY: By caller precondition, this pointer is valid pointer to a `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, "{}", WriterAdapter(data));
> + 0
> +}
> +
> +// Work around lack of generic const items.
> +pub(crate) trait ReadFile<T> {
> + const FILE_OPS: FileOps<T>;
> +}
> +
> +impl<T: Writer + 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(writer_open::<Self>),
> + // SAFETY: `file_operations` supports zeroes in all fields.
> + ..unsafe { core::mem::zeroed() }
> + };
> + // SAFETY: `operations` is all stock `seq_file` implementations except for `writer_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..0e6e461324de42a3d80b692264d50e78a48f561d
> --- /dev/null
> +++ b/rust/kernel/debugfs/traits.rs
> @@ -0,0 +1,33 @@
> +// 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 written 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 writable type inside a `Mutex`.
> +///
> +/// The derived implementation of `Debug` [may
> +/// change](https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability)
> +/// between Rust versions, so if stability is key for your use case, please implement `Writer`
> +/// explicitly instead.
> +pub trait Writer {
> + /// Formats the value using the given formatter.
> + fn write(&self, f: &mut Formatter<'_>) -> fmt::Result;
> +}
> +
> +impl<T: Writer> Writer for Mutex<T> {
> + fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
> + self.lock().write(f)
> + }
> +}
> +
> +impl<T: Debug> Writer for T {
> + fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
> + writeln!(f, "{self:?}")
> + }
> +}
I tried using this in a "tiny" test module I had written, and I get the
following build error:
--> samples/rust/rust_debugfs2.rs:64:53
|
64 | _file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
| -------------- ^^^^^^^^^^^^^^^^^ expected `&u32`, found `&&CStr`
| |
| arguments to this method are incorrect
|
= note: expected reference `&u32`
found reference `&&'static kernel::prelude::CStr`
I'm trying to "just" print a CStr, which is defined as:
struct HwSocInfo {
id: u32,
ver: u32,
raw_id: u32,
foundry: u32,
name: &'static CStr,
}
Is this just a "user is holding it wrong" error on my side, or can this api not
handle CStr values?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 10:17 ` Greg Kroah-Hartman
@ 2025-09-08 10:54 ` Danilo Krummrich
2025-09-08 10:56 ` Danilo Krummrich
2025-09-08 12:48 ` Greg Kroah-Hartman
0 siblings, 2 replies; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 10:54 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 12:17 PM CEST, Greg Kroah-Hartman wrote:
> I tried using this in a "tiny" test module I had written, and I get the
> following build error:
>
> --> samples/rust/rust_debugfs2.rs:64:53
> |
> 64 | _file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
> | -------------- ^^^^^^^^^^^^^^^^^ expected `&u32`, found `&&CStr`
> | |
> | arguments to this method are incorrect
> |
> = note: expected reference `&u32`
> found reference `&&'static kernel::prelude::CStr`
>
> I'm trying to "just" print a CStr, which is defined as:
>
> struct HwSocInfo {
> id: u32,
> ver: u32,
> raw_id: u32,
> foundry: u32,
> name: &'static CStr,
> }
>
> Is this just a "user is holding it wrong" error on my side, or can this api not
> handle CStr values?
What you're doing should fundamentally work.
The above error suggests that your declaration of `_file` is File<&u32> rather
than File<&'static CStr>.
Also note the double reference you create with `&hw_soc_info.name`, this should
just be `hw_soc_info.name`.
You can also test this case by applying the following diff the the sample in v5:
diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
index b26eea3ee723..475502f30b1a 100644
--- a/samples/rust/rust_debugfs.rs
+++ b/samples/rust/rust_debugfs.rs
@@ -59,6 +59,8 @@ struct RustDebugFs {
#[pin]
_compatible: File<CString>,
#[pin]
+ _test: File<&'static CStr>,
+ #[pin]
counter: File<AtomicUsize>,
#[pin]
inner: File<Mutex<Inner>>,
@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
.property_read::<CString>(c_str!("compatible"))
.required_by(dev)?,
),
+ _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
counter <- Self::build_counter(&debugfs),
inner <- Self::build_inner(&debugfs),
_debugfs: debugfs,
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 10:54 ` Danilo Krummrich
@ 2025-09-08 10:56 ` Danilo Krummrich
2025-09-08 12:48 ` Greg Kroah-Hartman
1 sibling, 0 replies; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 10:56 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 12:54 PM CEST, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 12:17 PM CEST, Greg Kroah-Hartman wrote:
>> I tried using this in a "tiny" test module I had written, and I get the
>> following build error:
>>
>> --> samples/rust/rust_debugfs2.rs:64:53
>> |
>> 64 | _file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
>> | -------------- ^^^^^^^^^^^^^^^^^ expected `&u32`, found `&&CStr`
>> | |
>> | arguments to this method are incorrect
>> |
>> = note: expected reference `&u32`
>> found reference `&&'static kernel::prelude::CStr`
>>
>> I'm trying to "just" print a CStr, which is defined as:
>>
>> struct HwSocInfo {
>> id: u32,
>> ver: u32,
>> raw_id: u32,
>> foundry: u32,
>> name: &'static CStr,
>> }
>>
>> Is this just a "user is holding it wrong" error on my side, or can this api not
>> handle CStr values?
>
> What you're doing should fundamentally work.
>
> The above error suggests that your declaration of `_file` is File<&u32> rather
> than File<&'static CStr>.
>
> Also note the double reference you create with `&hw_soc_info.name`, this should
> just be `hw_soc_info.name`.
>
> You can also test this case by applying the following diff the the sample in v5:
*sample in patch 5
>
> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> index b26eea3ee723..475502f30b1a 100644
> --- a/samples/rust/rust_debugfs.rs
> +++ b/samples/rust/rust_debugfs.rs
> @@ -59,6 +59,8 @@ struct RustDebugFs {
> #[pin]
> _compatible: File<CString>,
> #[pin]
> + _test: File<&'static CStr>,
> + #[pin]
> counter: File<AtomicUsize>,
> #[pin]
> inner: File<Mutex<Inner>>,
> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
> .property_read::<CString>(c_str!("compatible"))
> .required_by(dev)?,
> ),
> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
> counter <- Self::build_counter(&debugfs),
> inner <- Self::build_inner(&debugfs),
> _debugfs: debugfs,
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 10:54 ` Danilo Krummrich
2025-09-08 10:56 ` Danilo Krummrich
@ 2025-09-08 12:48 ` Greg Kroah-Hartman
2025-09-08 13:22 ` Danilo Krummrich
1 sibling, 1 reply; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-08 12:48 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 12:17 PM CEST, Greg Kroah-Hartman wrote:
> > I tried using this in a "tiny" test module I had written, and I get the
> > following build error:
> >
> > --> samples/rust/rust_debugfs2.rs:64:53
> > |
> > 64 | _file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
> > | -------------- ^^^^^^^^^^^^^^^^^ expected `&u32`, found `&&CStr`
> > | |
> > | arguments to this method are incorrect
> > |
> > = note: expected reference `&u32`
> > found reference `&&'static kernel::prelude::CStr`
> >
> > I'm trying to "just" print a CStr, which is defined as:
> >
> > struct HwSocInfo {
> > id: u32,
> > ver: u32,
> > raw_id: u32,
> > foundry: u32,
> > name: &'static CStr,
> > }
> >
> > Is this just a "user is holding it wrong" error on my side, or can this api not
> > handle CStr values?
>
> What you're doing should fundamentally work.
>
> The above error suggests that your declaration of `_file` is File<&u32> rather
> than File<&'static CStr>.
Ah, ick, I missed that the return type would be different here. Yes, I
was doing a bunch of file creation calls:
let mut _file = root.read_only_file(c_str!("id"), &hw_soc_info.id);
_file = root.read_only_file(c_str!("ver"), &hw_soc_info.ver);
_file = root.read_only_file(c_str!("raw_id"), &hw_soc_info.raw_id);
_file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
As I don't care about the return value here at all.
But really, I should just write this as:
root.read_only_file(c_str!("id"), &hw_soc_info.id);
root.read_only_file(c_str!("ver"), &hw_soc_info.ver);
root.read_only_file(c_str!("raw_id"), &hw_soc_info.raw_id);
root.read_only_file(c_str!("name"), hw_soc_info.name);
with, as you point out:
> Also note the double reference you create with `&hw_soc_info.name`, this should
> just be `hw_soc_info.name`.
Yes, sorry, my fault there.
> You can also test this case by applying the following diff the the sample in v5:
>
> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> index b26eea3ee723..475502f30b1a 100644
> --- a/samples/rust/rust_debugfs.rs
> +++ b/samples/rust/rust_debugfs.rs
> @@ -59,6 +59,8 @@ struct RustDebugFs {
> #[pin]
> _compatible: File<CString>,
> #[pin]
> + _test: File<&'static CStr>,
> + #[pin]
> counter: File<AtomicUsize>,
> #[pin]
> inner: File<Mutex<Inner>>,
> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
> .property_read::<CString>(c_str!("compatible"))
> .required_by(dev)?,
> ),
> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
Cool, but again, we do not want to ever be storing individual debugfs
files. Well, we can, but for 90% of the cases, we do not, we only want
to remove the whole directory when that goes out of scope, which will
clean up the files then.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 12:48 ` Greg Kroah-Hartman
@ 2025-09-08 13:22 ` Danilo Krummrich
2025-09-08 13:30 ` Greg Kroah-Hartman
0 siblings, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 13:22 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
>> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
>> index b26eea3ee723..475502f30b1a 100644
>> --- a/samples/rust/rust_debugfs.rs
>> +++ b/samples/rust/rust_debugfs.rs
>> @@ -59,6 +59,8 @@ struct RustDebugFs {
>> #[pin]
>> _compatible: File<CString>,
>> #[pin]
>> + _test: File<&'static CStr>,
>> + #[pin]
>> counter: File<AtomicUsize>,
>> #[pin]
>> inner: File<Mutex<Inner>>,
>> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
>> .property_read::<CString>(c_str!("compatible"))
>> .required_by(dev)?,
>> ),
>> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
>
> Cool, but again, we do not want to ever be storing individual debugfs
> files. Well, we can, but for 90% of the cases, we do not, we only want
> to remove the whole directory when that goes out of scope, which will
> clean up the files then.
This API does not work in the way that you have a struct storing the data you
want to expose *and* another one for the files with the data attached.
The File type contains the actual data. For instance, if you have a struct Foo,
where you want to expose the members through debugfs you would *not* do:
struct Foo {
a: u32,
b: u32,
}
struct FooFiles {
a: File<&u32>,
b: File<&u32>
}
and then create an instance of Foo *and* another instance of FooFiles to export
them via debugfs.
Instead you would change your struct Foo to just be:
struct Foo {
a: File<u32>,
b: File<u32>,
}
If you now create an instance of Foo (let's call it `foo`), then foo.a or foo.b
dereferences to the inner type, i.e. the u32. Or in other words `foo` still
behaves as if `a` and `b` would be u32 values. For instance:
if foo.a == 42 {
pr_info!("Foo::b = {}\n", foo.b);
}
The fact that the backing files of `a` and `b` are removed from debugfs when Foo
is dropped is necessary since otherwise we create a UAF.
Think of File<T> as a containers like you think of KBox<T>.
KBox<T> behaves exactly like T, but silently manages the backing kmalloc()
allocation that T lives in.
With File<T> it's exactly the same, it behaves exactly like the T that lives
within File<T>, but silently manages the debugfs file the T is exposed by.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 13:22 ` Danilo Krummrich
@ 2025-09-08 13:30 ` Greg Kroah-Hartman
2025-09-08 13:34 ` Alice Ryhl
2025-09-08 13:36 ` Danilo Krummrich
0 siblings, 2 replies; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-08 13:30 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> >> index b26eea3ee723..475502f30b1a 100644
> >> --- a/samples/rust/rust_debugfs.rs
> >> +++ b/samples/rust/rust_debugfs.rs
> >> @@ -59,6 +59,8 @@ struct RustDebugFs {
> >> #[pin]
> >> _compatible: File<CString>,
> >> #[pin]
> >> + _test: File<&'static CStr>,
> >> + #[pin]
> >> counter: File<AtomicUsize>,
> >> #[pin]
> >> inner: File<Mutex<Inner>>,
> >> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
> >> .property_read::<CString>(c_str!("compatible"))
> >> .required_by(dev)?,
> >> ),
> >> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
> >
> > Cool, but again, we do not want to ever be storing individual debugfs
> > files. Well, we can, but for 90% of the cases, we do not, we only want
> > to remove the whole directory when that goes out of scope, which will
> > clean up the files then.
>
> This API does not work in the way that you have a struct storing the data you
> want to expose *and* another one for the files with the data attached.
>
> The File type contains the actual data. For instance, if you have a struct Foo,
> where you want to expose the members through debugfs you would *not* do:
>
> struct Foo {
> a: u32,
> b: u32,
> }
>
> struct FooFiles {
> a: File<&u32>,
> b: File<&u32>
> }
>
> and then create an instance of Foo *and* another instance of FooFiles to export
> them via debugfs.
Ah, that's exactly what I was trying to do.
> Instead you would change your struct Foo to just be:
>
> struct Foo {
> a: File<u32>,
> b: File<u32>,
> }
>
> If you now create an instance of Foo (let's call it `foo`), then foo.a or foo.b
> dereferences to the inner type, i.e. the u32. Or in other words `foo` still
> behaves as if `a` and `b` would be u32 values. For instance:
>
> if foo.a == 42 {
> pr_info!("Foo::b = {}\n", foo.b);
> }
Oh that's not going to work well at all :(
Think about something "simple" like a pci config descriptor. You have a
structure, with fields, already sitting there. You want to expose those
fields in debugfs. So you want to only create debugfs files in one
location in a driver, you don't want ALL users of those fields to have
to go through a File<T> api, right? That would be crazy, all drivers
would end up always having File<T> everywhere.
> The fact that the backing files of `a` and `b` are removed from debugfs when Foo
> is dropped is necessary since otherwise we create a UAF.
That's fine, but:
> Think of File<T> as a containers like you think of KBox<T>.
Ok, but again, you are now forcing all users to think of debugfs as the
main "interface" to those variables, which is not true (nor should it
be.)
> KBox<T> behaves exactly like T, but silently manages the backing kmalloc()
> allocation that T lives in.
>
> With File<T> it's exactly the same, it behaves exactly like the T that lives
> within File<T>, but silently manages the debugfs file the T is exposed by.
And what happens if debugfs is not enabled? What about if creating the
file fails? The variable still needs to be present and active and
working.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 13:30 ` Greg Kroah-Hartman
@ 2025-09-08 13:34 ` Alice Ryhl
2025-09-08 13:38 ` Danilo Krummrich
2025-09-08 13:36 ` Danilo Krummrich
1 sibling, 1 reply; 38+ messages in thread
From: Alice Ryhl @ 2025-09-08 13:34 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Danilo Krummrich, Matthew Maurer, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Trevor Gross, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, Dirk Beheme, linux-kernel, rust-for-linux
On Mon, Sep 8, 2025 at 3:30 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> > On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> > > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> > >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> > >> index b26eea3ee723..475502f30b1a 100644
> > >> --- a/samples/rust/rust_debugfs.rs
> > >> +++ b/samples/rust/rust_debugfs.rs
> > >> @@ -59,6 +59,8 @@ struct RustDebugFs {
> > >> #[pin]
> > >> _compatible: File<CString>,
> > >> #[pin]
> > >> + _test: File<&'static CStr>,
> > >> + #[pin]
> > >> counter: File<AtomicUsize>,
> > >> #[pin]
> > >> inner: File<Mutex<Inner>>,
> > >> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
> > >> .property_read::<CString>(c_str!("compatible"))
> > >> .required_by(dev)?,
> > >> ),
> > >> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
> > >
> > > Cool, but again, we do not want to ever be storing individual debugfs
> > > files. Well, we can, but for 90% of the cases, we do not, we only want
> > > to remove the whole directory when that goes out of scope, which will
> > > clean up the files then.
> >
> > This API does not work in the way that you have a struct storing the data you
> > want to expose *and* another one for the files with the data attached.
> >
> > The File type contains the actual data. For instance, if you have a struct Foo,
> > where you want to expose the members through debugfs you would *not* do:
> >
> > struct Foo {
> > a: u32,
> > b: u32,
> > }
> >
> > struct FooFiles {
> > a: File<&u32>,
> > b: File<&u32>
> > }
> >
> > and then create an instance of Foo *and* another instance of FooFiles to export
> > them via debugfs.
>
> Ah, that's exactly what I was trying to do.
>
> > Instead you would change your struct Foo to just be:
> >
> > struct Foo {
> > a: File<u32>,
> > b: File<u32>,
> > }
> >
> > If you now create an instance of Foo (let's call it `foo`), then foo.a or foo.b
> > dereferences to the inner type, i.e. the u32. Or in other words `foo` still
> > behaves as if `a` and `b` would be u32 values. For instance:
> >
> > if foo.a == 42 {
> > pr_info!("Foo::b = {}\n", foo.b);
> > }
>
> Oh that's not going to work well at all :(
>
> Think about something "simple" like a pci config descriptor. You have a
> structure, with fields, already sitting there. You want to expose those
> fields in debugfs. So you want to only create debugfs files in one
> location in a driver, you don't want ALL users of those fields to have
> to go through a File<T> api, right? That would be crazy, all drivers
> would end up always having File<T> everywhere.
>
> > The fact that the backing files of `a` and `b` are removed from debugfs when Foo
> > is dropped is necessary since otherwise we create a UAF.
>
> That's fine, but:
>
> > Think of File<T> as a containers like you think of KBox<T>.
>
> Ok, but again, you are now forcing all users to think of debugfs as the
> main "interface" to those variables, which is not true (nor should it
> be.)
All of these things is why I recommended to Matthew that he should add
back the scoped API, since it doesn't have all these drawbacks.
Alice
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 13:34 ` Alice Ryhl
@ 2025-09-08 13:38 ` Danilo Krummrich
0 siblings, 0 replies; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 13:38 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Matthew Maurer, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Trevor Gross, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 3:34 PM CEST, Alice Ryhl wrote:
> All of these things is why I recommended to Matthew that he should add
> back the scoped API, since it doesn't have all these drawbacks.
Yeah, I think it's the correct solution to make those cases nicer.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 13:30 ` Greg Kroah-Hartman
2025-09-08 13:34 ` Alice Ryhl
@ 2025-09-08 13:36 ` Danilo Krummrich
2025-09-08 14:16 ` Greg Kroah-Hartman
1 sibling, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 13:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
>> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
>> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
>> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
>> >> index b26eea3ee723..475502f30b1a 100644
>> >> --- a/samples/rust/rust_debugfs.rs
>> >> +++ b/samples/rust/rust_debugfs.rs
>> >> @@ -59,6 +59,8 @@ struct RustDebugFs {
>> >> #[pin]
>> >> _compatible: File<CString>,
>> >> #[pin]
>> >> + _test: File<&'static CStr>,
>> >> + #[pin]
>> >> counter: File<AtomicUsize>,
>> >> #[pin]
>> >> inner: File<Mutex<Inner>>,
>> >> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
>> >> .property_read::<CString>(c_str!("compatible"))
>> >> .required_by(dev)?,
>> >> ),
>> >> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
>> >
>> > Cool, but again, we do not want to ever be storing individual debugfs
>> > files. Well, we can, but for 90% of the cases, we do not, we only want
>> > to remove the whole directory when that goes out of scope, which will
>> > clean up the files then.
>>
>> This API does not work in the way that you have a struct storing the data you
>> want to expose *and* another one for the files with the data attached.
>>
>> The File type contains the actual data. For instance, if you have a struct Foo,
>> where you want to expose the members through debugfs you would *not* do:
>>
>> struct Foo {
>> a: u32,
>> b: u32,
>> }
>>
>> struct FooFiles {
>> a: File<&u32>,
>> b: File<&u32>
>> }
>>
>> and then create an instance of Foo *and* another instance of FooFiles to export
>> them via debugfs.
>
> Ah, that's exactly what I was trying to do.
But that's bad, then we're back at the lifetime problem from the beginning,
because the File<&Foo> then somehow needs to ensure that the instance Foo
remains alive as long as File<&Foo> or the backing directory exists.
So, you eventually end of with Foo needing to be reference counted with its own
memory allocation, which horribly messes with your lifetimes in the driver.
You don't want a a field to be reference counted just because it's exposed via
debugfs.
>> Instead you would change your struct Foo to just be:
>>
>> struct Foo {
>> a: File<u32>,
>> b: File<u32>,
>> }
>>
>> If you now create an instance of Foo (let's call it `foo`), then foo.a or foo.b
>> dereferences to the inner type, i.e. the u32. Or in other words `foo` still
>> behaves as if `a` and `b` would be u32 values. For instance:
>>
>> if foo.a == 42 {
>> pr_info!("Foo::b = {}\n", foo.b);
>> }
>
> Oh that's not going to work well at all :(
>
> Think about something "simple" like a pci config descriptor. You have a
> structure, with fields, already sitting there. You want to expose those
> fields in debugfs.
This is more of a special case that is addressed by the Scope API in patch 6 and
patch 7, so we should be good.
>> The fact that the backing files of `a` and `b` are removed from debugfs when Foo
>> is dropped is necessary since otherwise we create a UAF.
>
> That's fine, but:
>
>> Think of File<T> as a containers like you think of KBox<T>.
>
> Ok, but again, you are now forcing all users to think of debugfs as the
> main "interface" to those variables, which is not true (nor should it
> be.)
>
>> KBox<T> behaves exactly like T, but silently manages the backing kmalloc()
>> allocation that T lives in.
>>
>> With File<T> it's exactly the same, it behaves exactly like the T that lives
>> within File<T>, but silently manages the debugfs file the T is exposed by.
>
> And what happens if debugfs is not enabled? What about if creating the
> file fails? The variable still needs to be present and active and
> working.
This is the case, the variable will still be present and active in any case.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 13:36 ` Danilo Krummrich
@ 2025-09-08 14:16 ` Greg Kroah-Hartman
2025-09-08 14:59 ` Danilo Krummrich
0 siblings, 1 reply; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-08 14:16 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon, Sep 08, 2025 at 03:36:46PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
> > On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> >> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> >> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> >> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> >> >> index b26eea3ee723..475502f30b1a 100644
> >> >> --- a/samples/rust/rust_debugfs.rs
> >> >> +++ b/samples/rust/rust_debugfs.rs
> >> >> @@ -59,6 +59,8 @@ struct RustDebugFs {
> >> >> #[pin]
> >> >> _compatible: File<CString>,
> >> >> #[pin]
> >> >> + _test: File<&'static CStr>,
> >> >> + #[pin]
> >> >> counter: File<AtomicUsize>,
> >> >> #[pin]
> >> >> inner: File<Mutex<Inner>>,
> >> >> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
> >> >> .property_read::<CString>(c_str!("compatible"))
> >> >> .required_by(dev)?,
> >> >> ),
> >> >> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
> >> >
> >> > Cool, but again, we do not want to ever be storing individual debugfs
> >> > files. Well, we can, but for 90% of the cases, we do not, we only want
> >> > to remove the whole directory when that goes out of scope, which will
> >> > clean up the files then.
> >>
> >> This API does not work in the way that you have a struct storing the data you
> >> want to expose *and* another one for the files with the data attached.
> >>
> >> The File type contains the actual data. For instance, if you have a struct Foo,
> >> where you want to expose the members through debugfs you would *not* do:
> >>
> >> struct Foo {
> >> a: u32,
> >> b: u32,
> >> }
> >>
> >> struct FooFiles {
> >> a: File<&u32>,
> >> b: File<&u32>
> >> }
> >>
> >> and then create an instance of Foo *and* another instance of FooFiles to export
> >> them via debugfs.
> >
> > Ah, that's exactly what I was trying to do.
>
> But that's bad, then we're back at the lifetime problem from the beginning,
> because the File<&Foo> then somehow needs to ensure that the instance Foo
> remains alive as long as File<&Foo> or the backing directory exists.
>
> So, you eventually end of with Foo needing to be reference counted with its own
> memory allocation, which horribly messes with your lifetimes in the driver.
Once I want to drop Foo, FooFiles should "go out of scope" and be gone.
If a backing file descriptor is still held open, it will then become
"stale" and not work. Much like the revokable stuff works.
Note, none of this is in the C code today, and debugfs is bound to root
permissions, so it's not really an issue, but I can understand the goal
of correctness...
Anyway, I looked at the scoped example here, and I don't see how that
works any differently. How can I use it to have a single Dir "handle"
that when goes out of scope, can drop the files attached to it that were
created to reference Foo.a and Foo.b in your example above?
> You don't want a a field to be reference counted just because it's exposed via
> debugfs.
Exactly, the data is the thing driving this, not the debugfs file.
> >> Instead you would change your struct Foo to just be:
> >>
> >> struct Foo {
> >> a: File<u32>,
> >> b: File<u32>,
> >> }
> >>
> >> If you now create an instance of Foo (let's call it `foo`), then foo.a or foo.b
> >> dereferences to the inner type, i.e. the u32. Or in other words `foo` still
> >> behaves as if `a` and `b` would be u32 values. For instance:
> >>
> >> if foo.a == 42 {
> >> pr_info!("Foo::b = {}\n", foo.b);
> >> }
> >
> > Oh that's not going to work well at all :(
> >
> > Think about something "simple" like a pci config descriptor. You have a
> > structure, with fields, already sitting there. You want to expose those
> > fields in debugfs.
>
> This is more of a special case that is addressed by the Scope API in patch 6 and
> patch 7, so we should be good.
See above for my lack of understanding of that :)
> > And what happens if debugfs is not enabled? What about if creating the
> > file fails? The variable still needs to be present and active and
> > working.
>
> This is the case, the variable will still be present and active in any case.
Ugh, but really, that's very unworkable overall. While I see the logic
here, it's making the debugfs interface be the "main" one, when really
that is just an afterthought and is NOT the thing to focus on at all.
Again, debugfs is just "on the side for debugging", let's not force it
to be the way that data is accessed within the kernel itself, like is
being done with the wrapping of File<T> here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 14:16 ` Greg Kroah-Hartman
@ 2025-09-08 14:59 ` Danilo Krummrich
2025-09-08 16:19 ` Greg Kroah-Hartman
0 siblings, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 14:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 4:16 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 03:36:46PM +0200, Danilo Krummrich wrote:
>> On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
>> > On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
>> >> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
>> >> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
>> >> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
>> >> >> index b26eea3ee723..475502f30b1a 100644
>> >> >> --- a/samples/rust/rust_debugfs.rs
>> >> >> +++ b/samples/rust/rust_debugfs.rs
>> >> >> @@ -59,6 +59,8 @@ struct RustDebugFs {
>> >> >> #[pin]
>> >> >> _compatible: File<CString>,
>> >> >> #[pin]
>> >> >> + _test: File<&'static CStr>,
>> >> >> + #[pin]
>> >> >> counter: File<AtomicUsize>,
>> >> >> #[pin]
>> >> >> inner: File<Mutex<Inner>>,
>> >> >> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
>> >> >> .property_read::<CString>(c_str!("compatible"))
>> >> >> .required_by(dev)?,
>> >> >> ),
>> >> >> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
>> >> >
>> >> > Cool, but again, we do not want to ever be storing individual debugfs
>> >> > files. Well, we can, but for 90% of the cases, we do not, we only want
>> >> > to remove the whole directory when that goes out of scope, which will
>> >> > clean up the files then.
>> >>
>> >> This API does not work in the way that you have a struct storing the data you
>> >> want to expose *and* another one for the files with the data attached.
>> >>
>> >> The File type contains the actual data. For instance, if you have a struct Foo,
>> >> where you want to expose the members through debugfs you would *not* do:
>> >>
>> >> struct Foo {
>> >> a: u32,
>> >> b: u32,
>> >> }
>> >>
>> >> struct FooFiles {
>> >> a: File<&u32>,
>> >> b: File<&u32>
>> >> }
>> >>
>> >> and then create an instance of Foo *and* another instance of FooFiles to export
>> >> them via debugfs.
>> >
>> > Ah, that's exactly what I was trying to do.
>>
>> But that's bad, then we're back at the lifetime problem from the beginning,
>> because the File<&Foo> then somehow needs to ensure that the instance Foo
>> remains alive as long as File<&Foo> or the backing directory exists.
>>
>> So, you eventually end of with Foo needing to be reference counted with its own
>> memory allocation, which horribly messes with your lifetimes in the driver.
>
> Once I want to drop Foo, FooFiles should "go out of scope" and be gone.
We agree on the goal here, but unfortunately it's not really possible. There are
two options that were already exercised:
(1) Force that FooFiles (or FooDir) is bound to the lifetime of a
reference of Foo with FooDir<&'a Foo>.
This isn't workable because we then can't store both of them into
the same parent structure.
(2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count
of Foo.
But this is bad for the mentioned reasons. :(
(3) The File<T> API we have now, which gives you the behavior you ask
for with Scope<T>.
Where Scope<T> creates a directory and owns the data you pass to it,
e.g. a pci config descriptor.
The user can create an arbitrary number of files exporting any of
the fields in date that live in the scope and don't need to be tracked
separately, i.e. don't create separate object instances.
The directory (and hence all the files) is removed once the Scope<T>
is dropped, including the data it owns.
> If a backing file descriptor is still held open, it will then become
> "stale" and not work. Much like the revokable stuff works.
>
> Note, none of this is in the C code today, and debugfs is bound to root
> permissions, so it's not really an issue, but I can understand the goal
> of correctness...
The lifetime guarantee we talk about is about the debugfs file still having a
pointer to data that has already been dropped / freed.
In C you have to remove the debugfs file or directly (and hence the file) before
the data exposed through it is freed. In C this is on the driver to take care
of.
(If in C a driver has multiple structures exported in the same debugfs directory
it has to manually take care of keeping all structures alive as long as the
directory (and hence all files) exist.)
In Rust we need the abstraction to guarantee this.
> Anyway, I looked at the scoped example here, and I don't see how that
> works any differently. How can I use it to have a single Dir "handle"
> that when goes out of scope, can drop the files attached to it that were
> created to reference Foo.a and Foo.b in your example above?
In the example above you would move Foo into the Scope<Foo>. For instance:
let dir = root_dir.scope(foo, cstr!("subdir"), |foo, dir| {
dir.read_only_file(c_str!("a"), foo.a);
dir.read_only_file(c_str!("b"), foo.b);
});
Note that those methods don't return anything, they're automatically bound to
the Scope in lifetime.
So, Foo could be your pci config descriptor.
If `dir` is dropped, everything dies, the Scope, the "subdir" directory, all the
files and also Foo.
I can provide some working code later on (currently in a meeting). :)
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 14:59 ` Danilo Krummrich
@ 2025-09-08 16:19 ` Greg Kroah-Hartman
2025-09-08 16:30 ` Danilo Krummrich
2025-09-08 17:58 ` Danilo Krummrich
0 siblings, 2 replies; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-08 16:19 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 5888 bytes --]
On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 4:16 PM CEST, Greg Kroah-Hartman wrote:
> > On Mon, Sep 08, 2025 at 03:36:46PM +0200, Danilo Krummrich wrote:
> >> On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
> >> > On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> >> >> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> >> >> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> >> >> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> >> >> >> index b26eea3ee723..475502f30b1a 100644
> >> >> >> --- a/samples/rust/rust_debugfs.rs
> >> >> >> +++ b/samples/rust/rust_debugfs.rs
> >> >> >> @@ -59,6 +59,8 @@ struct RustDebugFs {
> >> >> >> #[pin]
> >> >> >> _compatible: File<CString>,
> >> >> >> #[pin]
> >> >> >> + _test: File<&'static CStr>,
> >> >> >> + #[pin]
> >> >> >> counter: File<AtomicUsize>,
> >> >> >> #[pin]
> >> >> >> inner: File<Mutex<Inner>>,
> >> >> >> @@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
> >> >> >> .property_read::<CString>(c_str!("compatible"))
> >> >> >> .required_by(dev)?,
> >> >> >> ),
> >> >> >> + _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
> >> >> >
> >> >> > Cool, but again, we do not want to ever be storing individual debugfs
> >> >> > files. Well, we can, but for 90% of the cases, we do not, we only want
> >> >> > to remove the whole directory when that goes out of scope, which will
> >> >> > clean up the files then.
> >> >>
> >> >> This API does not work in the way that you have a struct storing the data you
> >> >> want to expose *and* another one for the files with the data attached.
> >> >>
> >> >> The File type contains the actual data. For instance, if you have a struct Foo,
> >> >> where you want to expose the members through debugfs you would *not* do:
> >> >>
> >> >> struct Foo {
> >> >> a: u32,
> >> >> b: u32,
> >> >> }
> >> >>
> >> >> struct FooFiles {
> >> >> a: File<&u32>,
> >> >> b: File<&u32>
> >> >> }
> >> >>
> >> >> and then create an instance of Foo *and* another instance of FooFiles to export
> >> >> them via debugfs.
> >> >
> >> > Ah, that's exactly what I was trying to do.
> >>
> >> But that's bad, then we're back at the lifetime problem from the beginning,
> >> because the File<&Foo> then somehow needs to ensure that the instance Foo
> >> remains alive as long as File<&Foo> or the backing directory exists.
> >>
> >> So, you eventually end of with Foo needing to be reference counted with its own
> >> memory allocation, which horribly messes with your lifetimes in the driver.
> >
> > Once I want to drop Foo, FooFiles should "go out of scope" and be gone.
>
> We agree on the goal here, but unfortunately it's not really possible. There are
> two options that were already exercised:
>
> (1) Force that FooFiles (or FooDir) is bound to the lifetime of a
> reference of Foo with FooDir<&'a Foo>.
>
> This isn't workable because we then can't store both of them into
> the same parent structure.
>
> (2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count
> of Foo.
>
> But this is bad for the mentioned reasons. :(
>
> (3) The File<T> API we have now, which gives you the behavior you ask
> for with Scope<T>.
>
> Where Scope<T> creates a directory and owns the data you pass to it,
> e.g. a pci config descriptor.
>
> The user can create an arbitrary number of files exporting any of
> the fields in date that live in the scope and don't need to be tracked
> separately, i.e. don't create separate object instances.
>
> The directory (and hence all the files) is removed once the Scope<T>
> is dropped, including the data it owns.
>
> > If a backing file descriptor is still held open, it will then become
> > "stale" and not work. Much like the revokable stuff works.
> >
> > Note, none of this is in the C code today, and debugfs is bound to root
> > permissions, so it's not really an issue, but I can understand the goal
> > of correctness...
>
> The lifetime guarantee we talk about is about the debugfs file still having a
> pointer to data that has already been dropped / freed.
>
> In C you have to remove the debugfs file or directly (and hence the file) before
> the data exposed through it is freed. In C this is on the driver to take care
> of.
>
> (If in C a driver has multiple structures exported in the same debugfs directory
> it has to manually take care of keeping all structures alive as long as the
> directory (and hence all files) exist.)
>
> In Rust we need the abstraction to guarantee this.
>
> > Anyway, I looked at the scoped example here, and I don't see how that
> > works any differently. How can I use it to have a single Dir "handle"
> > that when goes out of scope, can drop the files attached to it that were
> > created to reference Foo.a and Foo.b in your example above?
>
> In the example above you would move Foo into the Scope<Foo>. For instance:
>
> let dir = root_dir.scope(foo, cstr!("subdir"), |foo, dir| {
> dir.read_only_file(c_str!("a"), foo.a);
> dir.read_only_file(c_str!("b"), foo.b);
> });
>
> Note that those methods don't return anything, they're automatically bound to
> the Scope in lifetime.
>
> So, Foo could be your pci config descriptor.
>
> If `dir` is dropped, everything dies, the Scope, the "subdir" directory, all the
> files and also Foo.
>
> I can provide some working code later on (currently in a meeting). :)
Working code for the simple "foo" example will be good. Here's my
horrible (and will not build) example I was trying to get to work.
thanks,
greg k-h
[-- Attachment #2: rust_debugfs2.rs --]
[-- Type: application/rls-services+xml, Size: 1924 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 16:19 ` Greg Kroah-Hartman
@ 2025-09-08 16:30 ` Danilo Krummrich
2025-09-08 16:55 ` Danilo Krummrich
2025-09-08 17:58 ` Danilo Krummrich
1 sibling, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 16:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 6:19 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:
<snip>
>> We agree on the goal here, but unfortunately it's not really possible. There are
>> two options that were already exercised:
>>
>> (1) Force that FooFiles (or FooDir) is bound to the lifetime of a
>> reference of Foo with FooDir<&'a Foo>.
>>
>> This isn't workable because we then can't store both of them into
>> the same parent structure.
>>
>> (2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count
>> of Foo.
>>
>> But this is bad for the mentioned reasons. :(
>>
>> (3) The File<T> API we have now, which gives you the behavior you ask
>> for with Scope<T>.
>>
>> Where Scope<T> creates a directory and owns the data you pass to it,
>> e.g. a pci config descriptor.
>>
>> The user can create an arbitrary number of files exporting any of
>> the fields in date that live in the scope and don't need to be tracked
>> separately, i.e. don't create separate object instances.
>>
>> The directory (and hence all the files) is removed once the Scope<T>
>> is dropped, including the data it owns.
<snip>
>> I can provide some working code later on (currently in a meeting). :)
>
> Working code for the simple "foo" example will be good. Here's my
> horrible (and will not build) example I was trying to get to work.
Here it comes [1]. :)
[1] rust_debugfs_soc_info.rs
// SPDX-License-Identifier: GPL-2.0
//! Simple `debugfs::Scope` example.
use kernel::c_str;
use kernel::debugfs::{Dir, Scope};
use kernel::prelude::*;
module! {
type: MyModule,
name: "MyModule",
description: "Just a simple test module.",
license: "GPL",
}
#[derive(Debug)]
struct HwSocInfo {
name: &'static CStr,
ver: u32,
id: u32,
}
impl HwSocInfo {
fn new(name: &'static CStr, ver: u32, id: u32) -> Self {
Self { name, ver, id }
}
}
struct MyModule {
// Dropped when MyModule is released (e.g. through `rmmod`).
//
// This will drop the inner `HwSocInfo`, the "foo" directory, and all files created within this
// directory.
_scope: Pin<KBox<Scope<HwSocInfo>>>,
}
impl kernel::Module for MyModule {
fn init(_module: &'static kernel::ThisModule) -> Result<Self, Error> {
let root_dir = Dir::new(c_str!("my_module"));
// Obtain some `HwSocInfo`, could from anywhere.
let soc_info = HwSocInfo::new(c_str!("foo"), 24, 42);
let scope = KBox::pin_init(
// Create directory scope, that contains some data and a bunch of files exporting this
// data.
root_dir.scope(soc_info, c_str!("hw_soc_info"), |soc_info, dir| {
dir.read_only_file(c_str!("name"), &soc_info.name);
dir.read_only_file(c_str!("ver"), &soc_info.ver);
dir.read_only_file(c_str!("id"), &soc_info.id);
}),
GFP_KERNEL,
)?;
// Print the contents of `soc_info` that were moved into `scope`.
pr_info!("HwSocInfo: {:?}\n", &**scope);
Ok(Self { _scope: scope })
}
}
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 16:30 ` Danilo Krummrich
@ 2025-09-08 16:55 ` Danilo Krummrich
2025-09-10 15:21 ` Greg Kroah-Hartman
0 siblings, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 16:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 6:30 PM CEST, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 6:19 PM CEST, Greg Kroah-Hartman wrote:
>> On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:
>
> <snip>
>
>>> We agree on the goal here, but unfortunately it's not really possible. There are
>>> two options that were already exercised:
>>>
>>> (1) Force that FooFiles (or FooDir) is bound to the lifetime of a
>>> reference of Foo with FooDir<&'a Foo>.
>>>
>>> This isn't workable because we then can't store both of them into
>>> the same parent structure.
>>>
>>> (2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count
>>> of Foo.
>>>
>>> But this is bad for the mentioned reasons. :(
>>>
>>> (3) The File<T> API we have now, which gives you the behavior you ask
>>> for with Scope<T>.
>>>
>>> Where Scope<T> creates a directory and owns the data you pass to it,
>>> e.g. a pci config descriptor.
>>>
>>> The user can create an arbitrary number of files exporting any of
>>> the fields in date that live in the scope and don't need to be tracked
>>> separately, i.e. don't create separate object instances.
>>>
>>> The directory (and hence all the files) is removed once the Scope<T>
>>> is dropped, including the data it owns.
>
> <snip>
>
>>> I can provide some working code later on (currently in a meeting). :)
>>
>> Working code for the simple "foo" example will be good. Here's my
>> horrible (and will not build) example I was trying to get to work.
>
> Here it comes [1]. :)
>
> [1] rust_debugfs_soc_info.rs
>
> // SPDX-License-Identifier: GPL-2.0
>
> //! Simple `debugfs::Scope` example.
>
> use kernel::c_str;
> use kernel::debugfs::{Dir, Scope};
> use kernel::prelude::*;
>
> module! {
> type: MyModule,
> name: "MyModule",
> description: "Just a simple test module.",
> license: "GPL",
> }
>
> #[derive(Debug)]
> struct HwSocInfo {
> name: &'static CStr,
> ver: u32,
> id: u32,
> }
>
> impl HwSocInfo {
> fn new(name: &'static CStr, ver: u32, id: u32) -> Self {
> Self { name, ver, id }
> }
> }
>
> struct MyModule {
> // Dropped when MyModule is released (e.g. through `rmmod`).
> //
> // This will drop the inner `HwSocInfo`, the "foo" directory, and all files created within this
> // directory.
> _scope: Pin<KBox<Scope<HwSocInfo>>>,
And yes, I get that HwSocInfo now lives within a debugfs structure, just like
with
struct Data {
version: File<u32>,
}
but those become transparent wrappers if debugfs is disabled, i.e. zero
overhead. They also won't make the driver fail if anything with debugfs goes
south if enabled.
And I also understand your point that now they're part of a "real" data
structure. But in the end, debugfs *is* part of the driver. And while we should
ensure that it doesn't impact drivers as much as possible (which we do), I don't
think that we necessarily have to hide the fact entirely.
Having that said, I also don't really see an alternative. If we really want
debugfs structures to be entirely separate we would have to either
(1) reference count fields exposed through debugfs, or
(2) make the interface unsafe, use raw pointers and assert that a debugfs file
never out-lives the data it exposes, just like in C.
As I mentioned previously, while File<T> is visible in driver structures only,
(1) even enforces drivers to break their lifetime patterns, which is much worse
and not acceptable I think.
I would even say that (2) is better than (1), because it becomes safe when
debugfs is disabled. Yet I think both (1) and (2) are much worse that what we
have right now.
And I think the Scope API helps a lot in keeping things reasonable for cases
where a lot of fields of a single structure should be exposed separately, such
as the one you sketched up.
> }
>
> impl kernel::Module for MyModule {
> fn init(_module: &'static kernel::ThisModule) -> Result<Self, Error> {
> let root_dir = Dir::new(c_str!("my_module"));
>
> // Obtain some `HwSocInfo`, could from anywhere.
> let soc_info = HwSocInfo::new(c_str!("foo"), 24, 42);
>
> let scope = KBox::pin_init(
> // Create directory scope, that contains some data and a bunch of files exporting this
> // data.
> root_dir.scope(soc_info, c_str!("hw_soc_info"), |soc_info, dir| {
> dir.read_only_file(c_str!("name"), &soc_info.name);
> dir.read_only_file(c_str!("ver"), &soc_info.ver);
> dir.read_only_file(c_str!("id"), &soc_info.id);
> }),
> GFP_KERNEL,
> )?;
>
> // Print the contents of `soc_info` that were moved into `scope`.
> pr_info!("HwSocInfo: {:?}\n", &**scope);
>
> Ok(Self { _scope: scope })
> }
> }
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 16:55 ` Danilo Krummrich
@ 2025-09-10 15:21 ` Greg Kroah-Hartman
0 siblings, 0 replies; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-10 15:21 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon, Sep 08, 2025 at 06:55:48PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 6:30 PM CEST, Danilo Krummrich wrote:
> > On Mon Sep 8, 2025 at 6:19 PM CEST, Greg Kroah-Hartman wrote:
> >> On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:
> >
> > <snip>
> >
> >>> We agree on the goal here, but unfortunately it's not really possible. There are
> >>> two options that were already exercised:
> >>>
> >>> (1) Force that FooFiles (or FooDir) is bound to the lifetime of a
> >>> reference of Foo with FooDir<&'a Foo>.
> >>>
> >>> This isn't workable because we then can't store both of them into
> >>> the same parent structure.
> >>>
> >>> (2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count
> >>> of Foo.
> >>>
> >>> But this is bad for the mentioned reasons. :(
> >>>
> >>> (3) The File<T> API we have now, which gives you the behavior you ask
> >>> for with Scope<T>.
> >>>
> >>> Where Scope<T> creates a directory and owns the data you pass to it,
> >>> e.g. a pci config descriptor.
> >>>
> >>> The user can create an arbitrary number of files exporting any of
> >>> the fields in date that live in the scope and don't need to be tracked
> >>> separately, i.e. don't create separate object instances.
> >>>
> >>> The directory (and hence all the files) is removed once the Scope<T>
> >>> is dropped, including the data it owns.
> >
> > <snip>
> >
> >>> I can provide some working code later on (currently in a meeting). :)
> >>
> >> Working code for the simple "foo" example will be good. Here's my
> >> horrible (and will not build) example I was trying to get to work.
> >
> > Here it comes [1]. :)
> >
> > [1] rust_debugfs_soc_info.rs
> >
> > // SPDX-License-Identifier: GPL-2.0
> >
> > //! Simple `debugfs::Scope` example.
> >
> > use kernel::c_str;
> > use kernel::debugfs::{Dir, Scope};
> > use kernel::prelude::*;
> >
> > module! {
> > type: MyModule,
> > name: "MyModule",
> > description: "Just a simple test module.",
> > license: "GPL",
> > }
> >
> > #[derive(Debug)]
> > struct HwSocInfo {
> > name: &'static CStr,
> > ver: u32,
> > id: u32,
> > }
> >
> > impl HwSocInfo {
> > fn new(name: &'static CStr, ver: u32, id: u32) -> Self {
> > Self { name, ver, id }
> > }
> > }
> >
> > struct MyModule {
> > // Dropped when MyModule is released (e.g. through `rmmod`).
> > //
> > // This will drop the inner `HwSocInfo`, the "foo" directory, and all files created within this
> > // directory.
> > _scope: Pin<KBox<Scope<HwSocInfo>>>,
>
> And yes, I get that HwSocInfo now lives within a debugfs structure, just like
> with
>
> struct Data {
> version: File<u32>,
> }
>
> but those become transparent wrappers if debugfs is disabled, i.e. zero
> overhead. They also won't make the driver fail if anything with debugfs goes
> south if enabled.
>
> And I also understand your point that now they're part of a "real" data
> structure. But in the end, debugfs *is* part of the driver. And while we should
> ensure that it doesn't impact drivers as much as possible (which we do), I don't
> think that we necessarily have to hide the fact entirely.
>
> Having that said, I also don't really see an alternative. If we really want
> debugfs structures to be entirely separate we would have to either
>
> (1) reference count fields exposed through debugfs, or
>
> (2) make the interface unsafe, use raw pointers and assert that a debugfs file
> never out-lives the data it exposes, just like in C.
>
> As I mentioned previously, while File<T> is visible in driver structures only,
> (1) even enforces drivers to break their lifetime patterns, which is much worse
> and not acceptable I think.
>
> I would even say that (2) is better than (1), because it becomes safe when
> debugfs is disabled. Yet I think both (1) and (2) are much worse that what we
> have right now.
>
> And I think the Scope API helps a lot in keeping things reasonable for cases
> where a lot of fields of a single structure should be exposed separately, such
> as the one you sketched up.
Thanks for the example (and the rewrite of my example.)
This makes more sense, but I'm worried that this puts debugfs "front and
center" for data structures that normally had nothing to do with debugfs
at all.
So I took at look at the USB and PCI drivers to see what they do as
those are "real-world" users of debugfs, not just "soc info" stuff, and
it turns out that you are right. The use of debugfs is normally NOT
using just simple wrappers around variables like our examples are doing,
but are explicit "read/write" type things that do stuff to a structure,
AND those structures need to be properly referenced, so that things do
not go wrong when structures are removed.
So, after digging through this some more today, I think I'm ok with it.
Let's merge this now, and let's see how it gets used and if it's
unwieldy, hey, we can always change it!
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-08 16:19 ` Greg Kroah-Hartman
2025-09-08 16:30 ` Danilo Krummrich
@ 2025-09-08 17:58 ` Danilo Krummrich
1 sibling, 0 replies; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 17:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 261 bytes --]
On 9/8/25 6:19 PM, Greg Kroah-Hartman wrote:
> Working code for the simple "foo" example will be good. Here's my
> horrible (and will not build) example I was trying to get to work.
I think our examples were pretty close already, but here's also your file. :)
[-- Attachment #2: rust_debugfs2.rs --]
[-- Type: text/rust, Size: 2418 bytes --]
// SPDX-License-Identifier: GPL-2.0
// Copyright (C) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org>
// Copyright (C) 2025 The Linux Foundation
//! Sample debugfs rust module that emulates soc_info to try to see just how well the api can
//! work...
use core::fmt;
use kernel::c_str;
use kernel::debugfs::{Dir, Scope};
use kernel::prelude::*;
module! {
type: SocInfo,
name: "rust_soc_info",
authors: ["Greg Kroah-Hartman"],
description: "Rust soc_info sample driver",
license: "GPL",
}
fn foundry_print(foundry: &u32, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "Foundry: {}", foundry)
}
// Fake "hardware SOC info object that ideally would read from the hardware to get the info.
// For now just use some fake data
#[derive(Debug)]
struct HwSocInfo {
id: u32,
ver: u32,
raw_id: u32,
foundry: u32,
name: &'static CStr,
}
impl HwSocInfo {
fn new() -> Self {
Self {
id: 123,
ver: 456,
raw_id: 789,
foundry: 0,
name: c_str!("hw_soc name"),
}
}
}
struct SocInfo {
_debug_dir: Dir,
_hw_soc_info: Pin<KBox<Scope<HwSocInfo>>>,
}
impl kernel::Module for SocInfo {
fn init(_this: &'static ThisModule) -> Result<Self> {
// Read from the hardware and get our structure information
let hw_soc_info = HwSocInfo::new();
// Create the root directory
let root = Dir::new(c_str!("rust_soc_info"));
let scope = KBox::pin_init(
// Create directory scope, that contains some data and a bunch of files exporting this
// data.
root.scope(hw_soc_info, c_str!("hw_soc_info"), |hw_soc_info, dir| {
dir.read_only_file(c_str!("id"), &hw_soc_info.id);
dir.read_only_file(c_str!("ver"), &hw_soc_info.ver);
dir.read_only_file(c_str!("raw_id"), &hw_soc_info.raw_id);
dir.read_only_file(c_str!("name"), &hw_soc_info.name);
dir.read_callback_file(c_str!("foundry"), &hw_soc_info.foundry, &foundry_print);
}),
GFP_KERNEL,
)?;
let soc_info: &HwSocInfo = &scope;
// Print the contents of `soc_info` that were moved into `scope`.
pr_info!("HwSocInfo: {:?}\n", soc_info);
Ok(Self {
_debug_dir: root,
_hw_soc_info: scope,
})
}
}
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-04 21:13 ` [PATCH v11 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
2025-09-08 10:17 ` Greg Kroah-Hartman
@ 2025-09-09 7:29 ` Dirk Behme
2025-09-09 8:29 ` Danilo Krummrich
1 sibling, 1 reply; 38+ messages in thread
From: Dirk Behme @ 2025-09-09 7:29 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 04/09/2025 23:13, Matthew Maurer wrote:
> 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 `Writer` trait.
>
> The file's content is generated by the `Writer` 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 | 148 +++++++++++++++++++++++++++++++++++++++-
> rust/kernel/debugfs/entry.rs | 42 ++++++++++++
> rust/kernel/debugfs/file_ops.rs | 128 ++++++++++++++++++++++++++++++++++
> rust/kernel/debugfs/traits.rs | 33 +++++++++
> 4 files changed, 350 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 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::Writer;
>
> +mod file_ops;
> +use file_ops::{FileOps, ReadFile};
> #[cfg(CONFIG_DEBUG_FS)]
> mod entry;
> #[cfg(CONFIG_DEBUG_FS)]
> @@ -53,6 +59,34 @@ 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, E: 'a>(
> + &'a self,
> + name: &'a CStr,
> + data: impl PinInit<T, E> + 'a,
> + file_ops: &'static FileOps<T>,
> + ) -> impl PinInit<File<T>, E> + 'a
> + where
> + T: Sync + 'static,
> + {
> + 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 +113,116 @@ 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 [`Writer::write`] 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, E: 'a>(
> + &'a self,
> + name: &'a CStr,
> + data: impl PinInit<T, E> + 'a,
> + ) -> impl PinInit<File<T>, E> + 'a
> + where
> + T: Writer + Send + Sync + 'static,
> + {
> + let file_ops = &<T as ReadFile<_>>::FILE_OPS;
> + self.create_file(name, data, file_ops)
> + }
> +}
> +
> +#[pin_data]
> +/// Handle to a DebugFS scope, which ensures that attached `data` will outlive the provided
> +/// [`Entry`] without moving.
> +/// Currently, this is used to back [`File`] so that its `read` and/or `write` implementations
> +/// can assume that their backing data is still alive.
> +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<'b, T: 'b> Scope<T> {
> + fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
> + where
> + F: for<'a> FnOnce(&'a T) -> Entry + 'b,
Inspired by Greg's & Danilo's discussion I tried building with
CONFIG_DEBUG_FS disabled. And get
error[E0412]: cannot find type `Entry` in this scope
--> rust/kernel/debugfs.rs:351:37
|
351 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
| ^^^^^ not found in this scope
And giving it some Entry (for my 1.81.0)
error: hidden lifetime parameters in types are deprecated
--> rust/kernel/debugfs.rs:352:37
|
352 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
| ^^^^^ expected lifetime parameter
Dirk
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-09 7:29 ` Dirk Behme
@ 2025-09-09 8:29 ` Danilo Krummrich
2025-09-10 15:22 ` Greg Kroah-Hartman
0 siblings, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-09 8:29 UTC (permalink / raw)
To: Dirk Behme
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Greg Kroah-Hartman, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, linux-kernel, rust-for-linux
On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
> On 04/09/2025 23:13, Matthew Maurer wrote:
>> +#[cfg(not(CONFIG_DEBUG_FS))]
>> +impl<'b, T: 'b> Scope<T> {
>> + fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
>> + where
>> + F: for<'a> FnOnce(&'a T) -> Entry + 'b,
>
> Inspired by Greg's & Danilo's discussion I tried building with
> CONFIG_DEBUG_FS disabled. And get
>
> error[E0412]: cannot find type `Entry` in this scope
> --> rust/kernel/debugfs.rs:351:37
> |
> 351 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> | ^^^^^ not found in this scope
>
> And giving it some Entry (for my 1.81.0)
>
> error: hidden lifetime parameters in types are deprecated
> --> rust/kernel/debugfs.rs:352:37
> |
> 352 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> | ^^^^^ expected lifetime parameter
Yeah, I caught this as well and fixed it up on my end with the following diff:
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index ecfcce845d3f..1f25777743db 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -348,7 +348,7 @@ pub struct File<T> {
impl<'b, T: 'b> Scope<T> {
fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
where
- F: for<'a> FnOnce(&'a T) -> Entry + 'b,
+ F: for<'a> FnOnce(&'a T) + 'b,
{
try_pin_init! {
Self {
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-09 8:29 ` Danilo Krummrich
@ 2025-09-10 15:22 ` Greg Kroah-Hartman
2025-09-10 15:23 ` Danilo Krummrich
0 siblings, 1 reply; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-10 15:22 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Dirk Behme, Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, linux-kernel, rust-for-linux
On Tue, Sep 09, 2025 at 10:29:13AM +0200, Danilo Krummrich wrote:
> On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
> > On 04/09/2025 23:13, Matthew Maurer wrote:
> >> +#[cfg(not(CONFIG_DEBUG_FS))]
> >> +impl<'b, T: 'b> Scope<T> {
> >> + fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
> >> + where
> >> + F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> >
> > Inspired by Greg's & Danilo's discussion I tried building with
> > CONFIG_DEBUG_FS disabled. And get
> >
> > error[E0412]: cannot find type `Entry` in this scope
> > --> rust/kernel/debugfs.rs:351:37
> > |
> > 351 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> > | ^^^^^ not found in this scope
> >
> > And giving it some Entry (for my 1.81.0)
> >
> > error: hidden lifetime parameters in types are deprecated
> > --> rust/kernel/debugfs.rs:352:37
> > |
> > 352 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> > | ^^^^^ expected lifetime parameter
>
> Yeah, I caught this as well and fixed it up on my end with the following diff:
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index ecfcce845d3f..1f25777743db 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -348,7 +348,7 @@ pub struct File<T> {
> impl<'b, T: 'b> Scope<T> {
> fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
> where
> - F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> + F: for<'a> FnOnce(&'a T) + 'b,
> {
> try_pin_init! {
> Self {
>
Can you send this as a fix-up patch?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-10 15:22 ` Greg Kroah-Hartman
@ 2025-09-10 15:23 ` Danilo Krummrich
2025-09-10 15:36 ` Greg Kroah-Hartman
0 siblings, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-10 15:23 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dirk Behme, Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, linux-kernel, rust-for-linux
On 9/10/25 5:22 PM, Greg Kroah-Hartman wrote:
> On Tue, Sep 09, 2025 at 10:29:13AM +0200, Danilo Krummrich wrote:
>> On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
>>> On 04/09/2025 23:13, Matthew Maurer wrote:
>>>> +#[cfg(not(CONFIG_DEBUG_FS))]
>>>> +impl<'b, T: 'b> Scope<T> {
>>>> + fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
>>>> + where
>>>> + F: for<'a> FnOnce(&'a T) -> Entry + 'b,
>>>
>>> Inspired by Greg's & Danilo's discussion I tried building with
>>> CONFIG_DEBUG_FS disabled. And get
>>>
>>> error[E0412]: cannot find type `Entry` in this scope
>>> --> rust/kernel/debugfs.rs:351:37
>>> |
>>> 351 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
>>> | ^^^^^ not found in this scope
>>>
>>> And giving it some Entry (for my 1.81.0)
>>>
>>> error: hidden lifetime parameters in types are deprecated
>>> --> rust/kernel/debugfs.rs:352:37
>>> |
>>> 352 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
>>> | ^^^^^ expected lifetime parameter
>>
>> Yeah, I caught this as well and fixed it up on my end with the following diff:
>>
>> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
>> index ecfcce845d3f..1f25777743db 100644
>> --- a/rust/kernel/debugfs.rs
>> +++ b/rust/kernel/debugfs.rs
>> @@ -348,7 +348,7 @@ pub struct File<T> {
>> impl<'b, T: 'b> Scope<T> {
>> fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
>> where
>> - F: for<'a> FnOnce(&'a T) -> Entry + 'b,
>> + F: for<'a> FnOnce(&'a T) + 'b,
>> {
>> try_pin_init! {
>> Self {
>>
>
> Can you send this as a fix-up patch?
If you don't mind I would fix this (and one other nit) up on apply. :)
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-10 15:23 ` Danilo Krummrich
@ 2025-09-10 15:36 ` Greg Kroah-Hartman
2025-09-10 15:43 ` Danilo Krummrich
0 siblings, 1 reply; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-10 15:36 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Dirk Behme, Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, linux-kernel, rust-for-linux
On Wed, Sep 10, 2025 at 05:23:26PM +0200, Danilo Krummrich wrote:
> On 9/10/25 5:22 PM, Greg Kroah-Hartman wrote:
> > On Tue, Sep 09, 2025 at 10:29:13AM +0200, Danilo Krummrich wrote:
> >> On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
> >>> On 04/09/2025 23:13, Matthew Maurer wrote:
> >>>> +#[cfg(not(CONFIG_DEBUG_FS))]
> >>>> +impl<'b, T: 'b> Scope<T> {
> >>>> + fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
> >>>> + where
> >>>> + F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> >>>
> >>> Inspired by Greg's & Danilo's discussion I tried building with
> >>> CONFIG_DEBUG_FS disabled. And get
> >>>
> >>> error[E0412]: cannot find type `Entry` in this scope
> >>> --> rust/kernel/debugfs.rs:351:37
> >>> |
> >>> 351 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> >>> | ^^^^^ not found in this scope
> >>>
> >>> And giving it some Entry (for my 1.81.0)
> >>>
> >>> error: hidden lifetime parameters in types are deprecated
> >>> --> rust/kernel/debugfs.rs:352:37
> >>> |
> >>> 352 | F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> >>> | ^^^^^ expected lifetime parameter
> >>
> >> Yeah, I caught this as well and fixed it up on my end with the following diff:
> >>
> >> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> >> index ecfcce845d3f..1f25777743db 100644
> >> --- a/rust/kernel/debugfs.rs
> >> +++ b/rust/kernel/debugfs.rs
> >> @@ -348,7 +348,7 @@ pub struct File<T> {
> >> impl<'b, T: 'b> Scope<T> {
> >> fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
> >> where
> >> - F: for<'a> FnOnce(&'a T) -> Entry + 'b,
> >> + F: for<'a> FnOnce(&'a T) + 'b,
> >> {
> >> try_pin_init! {
> >> Self {
> >>
> >
> > Can you send this as a fix-up patch?
>
> If you don't mind I would fix this (and one other nit) up on apply. :)
I've pushed these to driver-core-testing if you could fix it up as I
don't know what the other change was?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-10 15:36 ` Greg Kroah-Hartman
@ 2025-09-10 15:43 ` Danilo Krummrich
2025-09-10 17:10 ` Danilo Krummrich
0 siblings, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-10 15:43 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dirk Behme, Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, linux-kernel, rust-for-linux
On 9/10/25 5:36 PM, Greg Kroah-Hartman wrote:
> I've pushed these to driver-core-testing if you could fix it up as I
> don't know what the other change was?
Sure, will do.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v11 2/7] rust: debugfs: Add support for read-only files
2025-09-10 15:43 ` Danilo Krummrich
@ 2025-09-10 17:10 ` Danilo Krummrich
0 siblings, 0 replies; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-10 17:10 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dirk Behme, Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, linux-kernel, rust-for-linux
On Wed Sep 10, 2025 at 5:43 PM CEST, Danilo Krummrich wrote:
> On 9/10/25 5:36 PM, Greg Kroah-Hartman wrote:
>> I've pushed these to driver-core-testing if you could fix it up as I
>> don't know what the other change was?
>
> Sure, will do.
* rust: debugfs: Add initial support for directories
* rust: debugfs: Add support for read-only files
[ Fixup build failure when CONFIG_DEBUGFS=n. - Danilo ]
* rust: debugfs: Add support for writable files
[ Fix up Result<()> -> Result. - Danilo ]
* rust: debugfs: Add support for callback-based files
[ Fix up Result<(), Error> -> Result. - Danilo ]
* samples: rust: Add debugfs sample driver
[ Change ACPI ID "LNUXDEBF" to "LNUXBEEF". - Danilo ]
* rust: debugfs: Add support for scoped directories
[ Fix up Result<(), Error> -> Result; fix spurious backtick in
doc-comment. - Danilo ]
* samples: rust: Add scoped debugfs sample driver
[ Rename "scoped_debugfs" -> "debugfs_scoped", fix up
Result<(), Error> -> Result. - Danilo ]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v11 3/7] rust: debugfs: Add support for writable files
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
2025-09-04 21:13 ` [PATCH v11 1/7] rust: debugfs: Add initial support for directories Matthew Maurer
2025-09-04 21:13 ` [PATCH v11 2/7] rust: debugfs: Add support for read-only files Matthew Maurer
@ 2025-09-04 21:13 ` Matthew Maurer
2025-09-04 21:13 ` [PATCH v11 4/7] rust: debugfs: Add support for callback-based files Matthew Maurer
` (4 subsequent siblings)
7 siblings, 0 replies; 38+ messages in thread
From: Matthew Maurer @ 2025-09-04 21:13 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 `Reader`
trait.
Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
rust/kernel/debugfs.rs | 37 ++++++++++++-
rust/kernel/debugfs/file_ops.rs | 113 +++++++++++++++++++++++++++++++++++++++-
rust/kernel/debugfs/traits.rs | 69 ++++++++++++++++++++++++
3 files changed, 216 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index b28665f58cd6a17e188aef5e8c539f6c7433a3b0..1f041f09a6eaf5603170ce4c7724b3ef50eecf13 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -16,10 +16,10 @@
use core::ops::Deref;
mod traits;
-pub use traits::Writer;
+pub use traits::{Reader, Writer};
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)]
@@ -142,6 +142,39 @@ pub fn read_only_file<'a, T, E: 'a>(
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 [`Writer`] implementation.
+ /// Writing to the file uses the [`Reader`] implementation.
+ pub fn read_write_file<'a, T, E: 'a>(
+ &'a self,
+ name: &'a CStr,
+ data: impl PinInit<T, E> + 'a,
+ ) -> impl PinInit<File<T>, E> + 'a
+ where
+ T: Writer + Reader + Send + Sync + 'static,
+ {
+ 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 [`Reader`]
+ /// implementation.
+ ///
+ /// The file is removed when the returned [`File`] is dropped.
+ pub fn write_only_file<'a, T, E: 'a>(
+ &'a self,
+ name: &'a CStr,
+ data: impl PinInit<T, E> + 'a,
+ ) -> impl PinInit<File<T>, E> + 'a
+ where
+ T: Reader + Send + Sync + 'static,
+ {
+ 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 c2fbef96580eaa2fab7cc8c1ba559c3284d12e1b..2060c8d14d83455efa6ec179669f2c3fcc35ccaf 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::Writer;
+use super::{Reader, Writer};
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;
@@ -126,3 +127,113 @@ impl<T: Writer + Sync> ReadFile<T> for T {
unsafe { FileOps::new(operations, 0o400) }
};
}
+
+fn read<T: Reader + 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.read_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 `Reader`.
+/// `buf` must be a valid user-space buffer.
+pub(crate) unsafe extern "C" fn write<T: Reader + 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) };
+ read(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: Writer + Reader + Sync> ReadWriteFile<T> for T {
+ const FILE_OPS: FileOps<T> = {
+ let operations = bindings::file_operations {
+ open: Some(writer_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 `writer_open`
+ // and `write`.
+ // `writer_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 `writer_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
+/// `Reader`.
+/// * `buf` must be a valid user-space buffer.
+pub(crate) unsafe extern "C" fn write_only_write<T: Reader + 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) };
+ read(data, buf, count)
+}
+
+pub(crate) trait WriteFile<T> {
+ const FILE_OPS: FileOps<T>;
+}
+
+impl<T: Reader + 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 0e6e461324de42a3d80b692264d50e78a48f561d..3d99482e53a395aef81bb045d3effe827f5f4386 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 written into a string.
///
@@ -31,3 +38,65 @@ fn write(&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 Reader {
+ /// Updates the value from the given user slice.
+ fn read_from_slice(&self, reader: &mut UserSliceReader) -> Result<()>;
+}
+
+impl<T: FromStr> Reader for Mutex<T> {
+ fn read_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_reader_for_atomic {
+ ($(($atomic_type:ty, $int_type:ty)),*) => {
+ $(
+ impl Reader for $atomic_type {
+ fn read_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_reader_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.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v11 4/7] rust: debugfs: Add support for callback-based files
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
` (2 preceding siblings ...)
2025-09-04 21:13 ` [PATCH v11 3/7] rust: debugfs: Add support for writable files Matthew Maurer
@ 2025-09-04 21:13 ` Matthew Maurer
2025-09-04 21:13 ` [PATCH v11 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
` (3 subsequent siblings)
7 siblings, 0 replies; 38+ messages in thread
From: Matthew Maurer @ 2025-09-04 21:13 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 `Writer` or `Reader` 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 | 89 ++++++++++++++++++++++
rust/kernel/debugfs/callback_adapters.rs | 122 +++++++++++++++++++++++++++++++
rust/kernel/debugfs/file_ops.rs | 8 ++
3 files changed, 219 insertions(+)
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 1f041f09a6eaf5603170ce4c7724b3ef50eecf13..1032f279da380c549991e903c4162b7e4aaec571 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::{Reader, Writer};
+mod callback_adapters;
+use callback_adapters::{FormatAdapter, NoWriter, WritableAdapter};
mod file_ops;
use file_ops::{FileOps, ReadFile, ReadWriteFile, WriteFile};
#[cfg(CONFIG_DEBUG_FS)]
@@ -143,6 +147,46 @@ pub fn read_only_file<'a, T, E: 'a>(
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, E: 'a, F>(
+ &'a self,
+ name: &'a CStr,
+ data: impl PinInit<T, E> + 'a,
+ _f: &'static F,
+ ) -> impl PinInit<File<T>, E> + 'a
+ where
+ T: Send + Sync + 'static,
+ F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+ {
+ 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 [`Writer`] implementation.
@@ -159,6 +203,31 @@ pub fn read_write_file<'a, T, E: 'a>(
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, E: 'a, F, W>(
+ &'a self,
+ name: &'a CStr,
+ data: impl PinInit<T, E> + 'a,
+ _f: &'static F,
+ _w: &'static W,
+ ) -> impl PinInit<File<T>, E> + 'a
+ where
+ T: Send + Sync + 'static,
+ F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+ W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+ {
+ 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 [`Reader`]
@@ -175,6 +244,26 @@ pub fn write_only_file<'a, 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, E: 'a, W>(
+ &'a self,
+ name: &'a CStr,
+ data: impl PinInit<T, E> + 'a,
+ _w: &'static W,
+ ) -> impl PinInit<File<T>, E> + 'a
+ where
+ T: Send + Sync + 'static,
+ W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+ {
+ let file_ops = <WritableAdapter<NoWriter<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..6c024230f676d55c8ddacb69de9c27587e29c636
--- /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 write or read implementation as a value rather
+//! than a trait implementation. If provided, it will override the trait implementation.
+
+use super::{Reader, Writer};
+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 `Reader` via a callback with the same representation as `T`.
+///
+/// * Layer it on top of `WriterAdapter` if you want to add a custom callback for `write`.
+/// * Layer it on top of `NoWriter` 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: Writer, W> Writer for WritableAdapter<D, W> {
+ fn write(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+ self.inner.write(fmt)
+ }
+}
+
+impl<D: Deref, W> Reader for WritableAdapter<D, W>
+where
+ W: Fn(&D::Target, &mut UserSliceReader) -> Result + Send + Sync + 'static,
+{
+ fn read_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 `Writer` 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> Writer for FormatAdapter<D, F>
+where
+ F: Fn(&D, &mut Formatter<'_>) -> fmt::Result + 'static,
+{
+ fn write(&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 NoWriter<D> {
+ inner: D,
+}
+
+// SAFETY: Stripping off the adapter only removes constraints
+unsafe impl<D> Adapter for NoWriter<D> {
+ type Inner = D;
+}
+
+impl<D> Deref for NoWriter<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 2060c8d14d83455efa6ec179669f2c3fcc35ccaf..50fead17b6f31feaf1caaef31c24ccdf6d8a5835 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::{Reader, Writer};
+use crate::debugfs::callback_adapters::Adapter;
use crate::prelude::*;
use crate::seq_file::SeqFile;
use crate::seq_print;
@@ -46,6 +47,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.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v11 5/7] samples: rust: Add debugfs sample driver
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
` (3 preceding siblings ...)
2025-09-04 21:13 ` [PATCH v11 4/7] rust: debugfs: Add support for callback-based files Matthew Maurer
@ 2025-09-04 21:13 ` Matthew Maurer
2025-09-05 9:00 ` Danilo Krummrich
2025-09-08 13:08 ` Greg Kroah-Hartman
2025-09-04 21:13 ` [PATCH v11 6/7] rust: debugfs: Add support for scoped directories Matthew Maurer
` (2 subsequent siblings)
7 siblings, 2 replies; 38+ messages in thread
From: Matthew Maurer @ 2025-09-04 21:13 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.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v11 5/7] samples: rust: Add debugfs sample driver
2025-09-04 21:13 ` [PATCH v11 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
@ 2025-09-05 9:00 ` Danilo Krummrich
2025-09-06 3:19 ` Matthew Maurer
2025-09-08 13:08 ` Greg Kroah-Hartman
1 sibling, 1 reply; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-05 9:00 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 Thu Sep 4, 2025 at 11:13 PM CEST, Matthew Maurer wrote:
> +kernel::acpi_device_table!(
> + ACPI_TABLE,
> + MODULE_ACPI_TABLE,
> + <RustDebugFs as platform::Driver>::IdInfo,
> + [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]
This should use "LNUXBEEF", as we explicitly reserved it for sample and test
code.
I think we could reserve more if we see a benefit, but so far it's only used by
the platform driver sample, so we should be good.
Either way, no need to resend for this only, it can be fixed up on apply. :)
> +);
> +
> +impl platform::Driver for RustDebugFs {
> + type IdInfo = ();
> + const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
NIT: This defaults to None, so it can be omitted.
> + const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 5/7] samples: rust: Add debugfs sample driver
2025-09-05 9:00 ` Danilo Krummrich
@ 2025-09-06 3:19 ` Matthew Maurer
2025-09-07 23:25 ` Danilo Krummrich
0 siblings, 1 reply; 38+ messages in thread
From: Matthew Maurer @ 2025-09-06 3:19 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 Fri, Sep 5, 2025 at 2:00 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Sep 4, 2025 at 11:13 PM CEST, Matthew Maurer wrote:
> > +kernel::acpi_device_table!(
> > + ACPI_TABLE,
> > + MODULE_ACPI_TABLE,
> > + <RustDebugFs as platform::Driver>::IdInfo,
> > + [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]
>
> This should use "LNUXBEEF", as we explicitly reserved it for sample and test
> code.
>
> I think we could reserve more if we see a benefit, but so far it's only used by
> the platform driver sample, so we should be good.
>
> Either way, no need to resend for this only, it can be fixed up on apply. :)
Ah, thanks, I was unaware that it was specially reserved and so tried
to pick something distinct. You fixing up this and the `OF_ID_TABLE`
on apply SGTM.
>
> > +);
> > +
> > +impl platform::Driver for RustDebugFs {
> > + type IdInfo = ();
> > + const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
>
> NIT: This defaults to None, so it can be omitted.
>
> > + const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 5/7] samples: rust: Add debugfs sample driver
2025-09-06 3:19 ` Matthew Maurer
@ 2025-09-07 23:25 ` Danilo Krummrich
0 siblings, 0 replies; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-07 23:25 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 Sat Sep 6, 2025 at 5:19 AM CEST, Matthew Maurer wrote:
> On Fri, Sep 5, 2025 at 2:00 AM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Thu Sep 4, 2025 at 11:13 PM CEST, Matthew Maurer wrote:
>> > +kernel::acpi_device_table!(
>> > + ACPI_TABLE,
>> > + MODULE_ACPI_TABLE,
>> > + <RustDebugFs as platform::Driver>::IdInfo,
>> > + [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]
>>
>> This should use "LNUXBEEF", as we explicitly reserved it for sample and test
>> code.
>>
>> I think we could reserve more if we see a benefit, but so far it's only used by
>> the platform driver sample, so we should be good.
>>
>> Either way, no need to resend for this only, it can be fixed up on apply. :)
>
> Ah, thanks, I was unaware that it was specially reserved and so tried
> to pick something distinct. You fixing up this and the `OF_ID_TABLE`
> on apply SGTM.
Ok, I will wait for Greg to have a look first.
- Danilo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v11 5/7] samples: rust: Add debugfs sample driver
2025-09-04 21:13 ` [PATCH v11 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
2025-09-05 9:00 ` Danilo Krummrich
@ 2025-09-08 13:08 ` Greg Kroah-Hartman
2025-09-08 13:30 ` Danilo Krummrich
1 sibling, 1 reply; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-08 13:08 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,
Danilo Krummrich, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, Dirk Beheme, linux-kernel, rust-for-linux
On Thu, Sep 04, 2025 at 09:13:56PM +0000, 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>
> ---
> 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>>,
Why are you saving the file pointers here at all? They shouldn't be
needed to keep around, all that should be needed is the Dir, right? If
not, this needs to be revisited...
And why do you need to keep "counter"? Shouldn't that be passed back
somehow in the callback automatically?
You use it:
> +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);
Here? This feels odd, the file is the wrapper for the variable you want
the file to show? That feels backwards. Usually the variable is always
present, and then you can create a debugfs file to read/modify it. But
if debugfs is NOT enabled, you can still use the variable, it's just the
file logic that is gone, right?
So what would this file look like without debugfs being enabled? Would
the atomic variable still be here? I think it wouldn't, which is not
ok...
Or am I confused again?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v11 5/7] samples: rust: Add debugfs sample driver
2025-09-08 13:08 ` Greg Kroah-Hartman
@ 2025-09-08 13:30 ` Danilo Krummrich
0 siblings, 0 replies; 38+ messages in thread
From: Danilo Krummrich @ 2025-09-08 13:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthew Maurer, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Rafael J. Wysocki, Sami Tolvanen, Timur Tabi, Benno Lossin,
Dirk Beheme, linux-kernel, rust-for-linux
On Mon Sep 8, 2025 at 3:08 PM CEST, Greg Kroah-Hartman wrote:
> On Thu, Sep 04, 2025 at 09:13:56PM +0000, 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>
>> ---
>> 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>>,
>
> Why are you saving the file pointers here at all? They shouldn't be
> needed to keep around, all that should be needed is the Dir, right? If
> not, this needs to be revisited...
It's not really the file pointers, but the actual data, i.e. a CString,
AtomicUsize and a Mutex<Inner>. That they're wrapped by the File<T> type, only
means that they're exported via debugfs (if enabled, otherwise they are not).
>> +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);
>
> Here? This feels odd, the file is the wrapper for the variable you want
> the file to show? That feels backwards. Usually the variable is always
> present, and then you can create a debugfs file to read/modify it. But
> if debugfs is NOT enabled, you can still use the variable, it's just the
> file logic that is gone, right?
Please see [1]. :)
[1] https://lore.kernel.org/lkml/DCNG8UF8XFT2.12S9I7MBNV5PX@kernel.org/
> So what would this file look like without debugfs being enabled? Would
> the atomic variable still be here? I think it wouldn't, which is not
> ok...
Yes, that atomic would still be there, the File<AtomicUsize> behaves exactly
like a standalone AtomicUsize (except that it exports it in debugfs), and in
case of debugfs disabled you would access the atomic the exact same way, just
that no actual debugfs file exporting the AtomicUsize is created in the
background -- so fully transparent.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v11 6/7] rust: debugfs: Add support for scoped directories
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
` (4 preceding siblings ...)
2025-09-04 21:13 ` [PATCH v11 5/7] samples: rust: Add debugfs sample driver Matthew Maurer
@ 2025-09-04 21:13 ` Matthew Maurer
2025-09-04 21:13 ` [PATCH v11 7/7] samples: rust: Add scoped debugfs sample driver Matthew Maurer
2025-09-08 7:01 ` [PATCH v11 0/7] rust: DebugFS Bindings Dirk Behme
7 siblings, 0 replies; 38+ messages in thread
From: Matthew Maurer @ 2025-09-04 21:13 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 | 262 +++++++++++++++++++++++++++++++++++++++++--
rust/kernel/debugfs/entry.rs | 73 +++++++++++-
2 files changed, 320 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 1032f279da380c549991e903c4162b7e4aaec571..ecfcce845d3f1e9183a55e16629564776f80c6f0 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.
@@ -264,17 +267,67 @@ pub fn write_callback_file<'a, T, E: 'a, W>(
.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, F>(
+ &'a self,
+ data: impl PinInit<T, E> + 'a,
+ name: &'a CStr,
+ init: F,
+ ) -> impl PinInit<Scope<T>, E> + 'a
+ where
+ F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
+ {
+ Scope::new(data, |data| {
+ let scoped = self.scoped_dir(name);
+ init(data, &scoped);
+ scoped.into_entry()
+ })
+ }
}
#[pin_data]
-/// Handle to a DebugFS scope, which ensures that attached `data` will outlive the provided
-/// [`Entry`] without moving.
-/// Currently, this is used to back [`File`] so that its `read` and/or `write` implementations
-/// can assume that their backing data is still alive.
-struct Scope<T> {
+/// Handle to a DebugFS scope, which ensures that attached `data` will outlive the DebugFS entry
+/// without moving.
+///
+/// This is internally used to back [`File`], and used in the API to represent the attachment
+/// of a directory lifetime to a data structure which may be jointly accessed by a number of
+/// different files.
+///
+/// When dropped, a `Scope` will remove all directories and files in the filesystem backed by the
+/// attached data structure prior to releasing the attached data.
+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.
@@ -312,14 +365,14 @@ fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E
#[cfg(CONFIG_DEBUG_FS)]
impl<'b, T: 'b> 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<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
where
- F: for<'a> FnOnce(&'a T) -> Entry + 'b,
+ F: for<'a> FnOnce(&'a T) -> Entry<'static> + 'b,
{
try_pin_init! {
Self {
@@ -335,6 +388,31 @@ fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E
}
}
+impl<'a, T: 'a> 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<E: 'a, F>(
+ data: impl PinInit<T, E> + 'a,
+ name: &'a CStr,
+ init: F,
+ ) -> impl PinInit<Self, E> + 'a
+ where
+ F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + '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 {
@@ -348,3 +426,169 @@ 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 [`Writer::write`]`.
+ ///
+ /// 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: Writer + 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, F>(&self, name: &CStr, data: &'data T, _f: &'static F)
+ where
+ T: Send + Sync + 'static,
+ F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+ {
+ 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 [`Writer`] implementation on `data`. Writing to the file uses
+ /// the [`Reader`] 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: Writer + Reader + 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, F, W>(
+ &self,
+ name: &CStr,
+ data: &'data T,
+ _f: &'static F,
+ _w: &'static W,
+ ) where
+ T: Send + Sync + 'static,
+ F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,
+ W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+ {
+ 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 [`Reader`] 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: Reader + 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, W>(&self, name: &CStr, data: &'data T, _w: &'static W)
+ where
+ T: Send + Sync + 'static,
+ W: Fn(&T, &mut UserSliceReader) -> Result<(), Error> + Send + Sync,
+ {
+ let vtable = &<WritableAdapter<NoWriter<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.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v11 7/7] samples: rust: Add scoped debugfs sample driver
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
` (5 preceding siblings ...)
2025-09-04 21:13 ` [PATCH v11 6/7] rust: debugfs: Add support for scoped directories Matthew Maurer
@ 2025-09-04 21:13 ` Matthew Maurer
2025-09-08 13:04 ` Greg Kroah-Hartman
2025-09-08 7:01 ` [PATCH v11 0/7] rust: DebugFS Bindings Dirk Behme
7 siblings, 1 reply; 38+ messages in thread
From: Matthew Maurer @ 2025-09-04 21:13 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.355.g5224444f11-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v11 7/7] samples: rust: Add scoped debugfs sample driver
2025-09-04 21:13 ` [PATCH v11 7/7] samples: rust: Add scoped debugfs sample driver Matthew Maurer
@ 2025-09-08 13:04 ` Greg Kroah-Hartman
0 siblings, 0 replies; 38+ messages in thread
From: Greg Kroah-Hartman @ 2025-09-08 13:04 UTC (permalink / raw)
To: Matthew Maurer
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Rafael J. Wysocki, Sami Tolvanen, Timur Tabi,
Benno Lossin, Dirk Beheme, linux-kernel, rust-for-linux
On Thu, Sep 04, 2025 at 09:13:58PM +0000, Matthew Maurer wrote:
> 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 ++++++++++++++++++++++++++++++++++++
Nit, should be called "rust_debugfs_scoped.rs" to make it more obvious
this is a debugfs thing (noun-verb).
thanks,
greg k-h
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v11 0/7] rust: DebugFS Bindings
2025-09-04 21:13 [PATCH v11 0/7] rust: DebugFS Bindings Matthew Maurer
` (6 preceding siblings ...)
2025-09-04 21:13 ` [PATCH v11 7/7] samples: rust: Add scoped debugfs sample driver Matthew Maurer
@ 2025-09-08 7:01 ` Dirk Behme
7 siblings, 0 replies; 38+ messages in thread
From: Dirk Behme @ 2025-09-08 7:01 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 04/09/2025 23:13, Matthew Maurer wrote:
> This series provides safe DebugFS bindings for Rust, with sample
> modules using them.
>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
On ARM64 on top of v6.17-rc1 I switched from v10 used previously to this
v11 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] 38+ messages in thread