Allows creation of files for references that live forever and lack
metadata through the `Display` implementation.
The reference must live forever because the corresponding file is
reference counted, so there's no way to say the lifetime outlives it
otherwise. This restriction will be relaxed later in the series through
use of `debugfs_remove`.
The `Display` implementation is used because `seq_printf` needs to route
through `%pA`, which in turn routes through Arguments. A more generic
API is provided later in the series.
Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
rust/kernel/debugfs.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 94 insertions(+)
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 4d06cce7099607f95b684bad329f791a815d3e86..b20df5fce692b3047c804f7ad5af90fc4248979b 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -7,8 +7,11 @@
//! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
use crate::error::{from_err_ptr, Result};
+use crate::seq_file::SeqFile;
+use crate::seq_print;
use crate::str::CStr;
use crate::types::{ARef, AlwaysRefCounted, Opaque};
+use core::fmt::Display;
use core::ptr::NonNull;
/// Handle to a DebugFS directory.
@@ -97,4 +100,95 @@ pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> {
fn as_ptr(&self) -> *mut bindings::dentry {
self.inner.get()
}
+
+ /// Create a file in a DebugFS directory with the provided name, and contents from invoking
+ /// [`Display::fmt`] on the provided reference.
+ ///
+ /// # Example
+ ///
+ /// ```
+ /// # use kernel::c_str;
+ /// # use kernel::debugfs::Dir;
+ /// let dir = Dir::new(c_str!("my_debugfs_dir"), None)?;
+ /// let file = dir.display_file(c_str!("foo"), &200)?;
+ /// # Ok::<(), Error>(())
+ /// ```
+ pub fn display_file<T: Display + Sized>(
+ &self,
+ name: &CStr,
+ data: &'static T,
+ ) -> Result<ARef<Self>> {
+ // SAFETY:
+ // * `name` is a NUL-terminated C string, living across the call, by CStr invariant
+ // * `parent` is a live dentry since we have a reference to it
+ // * `vtable` is all stock `seq_file` implementations except for `open`.
+ // `open`'s only requirement beyond what is provided to all open functions is that the
+ // inode's data pointer must point to a `T` that will outlive it, which we know because
+ // we have a static reference.
+ // * debugfs_create_file_full either returns an error code or a legal dentry pointer, so
+ // `NonNull::new_unchecked` is safe here.
+ let ptr = unsafe {
+ NonNull::new_unchecked(from_err_ptr(kernel::bindings::debugfs_create_file_full(
+ name.as_char_ptr(),
+ 0444,
+ self.as_ptr(),
+ data as *const _ as *mut _,
+ core::ptr::null(),
+ &<T as DisplayFile>::VTABLE,
+ ))?)
+ };
+ // SAFETY: Dir is a transparent wrapper for an Opaque<dentry>, and we received a live
+ // owning dentry from debugfs_create_dir, so we can wrap it in an ARef
+ Ok(unsafe { ARef::from_raw(ptr.cast()) })
+ }
+}
+
+/// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`
+///
+/// # Safety
+/// * inode's private pointer must point to a value of type T which will outlive the inode and will
+/// not be mutated during this call
+/// * file must point to a live, not-yet-initialized file object
+unsafe extern "C" fn display_open<T: Display>(
+ inode: *mut kernel::bindings::inode,
+ file: *mut kernel::bindings::file,
+) -> i32 {
+ // SAFETY:
+ // * file is acceptable by caller precondition
+ // * print_act will be called on a seq_file with private data set to the third argument, so we
+ // meet its safety requirements
+ // * The data pointer passed in the third argument is a valid T pointer that outlives this call
+ // by caller preconditions
+ unsafe { kernel::bindings::single_open(file, Some(display_act::<T>), (*inode).i_private) }
}
+
+/// Prints private data stashed in a seq_file to that seq file
+// # Safety
+// * seq must point to a live seq_file whose private data is a live pointer to a T which is not
+// being mutated.
+unsafe extern "C" fn display_act<T: Display>(
+ seq: *mut kernel::bindings::seq_file,
+ _: *mut core::ffi::c_void,
+) -> i32 {
+ // SAFETY: By caller precondition, this pointer is live, points to a value of type T, and is
+ // not being mutated.
+ let data = unsafe { &*((*seq).private as *mut T) };
+ // SAFETY: By caller precondition, seq_file points to a live seq_file, so we can lift it
+ let seq_file = unsafe { SeqFile::from_raw(seq) };
+ seq_print!(seq_file, "{}", data);
+ 0
+}
+
+// Work around lack of generic const items
+trait DisplayFile: Display + Sized {
+ const VTABLE: kernel::bindings::file_operations = kernel::bindings::file_operations {
+ read: Some(kernel::bindings::seq_read),
+ llseek: Some(kernel::bindings::seq_lseek),
+ release: Some(kernel::bindings::single_release),
+ open: Some(display_open::<Self> as _),
+ // SAFETY: file_operations supports zeroes in all fields
+ ..unsafe { core::mem::zeroed() }
+ };
+}
+
+impl<T: Display + Sized> DisplayFile for T {}
--
2.49.0.901.g37484f566f-googOn Tue, Apr 29, 2025 at 11:15:56PM +0000, Matthew Maurer wrote: > Allows creation of files for references that live forever and lack > metadata through the `Display` implementation. What do you mean "live forever"? There's always a structure somewhere that holds it, when it goes away the file/directory should also go away. > The reference must live forever because the corresponding file is > reference counted, so there's no way to say the lifetime outlives it > otherwise. This restriction will be relaxed later in the series through > use of `debugfs_remove`. Why not use that from the beginning? > The `Display` implementation is used because `seq_printf` needs to route > through `%pA`, which in turn routes through Arguments. A more generic > API is provided later in the series. Building stuff up to review only to remove it later makes it harder to review :) thanks, greg k-h
On Wed, Apr 30, 2025 at 5:06 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Tue, Apr 29, 2025 at 11:15:56PM +0000, Matthew Maurer wrote: > > Allows creation of files for references that live forever and lack > > metadata through the `Display` implementation. > > What do you mean "live forever"? There's always a structure somewhere > that holds it, when it goes away the file/directory should also go away. That's the builder implementation I mentioned later - it lets you connect the directory to a resource. This is an initial simpler version to keep the patches small which lets you export e.g. the contents of global variables safely. > > > The reference must live forever because the corresponding file is > > reference counted, so there's no way to say the lifetime outlives it > > otherwise. This restriction will be relaxed later in the series through > > use of `debugfs_remove`. > > Why not use that from the beginning? Because that requires the builder API + pinning, which I thought would be a more complex conversation, and wanted to keep the "just register a file" patch separate from the "here's how you ensure that data lives at least as long as the debugfs directory" patch. > > > The `Display` implementation is used because `seq_printf` needs to route > > through `%pA`, which in turn routes through Arguments. A more generic > > API is provided later in the series. > > Building stuff up to review only to remove it later makes it harder to > review :) It doesn't really get removed - `display_file_raw` is still the underlying implementation for the more generic API, and this API stays around as a convenience API. > > thanks, > > greg k-h
On Tue, Apr 29, 2025 at 11:15:56PM +0000, Matthew Maurer wrote:
> + /// Create a file in a DebugFS directory with the provided name, and contents from invoking
> + /// [`Display::fmt`] on the provided reference.
> + ///
> + /// # Example
> + ///
> + /// ```
> + /// # use kernel::c_str;
> + /// # use kernel::debugfs::Dir;
> + /// let dir = Dir::new(c_str!("my_debugfs_dir"), None)?;
Again, can never fail, so don't check it.
> + /// let file = dir.display_file(c_str!("foo"), &200)?;
Again, can never fail, so don't check it.
And "200" is wrong, did you test these?
> + /// # Ok::<(), Error>(())
> + /// ```
> + pub fn display_file<T: Display + Sized>(
> + &self,
> + name: &CStr,
> + data: &'static T,
> + ) -> Result<ARef<Self>> {
Again, will always succeed, you don't want any checking of Result<>
thanks,
greg k-h
On Wed, Apr 30, 2025 at 5:06 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Apr 29, 2025 at 11:15:56PM +0000, Matthew Maurer wrote:
> > + /// Create a file in a DebugFS directory with the provided name, and contents from invoking
> > + /// [`Display::fmt`] on the provided reference.
> > + ///
> > + /// # Example
> > + ///
> > + /// ```
> > + /// # use kernel::c_str;
> > + /// # use kernel::debugfs::Dir;
> > + /// let dir = Dir::new(c_str!("my_debugfs_dir"), None)?;
>
> Again, can never fail, so don't check it.
>
> > + /// let file = dir.display_file(c_str!("foo"), &200)?;
>
> Again, can never fail, so don't check it.
>
> And "200" is wrong, did you test these?
How is 200 wrong? This displays the number "200" in a file called
"foo" in the directory "my_debugfs_dir"?
>
>
> > + /// # Ok::<(), Error>(())
> > + /// ```
> > + pub fn display_file<T: Display + Sized>(
> > + &self,
> > + name: &CStr,
> > + data: &'static T,
> > + ) -> Result<ARef<Self>> {
>
> Again, will always succeed, you don't want any checking of Result<>
>
> thanks,
>
> greg k-h
On Wed, Apr 30, 2025 at 08:12:09AM -0700, Matthew Maurer wrote:
> On Wed, Apr 30, 2025 at 5:06 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Apr 29, 2025 at 11:15:56PM +0000, Matthew Maurer wrote:
> > > + /// Create a file in a DebugFS directory with the provided name, and contents from invoking
> > > + /// [`Display::fmt`] on the provided reference.
> > > + ///
> > > + /// # Example
> > > + ///
> > > + /// ```
> > > + /// # use kernel::c_str;
> > > + /// # use kernel::debugfs::Dir;
> > > + /// let dir = Dir::new(c_str!("my_debugfs_dir"), None)?;
> >
> > Again, can never fail, so don't check it.
> >
> > > + /// let file = dir.display_file(c_str!("foo"), &200)?;
> >
> > Again, can never fail, so don't check it.
> >
> > And "200" is wrong, did you test these?
>
> How is 200 wrong? This displays the number "200" in a file called
> "foo" in the directory "my_debugfs_dir"?
Oops, sorry, I thought that was the "mode" of the file being created :)
greg k-h
Hi Matthew, Since I was playing around with this series, I noticed Clippy complained about a few things (please see `make ..... CLIPPY=1`): On Wed, Apr 30, 2025 at 1:16 AM Matthew Maurer <mmaurer@google.com> wrote: > > + 0444, This is decimal rather than octal. > +// # Safety > +// * seq must point to a live seq_file whose private data is a live pointer to a T which is not > +// being mutated. This should be `///` (also, since I am here, please use Markdown etc.). There are a couple other classes of lints that Clippy mentions that should be cleaned up or allowed elsewhere. Thanks! Cheers, Miguel
On Tue, Apr 29, 2025 at 8:27 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > Hi Matthew, > > Since I was playing around with this series, I noticed Clippy > complained about a few things (please see `make ..... CLIPPY=1`): > > On Wed, Apr 30, 2025 at 1:16 AM Matthew Maurer <mmaurer@google.com> wrote: > > > > + 0444, > > This is decimal rather than octal. Oops, will-fix. > > > +// # Safety > > +// * seq must point to a live seq_file whose private data is a live pointer to a T which is not > > +// being mutated. > > This should be `///` (also, since I am here, please use Markdown etc.). I thought I used markdown everywhere, I'll do an extra pass before I upload the next version. > > There are a couple other classes of lints that Clippy mentions that > should be cleaned up or allowed elsewhere. It looks like when I transitioned over from running on a physical board to running on an emulator I typo'd my clippy-check script, sorry about that. I've got the errors confirmed coming out now, so I'll be clean there next time. > > Thanks! > > Cheers, > Miguel
© 2016 - 2026 Red Hat, Inc.