* [PATCH WIP 0/2] rust: debugfs: Support attaching data to DebugFS directories @ 2025-05-03 0:43 Matthew Maurer 2025-05-03 0:43 ` [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects Matthew Maurer 2025-05-03 0:44 ` [PATCH WIP 2/2] rust: debugfs: Extend sample to use attached data Matthew Maurer 0 siblings, 2 replies; 6+ messages in thread From: Matthew Maurer @ 2025-05-03 0:43 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Sami Tolvanen, Timur Tabi Cc: rust-for-linux, linux-kernel, Matthew Maurer We can wait to properly review and land this until after we've decided on the final interface for the first part, but since dakr@kernel.org was poking at how this might work in his review of the previous patchset, I wanted to upload this sketch as context. The general concept here is that you select an owning directory and attach data to it while converting its lifetime to be invariant (e.g. can't be shortened) so that you know that the DebugFS contents will be cleaned up before the data. You can then implement things underneath that directory using the attached data. Signed-off-by: Matthew Maurer <mmaurer@google.com> --- Matthew Maurer (2): rust: debugfs: Add interface to build debugfs off pinned objects rust: debugfs: Extend sample to use attached data rust/kernel/debugfs.rs | 206 ++++++++++++++++++++++++++++++++++++++++--- samples/rust/rust_debugfs.rs | 110 ++++++++++++++++++++++- 2 files changed, 302 insertions(+), 14 deletions(-) --- base-commit: 33035b665157558254b3c21c3f049fd728e72368 change-id: 20250501-debugfs-rust-attach-e164b67c9c16 prerequisite-change-id: 20250428-debugfs-rust-3cd5c97eb7d1:v4 prerequisite-patch-id: 7ac67017c11249cd04fc4beca6cfdb5b83aa89de prerequisite-patch-id: 2e8256a6ef25afc95279e740f43d17ec169a65f2 prerequisite-patch-id: 0abd76fd2d71ba300d8a5ce5b3b61751feec88fa prerequisite-patch-id: 5919cceb97187adfc71e9c1899f85d5af092bde6 Best regards, -- Matthew Maurer <mmaurer@google.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects 2025-05-03 0:43 [PATCH WIP 0/2] rust: debugfs: Support attaching data to DebugFS directories Matthew Maurer @ 2025-05-03 0:43 ` Matthew Maurer 2025-05-04 16:58 ` Danilo Krummrich 2025-05-03 0:44 ` [PATCH WIP 2/2] rust: debugfs: Extend sample to use attached data Matthew Maurer 1 sibling, 1 reply; 6+ messages in thread From: Matthew Maurer @ 2025-05-03 0:43 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Sami Tolvanen, Timur Tabi Cc: rust-for-linux, linux-kernel, Matthew Maurer Previously, we could only expose `'static` references to DebugFS because we don't know how long the file could live. This provides a way to package data alongside an owning directory to guarantee that it will live long enough to be accessed by the DebugFS files, while still allowing a dynamic lifecycle. Signed-off-by: Matthew Maurer <mmaurer@google.com> --- rust/kernel/debugfs.rs | 206 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 196 insertions(+), 10 deletions(-) diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs index cfdff63c10517f1aebf757c965289a49fed6ae85..f232598b510e036bd25e6846f153572ebfb459f3 100644 --- a/rust/kernel/debugfs.rs +++ b/rust/kernel/debugfs.rs @@ -6,10 +6,14 @@ //! //! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h) +use crate::prelude::PinInit; use crate::str::CStr; use core::fmt; use core::fmt::Display; -use core::marker::PhantomData; +use core::marker::{PhantomData, PhantomPinned}; +use core::ops::Deref; +use core::pin::Pin; +use pin_init::pin_data; /// Owning handle to a DebugFS directory. /// @@ -158,6 +162,8 @@ pub fn fmt_file<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>( f: &'static F, ) -> File<'b> { // SAFETY: As `data` lives for the static lifetime, it outlives the file + // As we return `File<'b>`, and we have a borrow to a directory with lifetime 'b, we have a + // lower bound of `'b` on the directory. unsafe { self.fmt_file_raw(name, data, f) } } @@ -169,11 +175,10 @@ pub fn fmt_file<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>( /// This means that before `data` may become invalid, either: /// * The refcount must go to zero. /// * The file must be rendered inaccessible, e.g. via `debugfs_remove`. - unsafe fn display_file_raw<'b, T: Display + Sized>( - &'b self, - name: &CStr, - data: *const T, - ) -> File<'b> { + /// * If `self` may take longer than `'a` to be destroyed, the caller is responsible for + /// shortening the lifetime of the return value to a lower bound for the lifetime of that + /// object. + unsafe fn display_file_raw<T: Display + Sized>(&self, name: &CStr, data: *const T) -> File<'a> { // SAFETY: // * `name` is a NUL-terminated C string, living across the call, by `CStr` invariant. // * `parent` is a live `dentry` since we have a reference to it. @@ -215,13 +220,16 @@ unsafe fn display_file_raw<'b, T: Display + Sized>( /// /// # Safety /// - /// `data` must outlive the resulting file's accessibility - unsafe fn fmt_file_raw<'b, T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>( - &'b self, + /// * `data` must outlive the resulting file's accessibility + /// * If `self` may take longer than `'a` to be destroyed, the caller is responsible for + /// shortening the lifetime of the return value to a lower bound for the lifetime of that + /// object. + unsafe fn fmt_file_raw<T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>( + &self, name: &CStr, data: &T, f: &'static F, - ) -> File<'b> { + ) -> File<'a> { #[cfg(CONFIG_DEBUG_FS)] let data_adapted = FormatAdapter::new(data, f); #[cfg(not(CONFIG_DEBUG_FS))] @@ -303,6 +311,184 @@ pub fn remove(self) { } } +/// A DebugFS directory combined with a backing store for data to implement it. +#[pin_data] +pub struct Values<T> { + dir: Dir<'static, false>, + // The order here is load-bearing - `dir` must be dropped before `backing`, as files under + // `dir` may point into `backing`. + #[pin] + backing: T, + // Since the files present under our directory may point into `backing`, we are `!Unpin`. + #[pin] + _pin: PhantomPinned, +} + +impl<T> Deref for Values<T> { + type Target = T; + fn deref(&self) -> &T { + &self.backing + } +} + +impl<T> Values<T> { + /// Attach backing data to a DebugFS directory. + /// + /// Attached data will be available inside [`Values::build`] to use when defining files in + /// the provided directory. When this object is dropped, it will remove the provided directory + /// before destroying the attached data. + /// + /// # Example + /// + /// ``` + /// # use kernel::{c_str, new_mutex}; + /// # use kernel::debugfs::{Dir, Values}; + /// let dir = Dir::new(c_str!("foo")); + /// let debugfs = KBox::pin_init(Values::attach(new_mutex!(0), dir), GFP_KERNEL)?; + /// // Files can be put inside `debugfs` which reference the constructed mutex. + /// # Ok::<(), Error>(()) + /// ``` + pub fn attach(backing: impl PinInit<T>, dir: Dir<'static, false>) -> impl PinInit<Self> { + pin_init::pin_init! { Self { + dir: dir, + backing <- backing, + _pin: PhantomPinned, + }} + } + + /// Runs a function which can create files from the backing data. + /// + /// # Example + /// + /// ``` + /// # use kernel::{c_str, new_mutex}; + /// # use kernel::debugfs::{Dir, Values}; + /// let dir = Dir::new(c_str!("foo")); + /// let debugfs = KBox::pin_init(Values::attach(new_mutex!(0), dir), GFP_KERNEL)?; + /// debugfs.as_ref().build(|mutex, dir| { + /// // Can access `BoundDir` methods on `dir` here, with lifetime unified to the + /// // lifetime of `mutex` + /// }); + /// # Ok::<(), Error>(()) + /// ``` + pub fn build<U, F: for<'b> FnOnce(&'b T, BoundDir<'b>) -> U>(self: Pin<&Self>, f: F) -> U { + // SAFETY: The `BoundDir` here is only being provided as a universally quantified argument + // to a function, so in the body, it will only be available in an existentially quantified + // context. This means that the function is legal to execute agains the true lifetime of + // the directory, even if we don't know exactly what that is. + f(&self.backing, unsafe { BoundDir::new(&self.dir) }) + } +} + +/// A `Dir`, bound to the lifetime for which it will exist. Unlike `&'a Dir`, this has an invariant +/// lifetime - it cannot be shortened or lengthened. +/// +/// # Invariants +/// +/// * `BoundDir`'s lifetime must match the actual lifetime the directory lives for. In practice, +/// this means that a `BoundDir` may only be used in an existentially quantified context. +/// * `dir` will never be promoted to an owning reference (e.g. via calling `Dir::owning`) +#[repr(transparent)] +pub struct BoundDir<'a> { + dir: Dir<'a, true>, + _invariant: PhantomData<fn(&'a ()) -> &'a ()>, +} + +impl<'a> BoundDir<'a> { + /// Create a new bound directory. + /// + /// # Safety + /// + /// `'a` is being used in a context where it is existentially quantified. + unsafe fn new(dir: &'a Dir<'_, false>) -> Self { + Self { + // We can create a non-owning `Dir` with our restricted lifetime because we do not + // allow this `dir` to be upgraded to an owning reference, and the owning reference + // provided necessarily outlives us. + dir: Dir { + dir: dir.dir, + _phantom: PhantomData, + }, + _invariant: PhantomData, + } + } + + /// Create a DebugFS subdirectory. + /// + /// This will produce another [`BoundDir`], scoped to the lifetime of the parent [`BoundDir`]. + /// + /// # Examples + /// + /// ``` + /// # use kernel::c_str; + /// # use kernel::debugfs::{Dir, Values}; + /// let parent = Dir::new(c_str!("parent")); + /// let values = KBox::pin_init(Values::attach(0, parent), GFP_KERNEL)?; + /// values.as_ref().build(|value, builder| { + /// builder.subdir(c_str!("child")); + /// }); + /// # Ok::<(), Error>(()) + /// ``` + pub fn subdir(&self, name: &CStr) -> Self { + Self { + dir: Dir::create(name, Some(&self.dir)), + _invariant: PhantomData, + } + } + + /// Create a file in a DebugFS directory with the provided name, and contents from invoking `f` + /// on the provided reference. + /// + /// `f` must be a function item or a non-capturing closure, or this will fail to compile. + /// + /// # Examples + /// + /// ``` + /// # use kernel::{c_str, new_mutex}; + /// # use kernel::debugfs::{Dir, Values}; + /// let parent = Dir::new(c_str!("parent")); + /// let values = KBox::pin_init(Values::attach(new_mutex!(0), parent), GFP_KERNEL)?; + /// values.as_ref().build(|value, builder| { + /// builder.fmt_file(c_str!("child"), value, &|value, f| { + /// writeln!(f, "Reading a mutex: {}", *value.lock()) + /// }); + /// }); + /// # Ok::<(), Error>(()) + /// ``` + pub fn fmt_file<T, F: Fn(&T, &mut fmt::Formatter<'_>) -> fmt::Result>( + &self, + name: &CStr, + data: &'a T, + f: &'static F, + ) -> File<'a> { + // SAFETY: As `data` lives for the same lifetime as our `BoundDir` lifetime, which is + // instantiated as the actual lifetime of the directory, it lives long enough. + // We don't need to shorten the file handle because `BoundDir`'s lifetime parameter is both + // a lower *and* upper bound. + unsafe { self.dir.fmt_file_raw(name, data, f) } + } + + /// Create a file in a DebugFS directory with the provided name, and contents from invoking + /// [`Display::fmt`] on the provided reference with a trailing newline. + /// + /// # Examples + /// + /// ``` + /// # use kernel::{c_str, new_mutex}; + /// # use kernel::debugfs::{Dir, Values}; + /// let parent = Dir::new(c_str!("parent")); + /// let values = KBox::pin_init(Values::attach((1, 2), parent), GFP_KERNEL)?; + /// values.as_ref().build(|value, builder| { + /// builder.display_file(c_str!("child_0"), &value.0); + /// builder.display_file(c_str!("child_1"), &value.1); + /// }); + /// # Ok::<(), Error>(()) + /// ``` + pub fn display_file<T: Display>(&self, name: &CStr, data: &'a T) -> File<'a> { + self.fmt_file(name, data, &|data, f| writeln!(f, "{}", data)) + } +} + #[cfg(CONFIG_DEBUG_FS)] mod helpers { use crate::seq_file::SeqFile; -- 2.49.0.906.g1f30a19c02-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects 2025-05-03 0:43 ` [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects Matthew Maurer @ 2025-05-04 16:58 ` Danilo Krummrich 2025-05-05 17:32 ` Matthew Maurer 0 siblings, 1 reply; 6+ messages in thread From: Danilo Krummrich @ 2025-05-04 16:58 UTC (permalink / raw) To: Matthew Maurer Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Sami Tolvanen, Timur Tabi, rust-for-linux, linux-kernel On Sat, May 03, 2025 at 12:43:59AM +0000, Matthew Maurer wrote: > +/// A DebugFS directory combined with a backing store for data to implement it. > +#[pin_data] > +pub struct Values<T> { > + dir: Dir<'static, false>, > + // The order here is load-bearing - `dir` must be dropped before `backing`, as files under > + // `dir` may point into `backing`. > + #[pin] > + backing: T, > + // Since the files present under our directory may point into `backing`, we are `!Unpin`. > + #[pin] > + _pin: PhantomPinned, > +} This only ever allows attaching data to the root directory, correct? What if I want to remove (or replace) a file or a subdir? Then I'd be left with the data for this specific file (or subdir) until the root is finally removed. It would also require Option<V> (where V is the type of a field in T), if I don't have an instance of V yet, when the root directory is created. I think we should store the data per file, rather than per root directory. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects 2025-05-04 16:58 ` Danilo Krummrich @ 2025-05-05 17:32 ` Matthew Maurer 2025-05-07 18:16 ` Danilo Krummrich 0 siblings, 1 reply; 6+ messages in thread From: Matthew Maurer @ 2025-05-05 17:32 UTC (permalink / raw) To: Danilo Krummrich Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Sami Tolvanen, Timur Tabi, rust-for-linux, linux-kernel On Sun, May 4, 2025 at 9:58 AM Danilo Krummrich <dakr@kernel.org> wrote: > > On Sat, May 03, 2025 at 12:43:59AM +0000, Matthew Maurer wrote: > > +/// A DebugFS directory combined with a backing store for data to implement it. > > +#[pin_data] > > +pub struct Values<T> { > > + dir: Dir<'static, false>, > > + // The order here is load-bearing - `dir` must be dropped before `backing`, as files under > > + // `dir` may point into `backing`. > > + #[pin] > > + backing: T, > > + // Since the files present under our directory may point into `backing`, we are `!Unpin`. > > + #[pin] > > + _pin: PhantomPinned, > > +} > > This only ever allows attaching data to the root directory, correct? What if I > want to remove (or replace) a file or a subdir? Then I'd be left with the data > for this specific file (or subdir) until the root is finally removed. The intended way to deal with this is that your debugfs root has structures or handles that let you compute what all your debugfs files should see, not necessarily fully populated if you want something dynamic. For example, one of your entries could be `Arc<Mutex<Vec<Record>>>`, and this could get updated elsewhere without thinking about DebugFS - you just need to know the shape of all the handles DebugFS will need. It'd be pretty easy for me to relax this to allow attachments to subtrees. I'd just need to do `Values<'a, T>` and `Dir<'a, false>`. It'd also make the argument for the safety of the builder interface slightly more complicated (things that statically outlive the lifetime of the dir would also be usable in the builder, which is safe, but trickier to argue for). You wouldn't be able to do nested attachments, but you could do attachments starting with any subtree. So for example, you could do: ``` let root = Dir::new(c_str!("foo")); let foo = root.subdir(c_str!("bar")).owning(); let bar = root.subdir(c_str!("baz")).owning(); let foo_values = Values::attach(foo_data, foo); let bar_values = Values::attach(bar_data, foo); ``` The tricky business here (and why I didn't do it in my initial draft) is that because `foo_values` and `bar_values` are restricted to live no longer than `root` (since they will be invalidated if `root` is dropped), it is hard to store these objects in a module context or similar object, which would make it tricky to use. Attaching to a root directory on the other hand is not tricky, because their lifecycle isn't dependent on some other object. A legal way of using this kind of scoped interface (but not immediately obvious to me how to design the safe interface to let users build it) might look like ``` struct MyDebugFsInfo<T> { // RawValues is a fictitious type that would be like `Values`, but with the actual lifetime parameter erased. subdirs: Vec<RawValues<T>>, // Order matters, root must be released last because earlier values are borrows root: Dir<'static, false>, } impl<T> MyDebugFsInfo<T> { fn new_subdir(&mut self, name: &CStr, value: T) { let subdir = self.root.subdir(name); // SAFETY: We don't allow the root to be destroyed before our structure, so let val = unsafe { RawValues::attach(value, self.root.subdir(name)) }; self.subdirs.push(val) } fn get<'a>(&'a self, idx: usize) -> &Values<'a, T> { let x = self.subdirs[idx]; // SAFETY: We know all our subdirs are children of the root we own, so if we have a read-only reference to ourselves, that's an upper bound on how long the value struct can live unsafe { RawValues::ascribe(x) } } } ``` If I wanted to do better than this in terms of ergonomics, I think I'd need some kind of refcounted wrapper where holding a subdir prevents the parent dir from being cleaned up, which I could add, but would be a Rust abstraction layer on top of the C one. > > It would also require Option<V> (where V is the type of a field in T), if I > don't have an instance of V yet, when the root directory is created. `Option<V>` won't actually save you - once you create a `Values<T>`, you are intentionally unable to mutate `T` anymore, because it may be read at any point in time after that by someone reading a debugfs file, so you can't do a potentially racing mutation. If you wanted a value to be attached to a debugfs, but didn't have one, you'd need to do `Mutex<Option<V>>`. > > I think we should store the data per file, rather than per root directory. This doesn't match what at least seems to be a common usage of debugfs, where a debugfs directory is used to represent an object with each file in the directory representing one of the properties on the object. This also means that we need to store N handles in a Rust driver (one per live file) compared to 1 handle in a C driver. === I do think that the normal idiomatic way of doing this in Rust would be to attach refcounted handles (e.g. similar to `ForeignOwnable`) to inodes, and when the inode is destroyed, have that refcount be deleted, but we don't currently have any `release` implementation for a DebugFS inode afaict, so everything has to be externally scoped via this attached pinned data setup I've been producing. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects 2025-05-05 17:32 ` Matthew Maurer @ 2025-05-07 18:16 ` Danilo Krummrich 0 siblings, 0 replies; 6+ messages in thread From: Danilo Krummrich @ 2025-05-07 18:16 UTC (permalink / raw) To: Matthew Maurer Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Sami Tolvanen, Timur Tabi, rust-for-linux, linux-kernel On Mon, May 05, 2025 at 10:32:39AM -0700, Matthew Maurer wrote: > On Sun, May 4, 2025 at 9:58 AM Danilo Krummrich <dakr@kernel.org> wrote: > > > > On Sat, May 03, 2025 at 12:43:59AM +0000, Matthew Maurer wrote: > > > +/// A DebugFS directory combined with a backing store for data to implement it. > > > +#[pin_data] > > > +pub struct Values<T> { > > > + dir: Dir<'static, false>, > > > + // The order here is load-bearing - `dir` must be dropped before `backing`, as files under > > > + // `dir` may point into `backing`. > > > + #[pin] > > > + backing: T, > > > + // Since the files present under our directory may point into `backing`, we are `!Unpin`. > > > + #[pin] > > > + _pin: PhantomPinned, > > > +} > > > > This only ever allows attaching data to the root directory, correct? What if I > > want to remove (or replace) a file or a subdir? Then I'd be left with the data > > for this specific file (or subdir) until the root is finally removed. > > The intended way to deal with this is that your debugfs root has > structures or handles that let you compute what all your debugfs files > should see, not necessarily fully populated if you want something > dynamic. For example, one of your entries could be > `Arc<Mutex<Vec<Record>>>`, and this could get updated elsewhere > without thinking about DebugFS - you just need to know the shape of > all the handles DebugFS will need. Well sure, but what if we can't initialize them (yet) at the time we create the debugfs root? Also, what if the data that should be exposed through a file in debugfs is not supposed to live for the full lifetime of the debugfs root (which might be forever)? > It'd be pretty easy for me to relax this to allow attachments to > subtrees. I'd just need to do `Values<'a, T>` and `Dir<'a, false>`. > It'd also make the argument for the safety of the builder interface > slightly more complicated (things that statically outlive the lifetime > of the dir would also be usable in the builder, which is safe, but > trickier to argue for). > > You wouldn't be able to do nested attachments, but you could do > attachments starting with any subtree. So for example, you could do: > ``` > let root = Dir::new(c_str!("foo")); > let foo = root.subdir(c_str!("bar")).owning(); > let bar = root.subdir(c_str!("baz")).owning(); > let foo_values = Values::attach(foo_data, foo); > let bar_values = Values::attach(bar_data, foo); > ``` > > The tricky business here (and why I didn't do it in my initial draft) > is that because `foo_values` and `bar_values` are restricted to live > no longer than `root` (since they will be invalidated if `root` is > dropped), it is hard to store these objects in a module context or > similar object, which would make it tricky to use. Attaching to a root > directory on the other hand is not tricky, because their lifecycle > isn't dependent on some other object. > > A legal way of using this kind of scoped interface (but not > immediately obvious to me how to design the safe interface to let > users build it) might look like > > ``` > struct MyDebugFsInfo<T> { > // RawValues is a fictitious type that would be like `Values`, but > with the actual lifetime parameter erased. > subdirs: Vec<RawValues<T>>, > // Order matters, root must be released last because earlier values > are borrows > root: Dir<'static, false>, > } > > impl<T> MyDebugFsInfo<T> { > fn new_subdir(&mut self, name: &CStr, value: T) { > let subdir = self.root.subdir(name); > // SAFETY: We don't allow the root to be destroyed before our structure, so > let val = unsafe { RawValues::attach(value, self.root.subdir(name)) }; > self.subdirs.push(val) > } > fn get<'a>(&'a self, idx: usize) -> &Values<'a, T> { > let x = self.subdirs[idx]; > // SAFETY: We know all our subdirs are children of the root we > own, so if we have a read-only reference to ourselves, that's an upper > bound on how long the value struct can live > unsafe { RawValues::ascribe(x) } > } > } > ``` Yes, I'm aware of this possibility and its complexity. That's why I think per file would suit better. > If I wanted to do better than this in terms of ergonomics, I think I'd > need some kind of refcounted wrapper where holding a subdir prevents > the parent dir from being cleaned up, which I could add, but would be > a Rust abstraction layer on top of the C one. I don't think something like that is desirable. > > > > It would also require Option<V> (where V is the type of a field in T), if I > > don't have an instance of V yet, when the root directory is created. > > `Option<V>` won't actually save you - once you create a `Values<T>`, > you are intentionally unable to mutate `T` anymore, because it may be > read at any point in time after that by someone reading a debugfs > file, so you can't do a potentially racing mutation. If you wanted a > value to be attached to a debugfs, but didn't have one, you'd need to > do `Mutex<Option<V>>`. Sure, or some other kind of synchronization. I left that aside, since it's not relevant for the point I wanted to make. Which is that with the per root data approach we'd need to work with Option types when the data isn't yet known when the root directory is created and if we want to be able to free the data before the root directory is removed (which might be never). > > > > I think we should store the data per file, rather than per root directory. > > This doesn't match what at least seems to be a common usage of > debugfs, where a debugfs directory is used to represent an object with > each file in the directory representing one of the properties on the > object. Why not treat File as container, i.e. make the property of an object a File<T> with T being the property? > This also means that we need to store N handles in a Rust driver (one > per live file) compared to 1 handle in a C driver. I can't follow what you mean here. Files in a C driver do need a handle to the data they expose too, no? > > === > > I do think that the normal idiomatic way of doing this in Rust would > be to attach refcounted handles (e.g. similar to `ForeignOwnable`) to > inodes, and when the inode is destroyed, have that refcount be > deleted, but we don't currently have any `release` implementation for > a DebugFS inode afaict, so everything has to be externally scoped via > this attached pinned data setup I've been producing. Why do you think that this would be "the normal idimatic way" of doing this? Does this imply that you think tying the lifetime of the object exposing a value to the value itself wouldn't be idiomatic for Rust? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH WIP 2/2] rust: debugfs: Extend sample to use attached data 2025-05-03 0:43 [PATCH WIP 0/2] rust: debugfs: Support attaching data to DebugFS directories Matthew Maurer 2025-05-03 0:43 ` [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects Matthew Maurer @ 2025-05-03 0:44 ` Matthew Maurer 1 sibling, 0 replies; 6+ messages in thread From: Matthew Maurer @ 2025-05-03 0:44 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Sami Tolvanen, Timur Tabi Cc: rust-for-linux, linux-kernel, Matthew Maurer Demonstrates how to attach data/handles needed for implementing DebugFS file to a directory. Signed-off-by: Matthew Maurer <mmaurer@google.com> --- samples/rust/rust_debugfs.rs | 110 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 4 deletions(-) diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs index 2b1119b7281fd15109b542e6853d4206c2c80afc..6da8dd2c91e8c206b2314eabb97d6d31843efeb5 100644 --- a/samples/rust/rust_debugfs.rs +++ b/samples/rust/rust_debugfs.rs @@ -4,10 +4,14 @@ //! Sample DebugFS exporting module +use core::fmt; +use core::fmt::{Display, Formatter}; use core::sync::atomic::{AtomicU32, Ordering}; use kernel::c_str; -use kernel::debugfs::Dir; +use kernel::debugfs::{BoundDir, Dir, Values}; +use kernel::new_mutex; use kernel::prelude::*; +use kernel::sync::{Arc, Mutex}; module! { type: RustDebugFs, @@ -20,7 +24,89 @@ struct RustDebugFs { // As we only hold this for drop effect (to remove the directory) we have a leading underscore // to indicate to the compiler that we don't expect to use this field directly. - _debugfs: Dir<'static>, + _debugfs: Pin<KBox<Values<Backing>>>, +} + +struct Composite { + major: u32, + minor: u32, +} + +struct Record { + name: &'static CStr, + size: usize, + stride: usize, +} + +struct Backing { + simple: u32, + composite: Composite, + custom: u32, + many: KVec<Record>, + atomic: AtomicU32, + locked: Arc<Mutex<u32>>, +} + +impl Display for Composite { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}.{}", self.major, self.minor) + } +} + +impl Backing { + fn new() -> Result<Self> { + let mut many = KVec::new(); + many.push( + Record { + name: c_str!("foo"), + size: 1, + stride: 2, + }, + GFP_KERNEL, + )?; + many.push( + Record { + name: c_str!("bar"), + size: 3, + stride: 4, + }, + GFP_KERNEL, + )?; + Ok(Self { + simple: 10, + composite: Composite { major: 1, minor: 2 }, + custom: 37, + many, + atomic: AtomicU32::new(7), + locked: Arc::pin_init(new_mutex!(0), GFP_KERNEL)?, + }) + } + + fn build<'a>(&'a self, root: BoundDir<'a>) { + // Just prints out the number in the field + root.display_file(c_str!("simple"), &self.simple); + // Uses the custom display implementation to print major.minor + root.display_file(c_str!("composite"), &self.composite); + // Uses the custom hook print the number in 0-padded hex with some decorations. + root.fmt_file(c_str!("custom"), &self.custom, &|custom, f| { + writeln!(f, "Foo! {:#010x} Bar!", custom) + }); + // Creates a directory for every record in the list, named the name of the item, with files + // for each attribute. + for record in self.many.iter() { + let dir = root.subdir(record.name); + dir.display_file(c_str!("size"), &record.size); + dir.display_file(c_str!("stride"), &record.stride); + } + // Access the attached atomic via `.load()` + root.fmt_file(c_str!("atomic"), &self.atomic, &|atomic, f| { + writeln!(f, "{}", atomic.load(Ordering::Relaxed)) + }); + // Access the attached mutex-guarded data via `.lock()` + root.fmt_file(c_str!("locked"), &self.locked, &|locked, f| { + writeln!(f, "{}", *locked.lock()) + }); + } } static EXAMPLE: AtomicU32 = AtomicU32::new(8); @@ -28,11 +114,11 @@ struct RustDebugFs { impl kernel::Module for RustDebugFs { fn init(_this: &'static ThisModule) -> Result<Self> { // Create a debugfs directory in the root of the filesystem called "sample_debugfs". - let debugfs = Dir::new(c_str!("sample_debugfs")); + let root = Dir::new(c_str!("sample_debugfs")); { // Create a subdirectory, so "sample_debugfs/subdir" now exists. - let sub = debugfs.subdir(c_str!("subdir")); + let sub = root.subdir(c_str!("subdir")); // Create a single file in the subdirectory called "example" that will read from the // `EXAMPLE` atomic variable. @@ -47,6 +133,22 @@ fn init(_this: &'static ThisModule) -> Result<Self> { EXAMPLE.store(10, Ordering::Relaxed); // Now, "sample_debugfs/subdir/example" will print "Reading atomic: 10\n" when read. + // We can also attach data scoped to our root directory + let backing = Backing::new()?; + // Grab a refcount pointing to the locked data + let locked = backing.locked.clone(); + let debugfs = KBox::pin_init(Values::attach(backing, root), GFP_KERNEL)?; + + // Once it's attached, we can invoke `Backing::build` to create the relevant files: + debugfs.as_ref().build(Backing::build); + + // We can still access read-only references the contents of the attached values. If the + // values allow interior mutability, like atomics, this lets us still change them: + debugfs.as_ref().atomic.store(8, Ordering::Relaxed); + + // If we attached refcounted data, we can use an external handle to access it + *locked.lock() = 42; + // Save the root debugfs directory we created to our module object. It will be // automatically cleaned up when our module is unloaded because dropping the module object // will drop the `Dir` handle. The base directory, the subdirectory, and the file will all -- 2.49.0.906.g1f30a19c02-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-05-07 18:16 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-03 0:43 [PATCH WIP 0/2] rust: debugfs: Support attaching data to DebugFS directories Matthew Maurer 2025-05-03 0:43 ` [PATCH WIP 1/2] rust: debugfs: Add interface to build debugfs off pinned objects Matthew Maurer 2025-05-04 16:58 ` Danilo Krummrich 2025-05-05 17:32 ` Matthew Maurer 2025-05-07 18:16 ` Danilo Krummrich 2025-05-03 0:44 ` [PATCH WIP 2/2] rust: debugfs: Extend sample to use attached data Matthew Maurer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).