Skip to contentSkip to conversation

rust: DebugFS Bindings

PATCH Superseded Matthew Maurer 4 participants 31 messages Apr 29, 2025 – Apr 30, 2025

Subsystems (2)
Driver core Rust

v1 of 11 Superseded Latest is v11 →

lore
Share & tools
lore

Matthew Maurer

author
#0 lore raw

cover rust: DebugFS Bindings

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

The primary intended way to use these bindings is to attach a data object to a DebugFS directory handle, and use the contents of that attached object to serve what's inside the directory. Through a combination of pinning and equivariance, we can ensure that the pointers in the attached object will remain valid until after the DebugFS directory is made inaccessible.

Signed-off-by Matthew Maurer <[email protected]>

---
Matthew Maurer (8):
rust: debugfs: Bind DebugFS directory creation
rust: debugfs: Bind file creation for long-lived Display
rust: debugfs: Add scoped builder interface
rust: debugfs: Allow subdir creation in builder interface
rust: debugfs: Support format hooks
rust: debugfs: Implement display_file in terms of fmt_file
rust: debugfs: Helper macro for common case implementations
rust: samples: Add debugfs sample

 MAINTAINERS                     |   2 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/dcache.c           |  12 +
 rust/helpers/helpers.c          |   1 +
 rust/kernel/debugfs.rs          | 514 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 samples/rust/Kconfig            |  11 +
 samples/rust/Makefile           |   1 +
 samples/rust/rust_debugfs.rs    | 120 ++++++++++
 9 files changed, 663 insertions(+)

---
base-commit: b6a7783d306baf3150ac54cd5124f6e85dd375b0
change-id: 20250428-debugfs-rust-3cd5c97eb7d1

Best regards,

Signature · 1 line
Matthew Maurer <[email protected]>
author
#1 lore raw

PATCH 1/8 rust: debugfs: Bind DebugFS directory creation

The basic API relies on dput to prevent leaks. Use of debugfs_remove is delayed until the more full-featured API, because we need to avoid the user having an reference to a dir that is recursively removed.

Signed-off-by Matthew Maurer <[email protected]>
---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/dcache.c           |  12 +++++
 rust/helpers/helpers.c          |   1 +
 rust/kernel/debugfs.rs          | 100 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 6 files changed, 116 insertions(+)
MAINTAINERS +1 -0
index 906881b6c5cb6ff743e13b251873b89138c69a1c..a3b835e427b083a4ddd690d9e7739851f0af47ae 100644
@@ -7271,6 +7271,7 @@ 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/device.rs
 F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
rust/bindings/bindings_helper.h +1 -0
index 8a2add69e5d66d1c2ebed9d2c950380e61c48842..787f928467faabd02a7f3cf041378fac856c4f89 100644
@@ -13,6 +13,7 @@
 #include <linux/blkdev.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>
rust/helpers/dcache.c new file +12 -0
new file mode 100644
index 0000000000000000000000000000000000000000..2396cdaa89a95a2be69fd84ec205e0f5f1b63f0c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2025 Google LLC.
+ */
+
+#include <linux/dcache.h>
+
+struct dentry *rust_helper_dget(struct dentry *d)
+{
+	return dget(d);
+}
rust/helpers/helpers.c +1 -0
index f34320e6d1f2fb56cc151ee2ffe5d331713fd36a..95f486c1175191483297b7140b99f1aa364c081c 100644
@@ -15,6 +15,7 @@
 #include "cpumask.c"
 #include "cred.c"
 #include "device.c"
+#include "dcache.c"
 #include "dma.c"
 #include "err.c"
 #include "fs.c"
rust/kernel/debugfs.rs new file +100 -0

100 lines not on this page · raw message

rust/kernel/lib.rs +1 -0
index c3762e80b314316b4b0cee3bfd9442f8f0510b91..86f6055b828d5f711578293d8916a517f2436977 100644
@@ -45,6 +45,7 @@
 #[doc(hidden)]
 pub mod build_assert;
 pub mod cred;
+pub mod debugfs;
 pub mod device;
 pub mod device_id;
 pub mod devres;
Signature · 1 line
2.49.0.901.g37484f566f-goog
maintainer
#2 lore raw

Re: [PATCH 1/8] rust: debugfs: Bind DebugFS directory creation

Quoted 5 lines · Matthew Maurer

The basic API relies on dput to prevent leaks. Use of debugfs_remove is delayed until the more full-featured API, because we need to avoid the user having an reference to a dir that is recursively removed.

Signed-off-by Matthew Maurer <[email protected]>

First off, many thanks for doing this. I like this in general, but I have loads of specific questions/comments. Don't take that as a criticism of this feature, I really want these bindings to be in the tree and work hopefully better/cleaner than the userspace ones do.

First off, the main "rule" of debugfs is that you should NEVER care about the return value of any debugfs function. So much so that the C side hides errors almost entirely where possible. I'd like to see this carried through to the Rust side as well, but I think you didn't do that for various "traditional" reasons.

Please try to unwind all of that in your next version, I'll point this out below in one place:

Quoted 126 lines
---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/dcache.c           |  12 +++++
 rust/helpers/helpers.c          |   1 +
 rust/kernel/debugfs.rs          | 100 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 6 files changed, 116 insertions(+)
MAINTAINERS +1 -0
index 906881b6c5cb6ff743e13b251873b89138c69a1c..a3b835e427b083a4ddd690d9e7739851f0af47ae 100644
@@ -7271,6 +7271,7 @@ 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/device.rs
 F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
rust/bindings/bindings_helper.h +1 -0
index 8a2add69e5d66d1c2ebed9d2c950380e61c48842..787f928467faabd02a7f3cf041378fac856c4f89 100644
@@ -13,6 +13,7 @@
 #include <linux/blkdev.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>
rust/helpers/dcache.c new file +12 -0
new file mode 100644
index 0000000000000000000000000000000000000000..2396cdaa89a95a2be69fd84ec205e0f5f1b63f0c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2025 Google LLC.
+ */
+
+#include <linux/dcache.h>
+
+struct dentry *rust_helper_dget(struct dentry *d)
+{
+	return dget(d);
+}
rust/helpers/helpers.c +1 -0
index f34320e6d1f2fb56cc151ee2ffe5d331713fd36a..95f486c1175191483297b7140b99f1aa364c081c 100644
@@ -15,6 +15,7 @@
 #include "cpumask.c"
 #include "cred.c"
 #include "device.c"
+#include "dcache.c"
 #include "dma.c"
 #include "err.c"
 #include "fs.c"
rust/kernel/debugfs.rs new file +57 -0
new file mode 100644
index 0000000000000000000000000000000000000000..4d06cce7099607f95b684bad329f791a815d3e86
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! DebugFS Abstraction
+//!
+//! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
+
+use crate::error::{from_err_ptr, Result};
+use crate::str::CStr;
+use crate::types::{ARef, AlwaysRefCounted, Opaque};
+use core::ptr::NonNull;
+
+/// Handle to a DebugFS directory.
+pub struct Dir {
+    inner: Opaque<bindings::dentry>,
+}
+
+// SAFETY: Dir is just a `dentry` under the hood, which the API promises can be transferred
+// between threads.
+unsafe impl Send for Dir {}
+
+// SAFETY: All the native functions we re-export use interior locking, and the contents of the
+// struct are opaque to Rust.
+unsafe impl Sync for Dir {}
+
+// SAFETY: Dir is actually `dentry`, and dget/dput are the reference counting functions
+// for it.
+unsafe impl AlwaysRefCounted for Dir {
+    #[inline]
+    fn inc_ref(&self) {
+        // SAFETY: Since we have a reference to the directory,
+        // it's live, so it's safe to call dget on it.
+        unsafe {
+            kernel::bindings::dget(self.as_ptr());
+        }
+    }
+    #[inline]
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: By the caller precondition on the trait, we know that the caller has a reference
+        // count to the object.
+        unsafe {
+            kernel::bindings::dput(obj.cast().as_ptr());
+        }
+    }
+}
+
+impl Dir {
+    /// Create a new directory in DebugFS. If `parent` is [`None`], it will be created at the root.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// {
+    ///     let dir = Dir::new(c_str!("my_debug_dir"), None)?;

rust/kernel/debugfs.rs line 67

Quoted 10 lines
rust/kernel/debugfs.rs new file +10 -0
+    ///     // The directory will exist in DebugFS here.
+    /// }
+    /// // The directory will no longer exist in DebugFS here.
+    /// # Ok::<(), Error>(())
+    /// ```
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let parent = Dir::new(c_str!("parent"), None)?;

Why are you checking this? You shouldn't, it should never return an error. What it returns you can ALWAYS pass back into any call that expects to get a "debugfs::Dir" object. You should never even be able to know if it succeeded or not as your code should never do anything different depending on the result.

rust/kernel/debugfs.rs line 70

Quoted 3 lines
rust/kernel/debugfs.rs new file +3 -0
+    /// let child = Dir::new(c_str!("child"), Some(&parent))?;
+    /// // parent/child exists in DebugFS here.
+    /// drop(parent);

Wait, what? Why would an explicit drop(parent) be required here? That feels wrong, and way too complex. Why can't you rely on the child creation to properly manage this if needed (hint, it shouldn't be.)

rust/kernel/debugfs.rs line 72

Quoted 2 lines
rust/kernel/debugfs.rs new file +2 -0
+    /// // The child dentry is still valid here, but DebugFS will have neither directory.
+    /// # Ok::<(), Error>(())

Again, no error checking please.

Quoted 12 lines
rust/kernel/debugfs.rs new file +12 -0
+    /// ```
+    pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> {
+        let parent_ptr = match parent {
+            Some(parent) => parent.as_ptr(),
+            None => core::ptr::null_mut(),
+        };
+        // SAFETY:
+        // * name argument points to a null terminated string that lives across the call, by
+        //   invariants of &CStr
+        // * If parent is None, parent accepts null pointers to mean create at root
+        // * If parent is Some, parent accepts live dentry debugfs pointers
+        // * `debugfs_create_dir` either returns an error code or a legal `dentry` pointer,

rust/kernel/debugfs.rs line 94

Quoted 10 lines
rust/kernel/debugfs.rs new file +10 -0
+        //   so we can call `NonNull::new_unchecked`.
+        let dir = unsafe {
+            NonNull::new_unchecked(from_err_ptr(kernel::bindings::debugfs_create_dir(
+                name.as_char_ptr(),
+                parent_ptr,
+            ))?)
+        };
+        // 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(dir.cast()) })

Why do you want this wrapped? And I think you "wrapped" it wrong here. When the object is "gone" it should have called debugfs_remove_recursive(), not dput(), in order to properly drop everything. Where is that call happening?

The debugfs core already handles the reference counting of this object, please don't add some extra reference count calls with dput/get, that's just going to be a mess.

You should NEVER be encoding the fact that the return value of a debugfs_*() call is a dentry. Just treat that as an opaque pointer that just happens to have a common name that might refer to something else. That pointer can be passed back into other debugfs functions, and THAT IS IT. No checking for it, no passing it to any vfs functions, or anything else.

So the dput() stuff here should not be present at all (hint, no C code that used debugfs ever calls that, so the rust bindings shouldn't either.)

thanks,

greg k-h

author
#3 lore raw

Re: [PATCH 1/8] rust: debugfs: Bind DebugFS directory creation

Quoted 13 lines · Greg Kroah-Hartman
Quoted 5 lines · Matthew Maurer

The basic API relies on dput to prevent leaks. Use of debugfs_remove is delayed until the more full-featured API, because we need to avoid the user having an reference to a dir that is recursively removed.

Signed-off-by Matthew Maurer <[email protected]>

First off, many thanks for doing this. I like this in general, but I have loads of specific questions/comments. Don't take that as a criticism of this feature, I really want these bindings to be in the tree and work hopefully better/cleaner than the userspace ones do.

Quoted 5 lines

First off, the main "rule" of debugfs is that you should NEVER care about the return value of any debugfs function. So much so that the C side hides errors almost entirely where possible. I'd like to see this carried through to the Rust side as well, but I think you didn't do that for various "traditional" reasons.

Sure, I mostly had to do error checking because I was using an ARef<Dir> to represent a directory, which meant that the underlying directory needed to be a valid pointer. Given that you've said that the returned dentry * should never be used as an actual dentry, only as an abstract handle to a DebugFS object, that requirement goes away, and I can remove the error handling code and always successfully return a Dir, even in cases where an error has occurred.

Quoted 4 lines

Wait, what? Why would an explicit drop(parent) be required here? That feels wrong, and way too complex. Why can't you rely on the child creation to properly manage this if needed (hint, it shouldn't be.)

The explicit drop is not required for normal usage, it was intended to be illustrative - I was trying to explain what the semantics would be if the parent dentry was released before the child was. As before, now that the child will not be an ARef<Dir>, and so not assumed to be a valid dentry pointer, this example won't be relevant anymore.

Quoted 17 lines

Why do you want this wrapped? And I think you "wrapped" it wrong here. When the object is "gone" it should have called debugfs_remove_recursive(), not dput(), in order to properly drop everything. Where is that call happening?

The debugfs core already handles the reference counting of this object, please don't add some extra reference count calls with dput/get, that's just going to be a mess.

You should NEVER be encoding the fact that the return value of a debugfs_*() call is a dentry. Just treat that as an opaque pointer that just happens to have a common name that might refer to something else. That pointer can be passed back into other debugfs functions, and THAT IS IT. No checking for it, no passing it to any vfs functions, or anything else.

Quoted 4 lines

So the dput() stuff here should not be present at all (hint, no C code that used debugfs ever calls that, so the rust bindings shouldn't either.)

I incorrectly assumed that if a dentry * was being returned, then at some point it would be used as a dentry * rather than as a debugfs-only handle. I bound it into an ARef to avoid backing myself into a corner later if someone needed to use the dentry * for something, but if that's against the API contract of DebugFS, I can avoid it entirely.

maintainer
#4 lore raw

Re: [PATCH 1/8] rust: debugfs: Bind DebugFS directory creation

Quoted 21 lines · Matthew Maurer
Quoted 13 lines · Greg Kroah-Hartman
Quoted 5 lines · Matthew Maurer

The basic API relies on dput to prevent leaks. Use of debugfs_remove is delayed until the more full-featured API, because we need to avoid the user having an reference to a dir that is recursively removed.

Signed-off-by Matthew Maurer <[email protected]>

First off, many thanks for doing this. I like this in general, but I have loads of specific questions/comments. Don't take that as a criticism of this feature, I really want these bindings to be in the tree and work hopefully better/cleaner than the userspace ones do.

Quoted 5 lines

First off, the main "rule" of debugfs is that you should NEVER care about the return value of any debugfs function. So much so that the C side hides errors almost entirely where possible. I'd like to see this carried through to the Rust side as well, but I think you didn't do that for various "traditional" reasons.

Quoted 7 lines

Sure, I mostly had to do error checking because I was using an ARef<Dir> to represent a directory, which meant that the underlying directory needed to be a valid pointer. Given that you've said that the returned dentry * should never be used as an actual dentry, only as an abstract handle to a DebugFS object, that requirement goes away, and I can remove the error handling code and always successfully return a Dir, even in cases where an error has occurred.

Great!

Except when debugfs is not enabled, then what are you going to return? The same structure, or an error?

I'd vote for the same pointer to the structure, just to make it more obvious that this is a totally opaque thing and that no caller should ever know or care if debugfs is working or even present in the system.

Note that some drivers will want to save a bit of space if debugfs is not enabled in the build, so be prepared to make the binding work somehow that way too. Can you have an "empty" object that takes no memory? Or is this too overthinking things?

Quoted 10 lines
Quoted 3 lines

Wait, what? Why would an explicit drop(parent) be required here? That feels wrong, and way too complex. Why can't you rely on the child creation to properly manage this if needed (hint, it shouldn't be.)

The explicit drop is not required for normal usage, it was intended to be illustrative - I was trying to explain what the semantics would be if the parent dentry was released before the child was. As before, now that the child will not be an ARef<Dir>, and so not assumed to be a valid dentry pointer, this example won't be relevant anymore.

Great!

thanks, hopefully this should make things simpler.

greg k-h

author
#5 lore raw

Re: [PATCH 1/8] rust: debugfs: Bind DebugFS directory creation

Quoted 36 lines · Greg Kroah-Hartman
Quoted 21 lines · Matthew Maurer
Quoted 13 lines · Greg Kroah-Hartman
Quoted 5 lines · Matthew Maurer

The basic API relies on dput to prevent leaks. Use of debugfs_remove is delayed until the more full-featured API, because we need to avoid the user having an reference to a dir that is recursively removed.

Signed-off-by Matthew Maurer <[email protected]>

First off, many thanks for doing this. I like this in general, but I have loads of specific questions/comments. Don't take that as a criticism of this feature, I really want these bindings to be in the tree and work hopefully better/cleaner than the userspace ones do.

Quoted 5 lines

First off, the main "rule" of debugfs is that you should NEVER care about the return value of any debugfs function. So much so that the C side hides errors almost entirely where possible. I'd like to see this carried through to the Rust side as well, but I think you didn't do that for various "traditional" reasons.

Quoted 7 lines

Sure, I mostly had to do error checking because I was using an ARef<Dir> to represent a directory, which meant that the underlying directory needed to be a valid pointer. Given that you've said that the returned dentry * should never be used as an actual dentry, only as an abstract handle to a DebugFS object, that requirement goes away, and I can remove the error handling code and always successfully return a Dir, even in cases where an error has occurred.

Great!

Except when debugfs is not enabled, then what are you going to return? The same structure, or an error?

Quoted 8 lines

I'd vote for the same pointer to the structure, just to make it more obvious that this is a totally opaque thing and that no caller should ever know or care if debugfs is working or even present in the system.

Note that some drivers will want to save a bit of space if debugfs is not enabled in the build, so be prepared to make the binding work somehow that way too. Can you have an "empty" object that takes no memory? Or is this too overthinking things?

Based on what you've expressed, I think what makes sense is:

* Initial patch will always return the same Dir, just sometimes it will be a wrapper around a pointer that is an error code, and sometimes it will be a useful dentry pointer. This will match the current behavior of C code to my understanding.
* Follow-up (probably still in this series) will check CONFIG_DEBUG_FS, and if it's off, will just make Dir into a ZST, and just discard the pointer. This would be an improvement upon the C interface, because drivers would get the shrinkage without needing to add conditionals on CONFIG_DEBUG_FS in their own driver.

Quoted 17 lines
Quoted 11 lines
Quoted 4 lines

Wait, what? Why would an explicit drop(parent) be required here? That feels wrong, and way too complex. Why can't you rely on the child creation to properly manage this if needed (hint, it shouldn't be.)

The explicit drop is not required for normal usage, it was intended to be illustrative - I was trying to explain what the semantics would be if the parent dentry was released before the child was. As before, now that the child will not be an ARef<Dir>, and so not assumed to be a valid dentry pointer, this example won't be relevant anymore.

Great!

thanks, hopefully this should make things simpler.

greg k-h

maintainer
#6 lore raw

Re: [PATCH 1/8] rust: debugfs: Bind DebugFS directory creation

Quoted 47 lines · Matthew Maurer
Quoted 36 lines · Greg Kroah-Hartman
Quoted 21 lines · Matthew Maurer
Quoted 13 lines · Greg Kroah-Hartman
Quoted 5 lines · Matthew Maurer

The basic API relies on dput to prevent leaks. Use of debugfs_remove is delayed until the more full-featured API, because we need to avoid the user having an reference to a dir that is recursively removed.

Signed-off-by Matthew Maurer <[email protected]>

First off, many thanks for doing this. I like this in general, but I have loads of specific questions/comments. Don't take that as a criticism of this feature, I really want these bindings to be in the tree and work hopefully better/cleaner than the userspace ones do.

Quoted 5 lines

First off, the main "rule" of debugfs is that you should NEVER care about the return value of any debugfs function. So much so that the C side hides errors almost entirely where possible. I'd like to see this carried through to the Rust side as well, but I think you didn't do that for various "traditional" reasons.

Quoted 7 lines

Sure, I mostly had to do error checking because I was using an ARef<Dir> to represent a directory, which meant that the underlying directory needed to be a valid pointer. Given that you've said that the returned dentry * should never be used as an actual dentry, only as an abstract handle to a DebugFS object, that requirement goes away, and I can remove the error handling code and always successfully return a Dir, even in cases where an error has occurred.

Great!

Except when debugfs is not enabled, then what are you going to return? The same structure, or an error?

Quoted 8 lines

I'd vote for the same pointer to the structure, just to make it more obvious that this is a totally opaque thing and that no caller should ever know or care if debugfs is working or even present in the system.

Note that some drivers will want to save a bit of space if debugfs is not enabled in the build, so be prepared to make the binding work somehow that way too. Can you have an "empty" object that takes no memory? Or is this too overthinking things?

Quoted 6 lines

Based on what you've expressed, I think what makes sense is:

* Initial patch will always return the same Dir, just sometimes it will be a wrapper around a pointer that is an error code, and sometimes it will be a useful dentry pointer. This will match the current behavior of C code to my understanding.

Great.

Quoted 5 lines

* Follow-up (probably still in this series) will check CONFIG_DEBUG_FS, and if it's off, will just make Dir into a ZST, and just discard the pointer. This would be an improvement upon the C interface, because drivers would get the shrinkage without needing to add conditionals on CONFIG_DEBUG_FS in their own driver.

You're going to have to check CONFIG_DEBUG_FS anyway for this series, otherwise things aren't going to build properly, so this sounds like a great way to handle that.

thanks,

greg k-h

author
#7 lore raw

PATCH 2/8 rust: debugfs: Bind file creation for long-lived Display

In reply to Matthew Maurer

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 <[email protected]>
---
 rust/kernel/debugfs.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)
rust/kernel/debugfs.rs +94 -0
index 4d06cce7099607f95b684bad329f791a815d3e86..b20df5fce692b3047c804f7ad5af90fc4248979b 100644
@@ -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 {}
Signature · 1 line
2.49.0.901.g37484f566f-goog
#8 lore raw

Re: [PATCH 2/8] rust: debugfs: Bind file creation for long-lived Display

Hi Matthew,

Since I was playing around with this series, I noticed Clippy complained about a few things (please see make ..... CLIPPY=1):

Quoted 2 lines · Matthew Maurer

+ 0444,

This is decimal rather than octal.

rust/kernel/debugfs.rs pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> { · line 168

Quoted 3 lines
rust/kernel/debugfs.rs +3 -0
+// # 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

author
#9 lore raw

Re: [PATCH 2/8] rust: debugfs: Bind file creation for long-lived Display

Quoted 11 lines · Miguel Ojeda

Hi Matthew,

Since I was playing around with this series, I noticed Clippy complained about a few things (please see make ..... CLIPPY=1):

Quoted 2 lines · Matthew Maurer

+ 0444,

This is decimal rather than octal.

Oops, will-fix.

Quoted 6 lines

rust/kernel/debugfs.rs pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> { · line 168

Quoted 4 lines
rust/kernel/debugfs.rs +3 -0
+// # 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.

Quoted 3 lines

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.

Quoted 5 lines

Thanks!

Cheers,
Miguel

maintainer
#10 lore raw

Re: [PATCH 2/8] rust: debugfs: Bind file creation for long-lived Display

In reply to Matthew Maurer
Quoted 2 lines · Matthew Maurer

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.

Quoted 4 lines

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?

Quoted 3 lines

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

author
#11 lore raw

Re: [PATCH 2/8] rust: debugfs: Bind file creation for long-lived Display

Quoted 7 lines · Greg Kroah-Hartman
Quoted 2 lines · Matthew Maurer

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.

Quoted 7 lines
Quoted 5 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 4 lines

thanks,

greg k-h

maintainer
#12 lore raw

Re: [PATCH 2/8] rust: debugfs: Bind file creation for long-lived Display

In reply to Matthew Maurer

rust/kernel/debugfs.rs pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> { · line 112

Quoted 9 lines · Matthew Maurer
rust/kernel/debugfs.rs +9 -0
+    /// 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.

Quoted 1 line

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

rust/kernel/debugfs.rs pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> { · line 120

Quoted 7 lines
rust/kernel/debugfs.rs +7 -0
+    /// # 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

author
#13 lore raw

Re: [PATCH 2/8] rust: debugfs: Bind file creation for long-lived Display

Quoted 12 lines · Greg Kroah-Hartman

rust/kernel/debugfs.rs pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> { · line 112

Quoted 9 lines · Matthew Maurer
rust/kernel/debugfs.rs +9 -0
+    /// 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)?;
Quoted 7 lines

Again, can never fail, so don't check it.

Quoted 1 line

+ /// 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"?

Quoted 15 lines

rust/kernel/debugfs.rs pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> { · line 120

Quoted 9 lines
rust/kernel/debugfs.rs +7 -0
+    /// # 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

maintainer
#14 lore raw

Re: [PATCH 2/8] rust: debugfs: Bind file creation for long-lived Display

Quoted 16 lines · Matthew Maurer
Quoted 14 lines · Greg Kroah-Hartman

rust/kernel/debugfs.rs pub fn new(name: &CStr, parent: Option<&Self>) -> Result<ARef<Self>> { · line 112

Quoted 9 lines · Matthew Maurer
rust/kernel/debugfs.rs +9 -0
+    /// 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.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

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

author
#15 lore raw

PATCH 3/8 rust: debugfs: Add scoped builder interface

In reply to Matthew Maurer

This adds an interface which allows access to references which may not live indefinitely by forcing them to live at least as long as the DebugFS directory itself.

Signed-off-by Matthew Maurer <[email protected]>
---
 rust/kernel/debugfs.rs | 163 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)
rust/kernel/debugfs.rs +163 -0
index b20df5fce692b3047c804f7ad5af90fc4248979b..f6240fd056f8598d5ef06bdaf61c5c33eab5b734 100644
@@ -12,7 +12,12 @@
 use crate::str::CStr;
 use crate::types::{ARef, AlwaysRefCounted, Opaque};
 use core::fmt::Display;
+use core::marker::{PhantomData, PhantomPinned};
+use core::mem::ManuallyDrop;
+use core::ops::Deref;
+use core::pin::Pin;
 use core::ptr::NonNull;
+use pin_init::{pin_data, pinned_drop, PinInit};
 
 /// Handle to a DebugFS directory.
 pub struct Dir {

@@ -117,6 +122,22 @@ pub fn display_file<T: Display + Sized>(
         &self,
         name: &CStr,
         data: &'static T,
+    ) -> Result<ARef<Self>> {
+        // SAFETY: As `data` lives for the static lifetime, it outlives the file.
+        unsafe { self.display_file_raw(name, data) }
+    }
+
+    /// Creates a DebugFS file backed by the display implementation of the provided pointer.
+    ///
+    /// # Safety
+    /// The pointee of `data` must outlive the accessibility of the `Dir` returned by this function.
+    /// 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<T: Display + Sized>(
+        &self,
+        name: &CStr,
+        data: *const T,
     ) -> Result<ARef<Self>> {
         // SAFETY:
         // * `name` is a NUL-terminated C string, living across the call, by CStr invariant

@@ -192,3 +213,145 @@ trait DisplayFile: Display + Sized {
 }
 
 impl<T: Display + Sized> DisplayFile for T {}
+
+#[pin_data(PinnedDrop)]
+/// A DebugFS directory combined with a backing store for data to implement it
+pub struct Values<T> {
+    #[pin]
+    backing: T,
+    // Calling `debugfs_remove`, as we will do in our `Drop` impl, will consume the refcount we
+    // hold after cleaning up all the child directories.
+    dir: ManuallyDrop<ARef<Dir>>,
+    // 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. When the resulting object is destroyed, the
+    /// DebugFS directory will be recursively removed as well.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"), None)?;
+    /// let _foo = KBox::pin_init(Values::attach(0, dir), GFP_KERNEL)?;
+    /// // foo can now be used with `Values::build` to allow access to the attached value when
+    /// // printing
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn attach(backing: impl PinInit<T>, dir: ARef<Dir>) -> impl PinInit<Self> {
+        pin_init::pin_init! { Self {
+            backing <- backing,
+            dir: ManuallyDrop::new(dir),
+            _pin: PhantomPinned,
+        } }
+    }
+
+    /// Runs a closure which has access to the backing data and a builder that will allow you to
+    /// build a DebugFS structure off the backing data using its methods.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"), None)?;
+    /// let foo = KBox::pin_init(Values::attach(0, dir), GFP_KERNEL)?;
+    /// foo.as_ref().build(|_value, _builder| {
+    ///   // Construct debugfs with access to the attached value here
+    /// });
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn build<U, F: for<'a> FnOnce(&'a T, Builder<'a>) -> U>(self: Pin<&Self>, f: F) -> U {
+        // SAFETY: The Builder produced here is technically at the lifetime of self, but is
+        // being used only in a universal context, so that information is immediately erased and
+        // replaced with the universally quantified 'a. By taking a Pin<&Self>, we enforce that
+        // self.backing remains alive for any 'a less than the lifetime of the struct. By not
+        // providing any mutable access to self.backing, we ensure that it's always safe to
+        // materialize a read-only reference to &self.backing for any 'a less than the lifetime of
+        // the struct.
+        f(&self.backing, unsafe { Builder::new(&self.dir) })
+    }
+}
+
+#[pinned_drop]
+impl<T> PinnedDrop for Values<T> {
+    fn drop(self: Pin<&mut Self>) {
+        // SAFETY: Our internal dir holds its own reference count, so it's always valid.
+        unsafe { kernel::bindings::debugfs_remove(self.dir.as_ptr()) }
+    }
+}
+
+/// A Dir, scoped to the lifetime for which it will exist. Unlike `&'a Dir`, this is equivariant,
+/// preventing the shortening of the lifetime.
+///
+/// # Invariants
+/// Builder will only ever be used with 'static or a universally quantified lifetime that is
+/// unified only with the lifetime of data structures guaranteed to outlive it and not have mutable
+/// references taken.
+#[repr(transparent)]
+#[derive(Copy, Clone)]
+pub struct Builder<'a> {
+    inner: &'a Dir,
+    _equivariant: PhantomData<fn(&'a ()) -> &'a ()>,
+}
+
+impl<'a> Builder<'a> {
+    /// # Safety
+    /// Caller must promise to use this function at static lifetime or only expose it to
+    /// universally quantified functions, unified only with lifetimes guaranteed to extend beyond
+    /// when the directory will be rendered inaccessible.
+    unsafe fn new(inner: &'a Dir) -> Self {
+        Self {
+            inner,
+            _equivariant: PhantomData,
+        }
+    }
+
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking
+    /// [`Display::fmt`] on the provided reference.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::{try_pin_init, c_str};
+    /// # use pin_init::stack_try_pin_init;
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"), None)?;
+    /// stack_try_pin_init!(let foo =? Values::attach((1, 2), dir));
+    /// foo.as_ref().build(|value, builder| {
+    ///   builder.display_file(c_str!("bar"), &value.0)?;
+    ///   builder.display_file(c_str!("baz"), &value.1)
+    /// })?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Result<()> {
+        // We forget the reference because its reference count is implicitly "owned" by the root
+        // builder, which we know will use `debugfs_remove` to clean this up. If we release the
+        // file here, it will be immediately deleted.
+        // SAFETY:
+        // Because `Builder`'s invariant says that our lifetime is how long the directory will
+        // be available, and is equivariant, `'a` will outlive the base directory, which will be
+        // torn down by `debugfs_remove` to prevent access even if an extra refcount is held
+        // somewhere.
+        core::mem::forget(unsafe { self.inner.display_file_raw(name, data)? });
+        Ok(())
+    }
+}
+
+impl<'a> Deref for Builder<'a> {
+    type Target = Dir;
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
Signature · 1 line
2.49.0.901.g37484f566f-goog
maintainer
#16 lore raw

Re: [PATCH 3/8] rust: debugfs: Add scoped builder interface

Quoted 3 lines · Matthew Maurer

This adds an interface which allows access to references which may not live indefinitely by forcing them to live at least as long as the DebugFS directory itself.

They can ONLY live as long as the debugfs directory lives, is that what you mean here?

Quoted 29 lines
Signed-off-by Matthew Maurer <[email protected]>
---
 rust/kernel/debugfs.rs | 163 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)
rust/kernel/debugfs.rs +7 -0
index b20df5fce692b3047c804f7ad5af90fc4248979b..f6240fd056f8598d5ef06bdaf61c5c33eab5b734 100644
@@ -12,7 +12,12 @@
 use crate::str::CStr;
 use crate::types::{ARef, AlwaysRefCounted, Opaque};
 use core::fmt::Display;
+use core::marker::{PhantomData, PhantomPinned};
+use core::mem::ManuallyDrop;
+use core::ops::Deref;
+use core::pin::Pin;
 use core::ptr::NonNull;
+use pin_init::{pin_data, pinned_drop, PinInit};
 
 /// Handle to a DebugFS directory.
 pub struct Dir {

@@ -117,6 +122,22 @@ pub fn display_file<T: Display + Sized>(
         &self,
         name: &CStr,
         data: &'static T,
+    ) -> Result<ARef<Self>> {
+        // SAFETY: As `data` lives for the static lifetime, it outlives the file.

rust/kernel/debugfs.rs pub fn display_file<T: Display + Sized>( · line 136

Quoted 10 lines
rust/kernel/debugfs.rs +10 -0
+        unsafe { self.display_file_raw(name, data) }
+    }
+
+    /// Creates a DebugFS file backed by the display implementation of the provided pointer.
+    ///
+    /// # Safety
+    /// The pointee of `data` must outlive the accessibility of the `Dir` returned by this function.
+    /// 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`

Perhaps the refcount should be the thing driving debugfs_remove()? That might save you lots of mess overall I think.

thanks,

greg k-h

author
#17 lore raw

PATCH 4/8 rust: debugfs: Allow subdir creation in builder interface

In reply to Matthew Maurer

Allows creating subdirectories in the builder intended to be cleaned up by debugfs_remove at the same time as the root.

Signed-off-by Matthew Maurer <[email protected]>
---
 rust/kernel/debugfs.rs | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
rust/kernel/debugfs.rs +26 -0
index f6240fd056f8598d5ef06bdaf61c5c33eab5b734..6c7cf7e97741b98d2c0654d01fca3de0d8047e97 100644
@@ -347,6 +347,32 @@ pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Resu
         core::mem::forget(unsafe { self.inner.display_file_raw(name, data)? });
         Ok(())
     }
+
+    /// Creates a nested directory that may live as long as its parent
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"), None)?;
+    /// let foo = KBox::pin_init(Values::attach(0, dir), GFP_KERNEL)?;
+    /// foo.as_ref().build(|_value, builder| {
+    ///   builder.dir(c_str!("bar"))?;
+    ///   Ok::<(), Error>(())
+    /// })?;
+    /// // foo/bar will still exist at this point in DebugFS
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn dir(&self, name: &CStr) -> Result<Builder<'a>> {
+        let dir = Dir::new(name, Some(self))?;
+        // SAFETY: We're suppressing the drop of the ARef we received, so we know it will live
+        // until its parent is `debugfs_remove`'d. The lifetime of the parent is 'a, so we can
+        // convert it to a similarly lived reference.
+        let dir: &'a Dir = unsafe { ARef::into_raw(dir).as_ref() };
+        // SAFETY: Since 'a is a builder lifetime, we can propagate our invariants
+        Ok(unsafe { Builder::new(dir) })
+    }
 }
 
 impl<'a> Deref for Builder<'a> {
Signature · 1 line
2.49.0.901.g37484f566f-goog
maintainer
#18 lore raw

Re: [PATCH 4/8] rust: debugfs: Allow subdir creation in builder interface

Quoted 13 lines · Matthew Maurer

Allows creating subdirectories in the builder intended to be cleaned up by debugfs_remove at the same time as the root.

Signed-off-by Matthew Maurer <[email protected]>
---
 rust/kernel/debugfs.rs | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
rust/kernel/debugfs.rs +0 -0
index f6240fd056f8598d5ef06bdaf61c5c33eab5b734..6c7cf7e97741b98d2c0654d01fca3de0d8047e97 100644

rust/kernel/debugfs.rs pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Resu · line 351

Quoted 5 lines
rust/kernel/debugfs.rs +2 -0
@@ -347,6 +347,32 @@ pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Resu
         core::mem::forget(unsafe { self.inner.display_file_raw(name, data)? });
         Ok(())
     }
+
+    /// Creates a nested directory that may live as long as its parent

Ick, why make it complex?

rust/kernel/debugfs.rs pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Resu · line 358

Quoted 7 lines
rust/kernel/debugfs.rs +7 -0
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"), None)?;

Again, no checking please :)

rust/kernel/debugfs.rs pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Resu · line 365

Quoted 7 lines
rust/kernel/debugfs.rs +7 -0
+    /// let foo = KBox::pin_init(Values::attach(0, dir), GFP_KERNEL)?;
+    /// foo.as_ref().build(|_value, builder| {
+    ///   builder.dir(c_str!("bar"))?;
+    ///   Ok::<(), Error>(())
+    /// })?;
+    /// // foo/bar will still exist at this point in DebugFS
+    /// # Ok::<(), Error>(())

Again, no errors :)

thanks,

greg k-h

author
#19 lore raw

PATCH 5/8 rust: debugfs: Support format hooks

In reply to Matthew Maurer

Rather than always using Display, allow hooking arbitrary functions to arbitrary files. Display technically has the expressiveness to do this, but requires a new type be declared for every different way to render things, which can be very clumsy.

Signed-off-by Matthew Maurer <[email protected]>
---
 rust/kernel/debugfs.rs | 125 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 124 insertions(+), 1 deletion(-)
rust/kernel/debugfs.rs +124 -1
index 6c7cf7e97741b98d2c0654d01fca3de0d8047e97..2faa59d2dae44ab708cb8fca0d23f06f73a95a3a 100644
@@ -11,7 +11,8 @@
 use crate::seq_print;
 use crate::str::CStr;
 use crate::types::{ARef, AlwaysRefCounted, Opaque};
-use core::fmt::Display;
+use core::fmt;
+use core::fmt::{Display, Formatter};
 use core::marker::{PhantomData, PhantomPinned};
 use core::mem::ManuallyDrop;
 use core::ops::Deref;

@@ -127,6 +128,35 @@ pub fn display_file<T: Display + Sized>(
         unsafe { self.display_file_raw(name, data) }
     }
 
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking `f`
+    /// on the provided reference. `f` must be a function item or a non-capturing closure, or this
+    /// will fail to compile.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use core::sync::atomic::{AtomicU32, Ordering};
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let dir = Dir::new(c_str!("foo"), None)?;
+    /// static MY_ATOMIC: AtomicU32 = AtomicU32::new(3);
+    /// let file = dir.fmt_file(c_str!("bar"), &MY_ATOMIC, &|val, f| {
+    ///   let out = val.load(Ordering::Relaxed);
+    ///   write!(f, "{out:#010x}\n")
+    /// })?;
+    /// MY_ATOMIC.store(10, Ordering::Relaxed);
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn fmt_file<T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
+        &self,
+        name: &CStr,
+        data: &'static T,
+        f: &'static F,
+    ) -> Result<ARef<Self>> {
+        // SAFETY: As `data` lives for the static lifetime, it outlives the file
+        unsafe { self.fmt_file_raw(name, data, f) }
+    }
+
     /// Creates a DebugFS file backed by the display implementation of the provided pointer.
     ///
     /// # Safety

@@ -162,6 +192,24 @@ unsafe fn display_file_raw<T: Display + Sized>(
         // owning dentry from debugfs_create_dir, so we can wrap it in an ARef
         Ok(unsafe { ARef::from_raw(ptr.cast()) })
     }
+
+    /// Create a file in a DebugFS directory with the provided name, and contents from invoking the
+    /// fomatter on the attached data. The attached function must be a ZST, and will cause a
+    /// compilation error if it is not.
+    ///
+    /// # Safety
+    ///
+    /// `data` must outlive the resulting file's accessibility
+    unsafe fn fmt_file_raw<T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
+        &self,
+        name: &CStr,
+        data: &T,
+        f: &'static F,
+    ) -> Result<ARef<Self>> {
+        let data_adapted = FormatAdapter::new(data, f);
+        // SAFETY: data outlives the file's accessibility, so data_adapted does too
+        unsafe { self.display_file_raw(name, data_adapted) }
+    }
 }
 
 /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`

@@ -373,6 +421,43 @@ pub fn dir(&self, name: &CStr) -> Result<Builder<'a>> {
         // SAFETY: Since 'a is a builder lifetime, we can propagate our invariants
         Ok(unsafe { Builder::new(dir) })
     }
+
+    /// 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.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use kernel::{c_str, new_mutex};
+    /// # use kernel::debugfs::{Dir, Values};
+    /// let dir = Dir::new(c_str!("foo"), None)?;
+    /// let foo = KBox::pin_init(Values::attach(new_mutex!(0), dir), GFP_KERNEL)?;
+    /// foo.as_ref().build(|value, builder| {
+    ///   builder.fmt_file(c_str!("bar"), value, &|val, f| {
+    ///     write!(f, "Mutex read: {}", *val.lock())
+    ///   })
+    /// })?;
+    /// *foo.lock() = 23;
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn fmt_file<T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>(
+        &self,
+        name: &CStr,
+        data: &'a T,
+        f: &'static F,
+    ) -> Result<()> {
+        // We forget the reference because its reference count is implicitly "owned" by the root
+        // builder, which we know will use `debugfs_remove` to clean this up. If we release the
+        // file here, it will be immediately deleted.
+        // SAFETY:
+        // Because `Builder`'s invariant says that our lifetime is how long the directory will
+        // be available, and is equivariant, `'a` will outlive the base directory, which will be
+        // torn down by `debugfs_remove` to prevent access even if an extra refcount is held
+        // somewhere.
+        core::mem::forget(unsafe { self.fmt_file_raw(name, data, f) }?);
+        Ok(())
+    }
 }
 
 impl<'a> Deref for Builder<'a> {

@@ -381,3 +466,41 @@ fn deref(&self) -> &Self::Target {
         &self.inner
     }
 }
+
+// INVARIANT: F is inhabited
+#[repr(transparent)]
+struct FormatAdapter<T, F> {
+    inner: T,
+    _formatter: PhantomData<F>,
+}
+
+impl<T, F> FormatAdapter<T, F> {
+    fn new<'a>(inner: &'a T, _f: &'static F) -> &'a Self {
+        // SAFETY: FormatAdapater is a repr(transparent) wrapper around T, so
+        // casting a reference is legal
+        // INVARIANT: We were passed a reference to F, so it is inhabited.
+        unsafe { core::mem::transmute(inner) }
+    }
+}
+
+impl<T, F: Fn(&T, &mut Formatter<'_>) -> fmt::Result + 'static> Display for FormatAdapter<T, F> {
+    fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
+        // SAFETY: FormatAdapter<_, F> can only be constructed if F is inhabited
+        let f: &F = unsafe { materialize_zst_fmt() };
+        f(&self.inner, fmt)
+    }
+}
+
+/// # Safety
+/// The caller asserts that F is inhabited
+unsafe fn materialize_zst_fmt<F>() -> &'static F {
+    // We don't have generic_const_exprs, and const items inside the function get promoted out and
+    // lose type variables, so we need to do the old-style assert to check for ZSTness
+    [(); 1][core::mem::size_of::<F>()];
+    let zst_dangle: NonNull<F> = NonNull::dangling();
+    // SAFETY:
+    // While the pointer is dangling, it is a dangling pointer to a ZST, based on the array
+    // assertion above. The type is also inhabited, by the caller's assertion. This means
+    // we can materialize it.
+    unsafe { zst_dangle.as_ref() }
+}
Signature · 1 line
2.49.0.901.g37484f566f-goog
#20 lore raw

Re: [PATCH 5/8] rust: debugfs: Support format hooks

rust/kernel/debugfs.rs pub fn display_file<T: Display + Sized>( · line 133

Quoted 4 lines · Matthew Maurer
rust/kernel/debugfs.rs +3 -0
+    /// 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.

The first paragraph of docs are the "title" ("short description"), which renders differently. Typically it should be kept short, e.g. a sentence. So I would probably move the second sentence to a second paragraph.

In any case, is it true that it will fail to compile? Please see below.

Quoted 1 line

+ /// # Example

We use plurals for these sections.

Quoted 1 line

+ ) -> Result<()> {

Can be Result.

rust/kernel/debugfs.rs pub fn dir(&self, name: &CStr) -> Result<Builder<'a>> { · line 454

Quoted 5 lines
rust/kernel/debugfs.rs +5 -0
+        // We forget the reference because its reference count is implicitly "owned" by the root
+        // builder, which we know will use `debugfs_remove` to clean this up. If we release the
+        // file here, it will be immediately deleted.
+        // SAFETY:
+        // Because `Builder`'s invariant says that our lifetime is how long the directory will

In our usual style, this would be e.g.:

    // file here, it will be immediately deleted.
    //
    // SAFETY: Because `Builder`'s ...

rust/kernel/debugfs.rs fn deref(&self) -> &Self::Target { · line 472

Quoted 3 lines
rust/kernel/debugfs.rs +3 -0
+// INVARIANT: F is inhabited
+#[repr(transparent)]
+struct FormatAdapter<T, F> {

For invariants, we put them in the docs, as a section, e.g.

    /// # Invariants
    ///
    /// `F` is inhabited.

rust/kernel/debugfs.rs fn deref(&self) -> &Self::Target { · line 480

Quoted 2 lines
rust/kernel/debugfs.rs +2 -0
+        // SAFETY: FormatAdapater is a repr(transparent) wrapper around T, so
+        // casting a reference is legal

Please use Markdown in comments too.

rust/kernel/debugfs.rs fn deref(&self) -> &Self::Target { · line 496

Quoted 3 lines
rust/kernel/debugfs.rs +3 -0
+/// # Safety
+/// The caller asserts that F is inhabited
+unsafe fn materialize_zst_fmt<F>() -> &'static F {

    /// # Safety
    ///
    /// `F` must be inhabited.

(I simplified the wording)

rust/kernel/debugfs.rs fn deref(&self) -> &Self::Target { · line 499

Quoted 3 lines
rust/kernel/debugfs.rs +3 -0
+    // We don't have generic_const_exprs, and const items inside the function get promoted out and
+    // lose type variables, so we need to do the old-style assert to check for ZSTness
+    [(); 1][core::mem::size_of::<F>()];

Shouldn't this be:

    const { assert!(core::mem::size_of::<F>() == 0) };

to prevent a runtime panic?

Or build_assert! otherwise, but the line above catches the issue if I try to pass a capturing closure from the sample module.

Thanks!

Cheers,
Miguel

author
#21 lore raw

PATCH 6/8 rust: debugfs: Implement display_file in terms of fmt_file

In reply to Matthew Maurer

fmt_file is fundamentally more flexible, this reduces the number of unsafe blocks, and allows us to append a newline for display_file which is a good default.

Previous display_file did not append a newline to ensure that all output strings, including those without trailing newlines, were an option.

Signed-off-by Matthew Maurer <[email protected]>
---
 rust/kernel/debugfs.rs | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)
rust/kernel/debugfs.rs +4 -14
index 2faa59d2dae44ab708cb8fca0d23f06f73a95a3a..835df2b5d7311f278d1d15fc8d688809d0ca363f 100644
@@ -108,7 +108,7 @@ fn as_ptr(&self) -> *mut bindings::dentry {
     }
 
     /// Create a file in a DebugFS directory with the provided name, and contents from invoking
-    /// [`Display::fmt`] on the provided reference.
+    /// [`Display::fmt`] on the provided reference with a trailing newline.
     ///
     /// # Example
     ///

@@ -124,8 +124,7 @@ pub fn display_file<T: Display + Sized>(
         name: &CStr,
         data: &'static T,
     ) -> Result<ARef<Self>> {
-        // SAFETY: As `data` lives for the static lifetime, it outlives the file.
-        unsafe { self.display_file_raw(name, data) }
+        self.fmt_file(name, data, &|val, f| write!(f, "{val}\n"))
     }
 
     /// Create a file in a DebugFS directory with the provided name, and contents from invoking `f`

@@ -367,7 +366,7 @@ unsafe fn new(inner: &'a Dir) -> Self {
     }
 
     /// Create a file in a DebugFS directory with the provided name, and contents from invoking
-    /// [`Display::fmt`] on the provided reference.
+    /// [`Display::fmt`] on the provided reference with a trailing newline.
     ///
     /// # Example
     ///

@@ -384,16 +383,7 @@ unsafe fn new(inner: &'a Dir) -> Self {
     /// # Ok::<(), Error>(())
     /// ```
     pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Result<()> {
-        // We forget the reference because its reference count is implicitly "owned" by the root
-        // builder, which we know will use `debugfs_remove` to clean this up. If we release the
-        // file here, it will be immediately deleted.
-        // SAFETY:
-        // Because `Builder`'s invariant says that our lifetime is how long the directory will
-        // be available, and is equivariant, `'a` will outlive the base directory, which will be
-        // torn down by `debugfs_remove` to prevent access even if an extra refcount is held
-        // somewhere.
-        core::mem::forget(unsafe { self.inner.display_file_raw(name, data)? });
-        Ok(())
+        self.fmt_file(name, data, &|val, f| write!(f, "{val}\n"))
     }
 
     /// Creates a nested directory that may live as long as its parent
Signature · 1 line
2.49.0.901.g37484f566f-goog
author
#22 lore raw

PATCH 7/8 rust: debugfs: Helper macro for common case implementations

In reply to Matthew Maurer

The most likely thing someone needs a custom hook for is a simple wrapper function around write!. This macro lets them just specify the format string directly rather than the fully expanded closure.

Signed-off-by Matthew Maurer <[email protected]>
---
 rust/kernel/debugfs.rs | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)
rust/kernel/debugfs.rs +21 -3
index 835df2b5d7311f278d1d15fc8d688809d0ca363f..edc6dd4cc5aedd4d3f1abc1a3793b39fce110d7b 100644
@@ -8,9 +8,9 @@
 
 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 crate::{debugfs_fmt_file, seq_print};
 use core::fmt;
 use core::fmt::{Display, Formatter};
 use core::marker::{PhantomData, PhantomPinned};

@@ -124,7 +124,7 @@ pub fn display_file<T: Display + Sized>(
         name: &CStr,
         data: &'static T,
     ) -> Result<ARef<Self>> {
-        self.fmt_file(name, data, &|val, f| write!(f, "{val}\n"))
+        debugfs_fmt_file!(self, name, data, "{}\n")
     }
 
     /// Create a file in a DebugFS directory with the provided name, and contents from invoking `f`

@@ -383,7 +383,7 @@ unsafe fn new(inner: &'a Dir) -> Self {
     /// # Ok::<(), Error>(())
     /// ```
     pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'a T) -> Result<()> {
-        self.fmt_file(name, data, &|val, f| write!(f, "{val}\n"))
+        debugfs_fmt_file!(self, name, data, "{}\n")
     }
 
     /// Creates a nested directory that may live as long as its parent

@@ -494,3 +494,21 @@ unsafe fn materialize_zst_fmt<F>() -> &'static F {
     // we can materialize it.
     unsafe { zst_dangle.as_ref() }
 }
+
+#[macro_export]
+/// Allows defining a debugfs file with a format string directly
+///
+/// # Example
+///
+/// ```
+/// # use kernel::debugfs::Dir;
+/// # use kernel::{c_str, debugfs_fmt_file};
+/// let dir = Dir::new(c_str!("foo"), None)?;
+/// debugfs_fmt_file!(dir, c_str!("bar"), &3, "bar={}")?;
+/// # Ok::<(), Error>(())
+/// ```
+macro_rules! debugfs_fmt_file {
+    ($dir:expr, $name:expr, $data:expr, $($arg:tt),*) => {
+        $dir.fmt_file($name, $data, &|binding, fmt| write!(fmt, $($arg),*, binding))
+    }
+}
Signature · 1 line
2.49.0.901.g37484f566f-goog
author
#23 lore raw

PATCH 8/8 rust: samples: Add debugfs sample

In reply to Matthew Maurer

Provides an example of using the Rust DebugFS bindings.

Signed-off-by Matthew Maurer <[email protected]>
---
 MAINTAINERS                  |   1 +
 samples/rust/Kconfig         |  11 ++++
 samples/rust/Makefile        |   1 +
 samples/rust/rust_debugfs.rs | 120 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+)
MAINTAINERS +1 -0
index a3b835e427b083a4ddd690d9e7739851f0af47ae..426bcdac025134e20911de8e2cf5c9efb0591814 100644
@@ -7278,6 +7278,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
samples/rust/Kconfig +11 -0
index 43cb72d72631bb2d6e06185e1d88778edff6ee13..6c42ed73f842cda26256039e6917bb443738d3f1 100644
@@ -51,6 +51,17 @@ config SAMPLE_RUST_DMA
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DEBUGFS
+	tristate "DebugFS Test Driver"
+	depends on DEBUG_FS
+	help
+	  This option builds the Rust DebugFS Test driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_debugfs.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_DRIVER_PCI
 	tristate "PCI Driver"
 	depends on PCI
samples/rust/Makefile +1 -0
index 6a466afd2a21eba84a3b7b2be29f25dce44e9053..b1fc4677ed53fcf7d0f5a3dbf322f65851bc1784 100644
@@ -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
samples/rust/rust_debugfs.rs new file +120 -0

120 lines not on this page · raw message

Signature · 1 line
2.49.0.901.g37484f566f-goog
maintainer
#24 lore raw

Re: [PATCH 8/8] rust: samples: Add debugfs sample

Quoted 11 lines · Matthew Maurer
samples/rust/rust_debugfs.rs new file +11 -0
+    fn build<'a>(&'a self, builder: Builder<'a>) -> Result<()> {
+        builder.display_file(c_str!("simple"), &self.simple)?;
+        builder.display_file(c_str!("composite"), &self.composite)?;
+        debugfs_fmt_file!(
+            builder,
+            c_str!("custom"),
+            &self.custom,
+            "Foo! {:#010x} Bar!\n"
+        )?;
+        for record in self.many.iter() {
+            let dir = builder.dir(record.name)?;

samples/rust/rust_debugfs.rs line 103

Quoted 10 lines
samples/rust/rust_debugfs.rs new file +10 -0
+            dir.display_file(c_str!("size"), &record.size)?;
+            dir.display_file(c_str!("stride"), &record.stride)?;
+        }
+        builder.fmt_file(c_str!("atomic"), &self.atomic, &|atomic, f| {
+            write!(f, "{}\n", atomic.load(Ordering::Relaxed))
+        })?;
+        builder.fmt_file(c_str!("locked"), &self.locked, &|locked, f| {
+            write!(f, "{}\n", *locked.lock())
+        })?;
+        Ok(())

Good luck removing all of those ? checks, this should work just fine without any of that.

I'm not set on the "builder" pattern here, it's not a normal one for C to be following, why do you feel debugfs should be using it?

thanks,

greg k-h

#25 lore raw
In reply to Matthew Maurer
Quoted 2 lines · Matthew Maurer

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

Adding Timur Tabi, because he was looking into how to do this.

thanks,
John Hubbard

Quoted 35 lines

The primary intended way to use these bindings is to attach a data object to a DebugFS directory handle, and use the contents of that attached object to serve what's inside the directory. Through a combination of pinning and equivariance, we can ensure that the pointers in the attached object will remain valid until after the DebugFS directory is made inaccessible.

Signed-off-by Matthew Maurer <[email protected]>

---
Matthew Maurer (8):
rust: debugfs: Bind DebugFS directory creation
rust: debugfs: Bind file creation for long-lived Display
rust: debugfs: Add scoped builder interface
rust: debugfs: Allow subdir creation in builder interface
rust: debugfs: Support format hooks
rust: debugfs: Implement display_file in terms of fmt_file
rust: debugfs: Helper macro for common case implementations
rust: samples: Add debugfs sample

  MAINTAINERS                     |   2 +
  rust/bindings/bindings_helper.h |   1 +
  rust/helpers/dcache.c           |  12 +
  rust/helpers/helpers.c          |   1 +
  rust/kernel/debugfs.rs          | 514 ++++++++++++++++++++++++++++++++++++++++
  rust/kernel/lib.rs              |   1 +
  samples/rust/Kconfig            |  11 +
  samples/rust/Makefile           |   1 +
  samples/rust/rust_debugfs.rs    | 120 ++++++++++
  9 files changed, 663 insertions(+)

---
base-commit: b6a7783d306baf3150ac54cd5124f6e85dd375b0
change-id: 20250428-debugfs-rust-3cd5c97eb7d1

Best regards,

maintainer
#26 lore raw
In reply to Matthew Maurer
Quoted 6 lines · Matthew Maurer

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

The primary intended way to use these bindings is to attach a data object to a DebugFS directory handle, and use the contents of that attached object to serve what's inside the directory.

That's cool, but really, not the common use for debugfs. Why not make the "simple" way work first (i.e. just create/remove directories and files attached to variables and generic read/write functions) and then, if really needed, we can add the structure layout mess like you are wanting to do here.

And yes, I know why you want to tie debugfs layout to a structure layout, it makes one type of debugfs use really easy to write in rust, but that's not the common user for what we have today. Let's address the common use first please, save the "builder" pattern stuff for after we nail all of that down.

thanks,

greg k-h

author
#27 lore raw
Quoted 9 lines

And yes, I know why you want to tie debugfs layout to a structure layout, it makes one type of debugfs use really easy to write in rust, but that's not the common user for what we have today. Let's address the common use first please, save the "builder" pattern stuff for after we nail all of that down.

thanks,

greg k-h

I'll remove that API in the next version of the patch series to get the basics down first, but to give some motivation to what I was trying to support which *is* done in C today, see qcom-socinfo [1] - it uses a backing socinfo_params struct which is expected to outlive its whole directory structure.

[1]: https://github.com/torvalds/linux/blob/b6ea1680d0ac0e45157a819c41b46565f4616186/drivers/soc/qcom/socinfo.c#L133-L156

maintainer
#28 lore raw
Quoted 8 lines · Matthew Maurer
Quoted 8 lines

And yes, I know why you want to tie debugfs layout to a structure layout, it makes one type of debugfs use really easy to write in rust, but that's not the common user for what we have today. Let's address the common use first please, save the "builder" pattern stuff for after we nail all of that down.

thanks,

Quoted 7 lines
Quoted 1 line

greg k-h

I'll remove that API in the next version of the patch series to get the basics down first, but to give some motivation to what I was trying to support which *is* done in C today, see qcom-socinfo [1] - it uses a backing socinfo_params struct which is expected to outlive its whole directory structure.

What exactly do you mean by "outlive"? Right now debugfs has no way to "own" a structure and it just "has to work" so that the file will always be there and hope that the backing variable is also still there. I guess you are trying to encode that "hope" into something real? :)

Quoted 2 lines

[1]: https://github.com/torvalds/linux/blob/b6ea1680d0ac0e45157a819c41b46565f4616186/drivers/soc/qcom/socinfo.c#L133-L156

Yes, SOC drivers are a mess of debugfs files, but manually creating them is "fine" to start with, let's not go wild to start with. If we see common patterns outside of the single soc driver use case, then we can propose exporting structures as a directory structure in debugfs, but I really think that is a very minor use of the api at the moment.

thanks,

greg k-h

author
#29 lore raw
Quoted 18 lines · Greg Kroah-Hartman
Quoted 8 lines · Matthew Maurer
Quoted 8 lines

And yes, I know why you want to tie debugfs layout to a structure layout, it makes one type of debugfs use really easy to write in rust, but that's not the common user for what we have today. Let's address the common use first please, save the "builder" pattern stuff for after we nail all of that down.

thanks,

Quoted 7 lines
Quoted 1 line

greg k-h

I'll remove that API in the next version of the patch series to get the basics down first, but to give some motivation to what I was trying to support which *is* done in C today, see qcom-socinfo [1] - it uses a backing socinfo_params struct which is expected to outlive its whole directory structure.

Quoted 4 lines

What exactly do you mean by "outlive"? Right now debugfs has no way to "own" a structure and it just "has to work" so that the file will always be there and hope that the backing variable is also still there. I guess you are trying to encode that "hope" into something real? :)

Yes, the Values structure used by the builder interface enforces that the backing variable must live at least as long as the Dir corresponding to the lowest directory it's going to be used in. To make DebugFS safe in Rust, we either need to:
1. Have DebugFS files own things (not currently done, doesn't seem to match intentions).
2. Expose things that live forever (e.g. globals, like the early patches do)
3. Have a pinned structure that is guaranteed to outlive the files that use it (what the builder interface is trying to support)

Quoted 13 lines
Quoted 3 lines

[1]: https://github.com/torvalds/linux/blob/b6ea1680d0ac0e45157a819c41b46565f4616186/drivers/soc/qcom/socinfo.c#L133-L156

Yes, SOC drivers are a mess of debugfs files, but manually creating them is "fine" to start with, let's not go wild to start with. If we see common patterns outside of the single soc driver use case, then we can propose exporting structures as a directory structure in debugfs, but I really think that is a very minor use of the api at the moment.

thanks,

greg k-h

maintainer
#30 lore raw
Quoted 25 lines · Matthew Maurer
Quoted 18 lines · Greg Kroah-Hartman
Quoted 8 lines · Matthew Maurer
Quoted 8 lines

And yes, I know why you want to tie debugfs layout to a structure layout, it makes one type of debugfs use really easy to write in rust, but that's not the common user for what we have today. Let's address the common use first please, save the "builder" pattern stuff for after we nail all of that down.

thanks,

Quoted 7 lines
Quoted 1 line

greg k-h

I'll remove that API in the next version of the patch series to get the basics down first, but to give some motivation to what I was trying to support which *is* done in C today, see qcom-socinfo [1] - it uses a backing socinfo_params struct which is expected to outlive its whole directory structure.

Quoted 4 lines

What exactly do you mean by "outlive"? Right now debugfs has no way to "own" a structure and it just "has to work" so that the file will always be there and hope that the backing variable is also still there. I guess you are trying to encode that "hope" into something real? :)

Quoted 6 lines

Yes, the Values structure used by the builder interface enforces that the backing variable must live at least as long as the Dir corresponding to the lowest directory it's going to be used in. To make DebugFS safe in Rust, we either need to:
1. Have DebugFS files own things (not currently done, doesn't seem to match intentions).

Agreed, let's not do that.

Quoted 1 line

2. Expose things that live forever (e.g. globals, like the early patches do)

Let's start with this for now.

Quoted 2 lines

3. Have a pinned structure that is guaranteed to outlive the files that use it (what the builder interface is trying to support)

Let's add this "after" we get the basics down. Should make things simpler for you to work on now, and for us to review.

Baby steps :)

thanks,

greg k-h