rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH WIP v2 0/2] rust: debugfs: Support attaching data to DebugFS directories
@ 2025-05-06  1:04 Matthew Maurer
  2025-05-06  1:04 ` [PATCH WIP v2 1/2] rust: debugfs: Add interface to build debugfs off pinned objects Matthew Maurer
  2025-05-06  1:04 ` [PATCH WIP v2 2/2] rust: debugfs: Extend sample to use attached data Matthew Maurer
  0 siblings, 2 replies; 3+ messages in thread
From: Matthew Maurer @ 2025-05-06  1:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Sami Tolvanen, Timur Tabi
  Cc: rust-for-linux, linux-kernel, Matthew Maurer

We can wait to properly review and land this until after we've decided
on the final interface for the first part, but since dakr@kernel.org was
poking at how this might work in his review of the previous patchset, I
wanted to upload this sketch as context.

The general concept here is that you select an owning directory and
attach data to it while converting its lifetime to be invariant (e.g.
can't be shortened) so that you know that the DebugFS contents will be
cleaned up before the data. You can then implement things underneath
that directory using the attached data.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
Changes in v2:
- Rebased on top of new DebugFS bindings to make it easier to play with.
- Attachment model still hasn't changed as that's still being discussed.
  See: https://lore.kernel.org/all/CAGSQo03sHhUX-Eo31cMmeNMaNnzWxU0c-ObTkr7Z1CJ2uQ6c4g@mail.gmail.com/
- Link to v1: https://lore.kernel.org/r/20250503-debugfs-rust-attach-v1-0-dc37081fbfbc@google.com

---
Matthew Maurer (2):
      rust: debugfs: Add interface to build debugfs off pinned objects
      rust: debugfs: Extend sample to use attached data

 rust/kernel/debugfs.rs       | 218 +++++++++++++++++++++++++++++++++++++++++--
 samples/rust/rust_debugfs.rs | 110 +++++++++++++++++++++-
 2 files changed, 314 insertions(+), 14 deletions(-)
---
base-commit: 33035b665157558254b3c21c3f049fd728e72368
change-id: 20250501-debugfs-rust-attach-e164b67c9c16
prerequisite-change-id: 20250428-debugfs-rust-3cd5c97eb7d1:v5
prerequisite-patch-id: 1bc0f0d9ea12a4dfa986bd6b88863b0561d7e215
prerequisite-patch-id: 6338f30c269eb6ec314d3e4c0b9c2c0215f58325
prerequisite-patch-id: 862adaf3d9fbc860a8eb23bf8779c4b299c0149e
prerequisite-patch-id: 0198767a9410d24382e72ca9cf1e3d205deb4fd3

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


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

* [PATCH WIP v2 1/2] rust: debugfs: Add interface to build debugfs off pinned objects
  2025-05-06  1:04 [PATCH WIP v2 0/2] rust: debugfs: Support attaching data to DebugFS directories Matthew Maurer
@ 2025-05-06  1:04 ` Matthew Maurer
  2025-05-06  1:04 ` [PATCH WIP v2 2/2] rust: debugfs: Extend sample to use attached data Matthew Maurer
  1 sibling, 0 replies; 3+ messages in thread
From: Matthew Maurer @ 2025-05-06  1:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Sami Tolvanen, Timur Tabi
  Cc: rust-for-linux, linux-kernel, Matthew Maurer

Previously, we could only expose `'static` references to DebugFS because
we don't know how long the file could live. This provides a way to
package data alongside an owning directory to guarantee that it will
live long enough to be accessed by the DebugFS files, while still
allowing a dynamic lifecycle.

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

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index e2f5960bb87f24607780b3f4e67039e379f3bda6..560a2b68c7d28371ae11bc90efd15e7ada17ff77 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -5,10 +5,15 @@
 //!
 //! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
 
+use crate::prelude::PinInit;
 use crate::str::CStr;
 use core::fmt;
 use core::fmt::Display;
-use core::marker::PhantomData;
+use core::marker::{PhantomData, PhantomPinned};
+use core::mem::ManuallyDrop;
+use core::ops::Deref;
+use core::pin::Pin;
+use pin_init::pin_data;
 
 /// Owning handle to a DebugFS entry.
 ///
@@ -80,6 +85,19 @@ fn new() -> Self {
     fn as_ptr(&self) -> *mut bindings::dentry {
         self.entry
     }
+
+    /// Rescopes the entry based on a borrow.
+    ///
+    /// # Safety
+    ///
+    /// Caller promises they will not drop the wrapped entry.
+    unsafe fn borrow<'b>(&'b self) -> ManuallyDrop<Entry<'b>> {
+        ManuallyDrop::new(Entry {
+            #[cfg(CONFIG_DEBUG_FS)]
+            entry: self.entry,
+            _phantom: PhantomData,
+        })
+    }
 }
 
 impl Drop for Entry<'_> {
@@ -186,6 +204,8 @@ pub fn fmt_file<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
         f: &'static F,
     ) -> File<'b> {
         // SAFETY: As `data` lives for the static lifetime, it outlives the file
+        // As we return `File<'b>`, and we have a borrow to a directory with lifetime 'b, we have a
+        // lower bound of `'b` on the directory.
         unsafe { self.fmt_file_raw(name, data, f) }
     }
 
@@ -197,11 +217,10 @@ pub fn fmt_file<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
     /// This means that before `data` may become invalid, either:
     /// * The refcount must go to zero.
     /// * The file must be rendered inaccessible, e.g. via `debugfs_remove`.
-    unsafe fn display_file_raw<'b, T: Display + Sized>(
-        &'b self,
-        name: &CStr,
-        data: *const T,
-    ) -> File<'b> {
+    /// * If `self` may take longer than `'a` to be destroyed, the caller is responsible for
+    ///   shortening the lifetime of the return value to a lower bound for the lifetime of that
+    ///   object.
+    unsafe fn display_file_raw<T: Display + Sized>(&self, name: &CStr, data: *const T) -> File<'a> {
         // SAFETY:
         // * `name` is a NUL-terminated C string, living across the call, by `CStr` invariant.
         // * `parent` is a live `dentry` since we have a reference to it.
@@ -243,13 +262,16 @@ unsafe fn display_file_raw<'b, T: Display + Sized>(
     ///
     /// # Safety
     ///
-    /// `data` must outlive the resulting file's accessibility
-    unsafe fn fmt_file_raw<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
-        &'b self,
+    /// * `data` must outlive the resulting file's accessibility
+    /// * If `self` may take longer than `'a` to be destroyed, the caller is responsible for
+    ///   shortening the lifetime of the return value to a lower bound for the lifetime of that
+    ///   object.
+    unsafe fn fmt_file_raw<T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
+        &self,
         name: &CStr,
         data: &T,
         f: &'static F,
-    ) -> File<'b> {
+    ) -> File<'a> {
         #[cfg(CONFIG_DEBUG_FS)]
         let data_adapted = FormatAdapter::new(data, f);
         #[cfg(not(CONFIG_DEBUG_FS))]
@@ -280,6 +302,182 @@ pub fn new(name: &CStr) -> Self {
 #[repr(transparent)]
 pub struct File<'a>(Entry<'a>);
 
+/// A DebugFS directory combined with a backing store for data to implement it.
+#[pin_data]
+pub struct Values<T> {
+    dir: Dir<'static>,
+    // The order here is load-bearing - `dir` must be dropped before `backing`, as files under
+    // `dir` may point into `backing`.
+    #[pin]
+    backing: T,
+    // Since the files present under our directory may point into `backing`, we are `!Unpin`.
+    #[pin]
+    _pin: PhantomPinned,
+}
+
+impl<T> Deref for Values<T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.backing
+    }
+}
+
+impl<T> Values<T> {
+    /// Attach backing data to a DebugFS directory.
+    ///
+    /// Attached data will be available inside [`Values::build`] to use when defining files in
+    /// the provided directory. When this object is dropped, it will remove the provided directory
+    /// before destroying the attached data.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::{c_str, new_mutex};
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"));
+    /// let debugfs = KBox::pin_init(Values::attach(new_mutex!(0), dir), GFP_KERNEL)?;
+    /// // Files can be put inside `debugfs` which reference the constructed mutex.
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn attach(backing: impl PinInit<T>, dir: Dir<'static>) -> impl PinInit<Self> {
+        pin_init::pin_init! { Self {
+            dir: dir,
+            backing <- backing,
+            _pin: PhantomPinned,
+        }}
+    }
+
+    /// Runs a function which can create files from the backing data.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::{c_str, new_mutex};
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"));
+    /// let debugfs = KBox::pin_init(Values::attach(new_mutex!(0), dir), GFP_KERNEL)?;
+    /// debugfs.as_ref().build(|mutex, dir| {
+    ///   // Can access `BoundDir` methods on `dir` here, with lifetime unified to the
+    ///   // lifetime of `mutex`
+    /// });
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn build<U, F: for<'b> FnOnce(&'b T, BoundDir<'b>) -> U>(self: Pin<&Self>, f: F) -> U {
+        // SAFETY: The `BoundDir` here is only being provided as a universally quantified argument
+        // to a function, so in the body, it will only be available in an existentially quantified
+        // context. This means that the function is legal to execute agains the true lifetime of
+        // the directory, even if we don't know exactly what that is.
+        f(&self.backing, unsafe { BoundDir::new(&self.dir) })
+    }
+}
+
+/// A `Dir`, bound to the lifetime for which it will exist. Unlike `&'a Dir`, this has an invariant
+/// lifetime - it cannot be shortened or lengthened.
+///
+/// # Invariants
+///
+/// * `BoundDir`'s lifetime must match the actual lifetime the directory lives for. In practice,
+///   this means that a `BoundDir` may only be used in an existentially quantified context.
+/// * `dir` will never be promoted to an owning reference (e.g. via calling `Dir::owning`)
+#[repr(transparent)]
+pub struct BoundDir<'a> {
+    dir: ManuallyDrop<Dir<'a>>,
+    _invariant: PhantomData<fn(&'a ()) -> &'a ()>,
+}
+
+impl<'a> BoundDir<'a> {
+    /// Create a new bound directory.
+    ///
+    /// # Safety
+    ///
+    /// `'a` is being used in a context where it is existentially quantified.
+    unsafe fn new(dir: &'a Dir<'_>) -> Self {
+        // SAFETY: We will keep the return wrapped in ManuallyDrop, so we will not drop it.
+        let entry = unsafe { dir.0.borrow() };
+        // Rewrap it into a directory
+        let dir = ManuallyDrop::new(Dir(ManuallyDrop::into_inner(entry)));
+        Self {
+            dir,
+            _invariant: PhantomData,
+        }
+    }
+
+    /// Create a DebugFS subdirectory.
+    ///
+    /// This will produce another [`BoundDir`], scoped to the lifetime of the parent [`BoundDir`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let parent = Dir::new(c_str!("parent"));
+    /// let values = KBox::pin_init(Values::attach(0, parent), GFP_KERNEL)?;
+    /// values.as_ref().build(|value, builder| {
+    ///   builder.subdir(c_str!("child"));
+    /// });
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn subdir<'b>(&'b self, name: &CStr) -> BoundDir<'b> {
+        BoundDir {
+            dir: ManuallyDrop::new(Dir::create(name, Some(&self.dir))),
+            _invariant: PhantomData,
+        }
+    }
+
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking `f`
+    /// on the provided reference.
+    ///
+    /// `f` must be a function item or a non-capturing closure, or this will fail to compile.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::{c_str, new_mutex};
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let parent = Dir::new(c_str!("parent"));
+    /// let values = KBox::pin_init(Values::attach(new_mutex!(0), parent), GFP_KERNEL)?;
+    /// values.as_ref().build(|value, builder| {
+    ///   builder.fmt_file(c_str!("child"), value, &|value, f| {
+    ///     writeln!(f, "Reading a mutex: {}", *value.lock())
+    ///   });
+    /// });
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn fmt_file<T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
+        &self,
+        name: &CStr,
+        data: &'a T,
+        f: &'static F,
+    ) -> File<'a> {
+        // SAFETY: As `data` lives for the same lifetime as our `BoundDir` lifetime, which is
+        // instantiated as the actual lifetime of the directory, it lives long enough.
+        // We don't need to shorten the file handle because `BoundDir`'s lifetime parameter is both
+        // a lower *and* upper bound.
+        unsafe { self.dir.fmt_file_raw(name, data, f) }
+    }
+
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking
+    /// [`Display::fmt`] on the provided reference with a trailing newline.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::{c_str, new_mutex};
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let parent = Dir::new(c_str!("parent"));
+    /// let values = KBox::pin_init(Values::attach((1, 2), parent), GFP_KERNEL)?;
+    /// values.as_ref().build(|value, builder| {
+    ///   builder.display_file(c_str!("child_0"), &value.0);
+    ///   builder.display_file(c_str!("child_1"), &value.1);
+    /// });
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn display_file<T: Display>(&self, name: &CStr, data: &'a T) -> File<'a> {
+        self.fmt_file(name, data, &|data, f| writeln!(f, "{}", data))
+    }
+}
+
 #[cfg(CONFIG_DEBUG_FS)]
 mod helpers {
     use crate::seq_file::SeqFile;

-- 
2.49.0.967.g6a0df3ecc3-goog


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

* [PATCH WIP v2 2/2] rust: debugfs: Extend sample to use attached data
  2025-05-06  1:04 [PATCH WIP v2 0/2] rust: debugfs: Support attaching data to DebugFS directories Matthew Maurer
  2025-05-06  1:04 ` [PATCH WIP v2 1/2] rust: debugfs: Add interface to build debugfs off pinned objects Matthew Maurer
@ 2025-05-06  1:04 ` Matthew Maurer
  1 sibling, 0 replies; 3+ messages in thread
From: Matthew Maurer @ 2025-05-06  1:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Sami Tolvanen, Timur Tabi
  Cc: rust-for-linux, linux-kernel, Matthew Maurer

Demonstrates how to attach data/handles needed for implementing DebugFS
file to a directory.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 samples/rust/rust_debugfs.rs | 110 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 106 insertions(+), 4 deletions(-)

diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
index a4b17c8241330b2f6caf8f17a5b2366138de6ced..efaca1b140f710fcc7d0467e9b00e02a69c2cf55 100644
--- a/samples/rust/rust_debugfs.rs
+++ b/samples/rust/rust_debugfs.rs
@@ -4,11 +4,15 @@
 
 //! Sample DebugFS exporting module
 
+use core::fmt;
+use core::fmt::{Display, Formatter};
 use core::mem::{forget, ManuallyDrop};
 use core::sync::atomic::{AtomicU32, Ordering};
 use kernel::c_str;
-use kernel::debugfs::Dir;
+use kernel::debugfs::{BoundDir, Dir, Values};
+use kernel::new_mutex;
 use kernel::prelude::*;
+use kernel::sync::{Arc, Mutex};
 
 module! {
     type: RustDebugFs,
@@ -21,7 +25,89 @@
 struct RustDebugFs {
     // As we only hold this for drop effect (to remove the directory) we have a leading underscore
     // to indicate to the compiler that we don't expect to use this field directly.
-    _debugfs: Dir<'static>,
+    _debugfs: Pin<KBox<Values<Backing>>>,
+}
+
+struct Composite {
+    major: u32,
+    minor: u32,
+}
+
+struct Record {
+    name: &'static CStr,
+    size: usize,
+    stride: usize,
+}
+
+struct Backing {
+    simple: u32,
+    composite: Composite,
+    custom: u32,
+    many: KVec<Record>,
+    atomic: AtomicU32,
+    locked: Arc<Mutex<u32>>,
+}
+
+impl Display for Composite {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{}.{}", self.major, self.minor)
+    }
+}
+
+impl Backing {
+    fn new() -> Result<Self> {
+        let mut many = KVec::new();
+        many.push(
+            Record {
+                name: c_str!("foo"),
+                size: 1,
+                stride: 2,
+            },
+            GFP_KERNEL,
+        )?;
+        many.push(
+            Record {
+                name: c_str!("bar"),
+                size: 3,
+                stride: 4,
+            },
+            GFP_KERNEL,
+        )?;
+        Ok(Self {
+            simple: 10,
+            composite: Composite { major: 1, minor: 2 },
+            custom: 37,
+            many,
+            atomic: AtomicU32::new(7),
+            locked: Arc::pin_init(new_mutex!(0), GFP_KERNEL)?,
+        })
+    }
+
+    fn build<'a>(&'a self, root: BoundDir<'a>) {
+        // Just prints out the number in the field
+        forget(root.display_file(c_str!("simple"), &self.simple));
+        // Uses the custom display implementation to print major.minor
+        forget(root.display_file(c_str!("composite"), &self.composite));
+        // Uses the custom hook print the number in 0-padded hex with some decorations.
+        forget(root.fmt_file(c_str!("custom"), &self.custom, &|custom, f| {
+            writeln!(f, "Foo! {:#010x} Bar!", custom)
+        }));
+        // Creates a directory for every record in the list, named the name of the item, with files
+        // for each attribute.
+        for record in self.many.iter() {
+            let dir = ManuallyDrop::new(root.subdir(record.name));
+            forget(dir.display_file(c_str!("size"), &record.size));
+            forget(dir.display_file(c_str!("stride"), &record.stride));
+        }
+        // Access the attached atomic via `.load()`
+        forget(root.fmt_file(c_str!("atomic"), &self.atomic, &|atomic, f| {
+            writeln!(f, "{}", atomic.load(Ordering::Relaxed))
+        }));
+        // Access the attached mutex-guarded data via `.lock()`
+        forget(root.fmt_file(c_str!("locked"), &self.locked, &|locked, f| {
+            writeln!(f, "{}", *locked.lock())
+        }));
+    }
 }
 
 static EXAMPLE: AtomicU32 = AtomicU32::new(8);
@@ -29,13 +115,13 @@ struct RustDebugFs {
 impl kernel::Module for RustDebugFs {
     fn init(_this: &'static ThisModule) -> Result<Self> {
         // Create a debugfs directory in the root of the filesystem called "sample_debugfs".
-        let debugfs = Dir::new(c_str!("sample_debugfs"));
+        let root = Dir::new(c_str!("sample_debugfs"));
 
         {
             // Create a subdirectory, so "sample_debugfs/subdir" now exists.
             // We wrap it in `ManuallyDrop` so that the subdirectory is not automatically discarded
             // at the end of the scope - it will be cleaned up when `debugfs` is.
-            let sub = ManuallyDrop::new(debugfs.subdir(c_str!("subdir")));
+            let sub = ManuallyDrop::new(root.subdir(c_str!("subdir")));
 
             // Create a single file in the subdirectory called "example" that will read from the
             // `EXAMPLE` atomic variable.
@@ -51,6 +137,22 @@ fn init(_this: &'static ThisModule) -> Result<Self> {
         EXAMPLE.store(10, Ordering::Relaxed);
         // Now, "sample_debugfs/subdir/example" will print "Reading atomic: 10\n" when read.
 
+        // We can also attach data scoped to our root directory
+        let backing = Backing::new()?;
+        // Grab a refcount pointing to the locked data
+        let locked = backing.locked.clone();
+        let debugfs = KBox::pin_init(Values::attach(backing, root), GFP_KERNEL)?;
+
+        // Once it's attached, we can invoke `Backing::build` to create the relevant files:
+        debugfs.as_ref().build(Backing::build);
+
+        // We can still access read-only references the contents of the attached values. If the
+        // values allow interior mutability, like atomics, this lets us still change them:
+        debugfs.as_ref().atomic.store(8, Ordering::Relaxed);
+
+        // If we attached refcounted data, we can use an external handle to access it
+        *locked.lock() = 42;
+
         // Save the root debugfs directory we created to our module object. It will be
         // automatically cleaned up when our module is unloaded because dropping the module object
         // will drop the `Dir` handle. The base directory, the subdirectory, and the file will all

-- 
2.49.0.967.g6a0df3ecc3-goog


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

end of thread, other threads:[~2025-05-06  1:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-06  1:04 [PATCH WIP v2 0/2] rust: debugfs: Support attaching data to DebugFS directories Matthew Maurer
2025-05-06  1:04 ` [PATCH WIP v2 1/2] rust: debugfs: Add interface to build debugfs off pinned objects Matthew Maurer
2025-05-06  1:04 ` [PATCH WIP v2 2/2] rust: debugfs: Extend sample to use attached data Matthew Maurer

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