On Wed, Dec 17, 2025 at 5:49 PM Timur Tabi <[email protected]> wrote:
>
> Dir::scope() was declared as taking &'a self, which tied the returned
> PinInit's lifetime to the Dir. This prevented using scope() with a
> locally-created Dir:
>
> let dir = Dir::lookup(...).unwrap_or_else(Dir::empty);
> let scope = dir.scope(...); // Error: returns value referencing local
This looks like you're trying to return an `impl PinInit<Scope>` from
your function (which isn't written in the example, but I assume that's
what you eventually do, otherwise this wouldn't be an error).
The generalized solution to this is to use `pin_init_scope`, i.e.
```
pin_init_scope(|| {
let dir = // Generate a `Dir` somehow here
let scope = dir.scope(/* Scope defining function here */)
// Anything else you wanted to do
Ok(scope)
})
```
>
> The borrow was unnecessary since scoped_dir() internally clones the
> Arc<Entry>. Fix this by cloning self.0 before the closure, allowing the
> closure to capture the cloned value via move instead of borrowing self.
>
> This also removes the now-unused scoped_dir() helper method, inlining
> its logic directly into scope().
>
> Signed-off-by: Timur Tabi <[email protected]>
> ---
> rust/kernel/debugfs.rs | 41 +++++++++++++++++------------------------
> 1 file changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index b0cfe22982d6..35f9cbb261e7 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -397,27 +397,6 @@ pub fn write_callback_file<'a, T, E: 'a, W>(
> self.create_file(name, data, file_ops)
> }
>
> - // While this function is safe, it is intentionally not public because
> it's a bit of a
> - // footgun.
> - //
> - // Unless you also extract the `entry` later and schedule it for `Drop`
> at the appropriate
> - // time, a `ScopedDir` with a `Dir` parent will never be deleted.
> - fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
> - #[cfg(CONFIG_DEBUG_FS)]
> - {
> - let parent_entry = match &self.0 {
> - None => return ScopedDir::empty(),
> - Some(entry) => entry.clone(),
> - };
> - ScopedDir {
> - entry: ManuallyDrop::new(Entry::dynamic_dir(name,
> Some(parent_entry))),
> - _phantom: PhantomData,
> - }
> - }
> - #[cfg(not(CONFIG_DEBUG_FS))]
> - ScopedDir::empty()
> - }
> -
> /// Creates a new scope, which is a directory associated with some data
> `T`.
> ///
> /// The created directory will be a subdirectory of `self`. The `init`
> closure is called to
> @@ -427,7 +406,7 @@ fn scoped_dir<'data>(&self, name: &CStr) ->
> ScopedDir<'data, 'static> {
> /// The entire directory tree created within the scope will be removed
> when the returned
> /// `Scope` handle is dropped.
> pub fn scope<'a, T: 'a, E: 'a, F>(
> - &'a self,
> + &self,
> data: impl PinInit<T, E> + 'a,
> name: &'a CStr,
> init: F,
> @@ -435,8 +414,22 @@ pub fn scope<'a, T: 'a, E: 'a, F>(
> where
> F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) +
> 'a,
> {
> - Scope::new(data, |data| {
> - let scoped = self.scoped_dir(name);
> + // Clone the parent entry so the closure doesn't need to borrow
> `self`.
> + #[cfg(CONFIG_DEBUG_FS)]
> + let parent_entry = self.0.clone();
This isn't necessarily a show-stopper, but this lets us create a weird
intermediate state where from the user's PoV, there are no references
to a directory (they don't have the `Dir` anymore, and a `Scope`
doesn't exist yet, just a `PinInit<Scope>`), but the directory is held
alive. It feels weird, but I suppose users won't usually have a
`PinInit<Scope>` around for long.
The weirdness in debugging would look like:
1. Create dir
2. See dir in filesystem
3. Call `scope`, but don't actually activate the PinInit yet (either
due to coding error or because the target data structure won't be
available until later.
4. See no subdirectories under dir in filesystem
5. Drop `dir`
6. See dir still hanging around
This would also happen with my `pin_init_scope` suggestion, but at
least then user code would be explicitly capturing the `Dir` right in
front of them in the existing-dir case, and in the "create-or-lookup
new dir" case like you've got here, would delay directory creation
until we're actually instantiating the directory somewhere.
> +
> + Scope::new(data, move |data| {
> + #[cfg(CONFIG_DEBUG_FS)]
> + let scoped = match parent_entry {
> + None => ScopedDir::empty(),
> + Some(entry) => ScopedDir {
> + entry: ManuallyDrop::new(Entry::dynamic_dir(name,
> Some(entry))),
> + _phantom: PhantomData,
> + },
> + };
> + #[cfg(not(CONFIG_DEBUG_FS))]
> + let scoped = ScopedDir::empty();
> +
> init(data, &scoped);
> scoped.into_entry()
> })
> --
> 2.52.0
>
>
On Wed, Dec 17, 2025 at 5:49 PM Timur Tabi <[email protected]> wrote:
>
> Dir::scope() was declared as taking &'a self, which tied the returned
> PinInit's lifetime to the Dir. This prevented using scope() with a
> locally-created Dir:
>
> let dir = Dir::lookup(...).unwrap_or_else(Dir::empty);
> let scope = dir.scope(...); // Error: returns value referencing local
>
> The borrow was unnecessary since scoped_dir() internally clones the
> Arc<Entry>. Fix this by cloning self.0 before the closure, allowing the
> closure to capture the cloned value via move instead of borrowing self.
>
> This also removes the now-unused scoped_dir() helper method, inlining
> its logic directly into scope().
>
> Signed-off-by: Timur Tabi <[email protected]>
> ---
> rust/kernel/debugfs.rs | 41 +++++++++++++++++------------------------
> 1 file changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index b0cfe22982d6..35f9cbb261e7 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -397,27 +397,6 @@ pub fn write_callback_file<'a, T, E: 'a, W>(
> self.create_file(name, data, file_ops)
> }
>
> - // While this function is safe, it is intentionally not public because
> it's a bit of a
> - // footgun.
> - //
> - // Unless you also extract the `entry` later and schedule it for `Drop`
> at the appropriate
> - // time, a `ScopedDir` with a `Dir` parent will never be deleted.
> - fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
> - #[cfg(CONFIG_DEBUG_FS)]
> - {
> - let parent_entry = match &self.0 {
> - None => return ScopedDir::empty(),
> - Some(entry) => entry.clone(),
> - };
> - ScopedDir {
> - entry: ManuallyDrop::new(Entry::dynamic_dir(name,
> Some(parent_entry))),
> - _phantom: PhantomData,
> - }
> - }
> - #[cfg(not(CONFIG_DEBUG_FS))]
> - ScopedDir::empty()
> - }
> -
> /// Creates a new scope, which is a directory associated with some data
> `T`.
> ///
> /// The created directory will be a subdirectory of `self`. The `init`
> closure is called to
> @@ -427,7 +406,7 @@ fn scoped_dir<'data>(&self, name: &CStr) ->
> ScopedDir<'data, 'static> {
> /// The entire directory tree created within the scope will be removed
> when the returned
> /// `Scope` handle is dropped.
> pub fn scope<'a, T: 'a, E: 'a, F>(
> - &'a self,
> + &self,
> data: impl PinInit<T, E> + 'a,
> name: &'a CStr,
> init: F,
> @@ -435,8 +414,22 @@ pub fn scope<'a, T: 'a, E: 'a, F>(
> where
> F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) +
> 'a,
> {
> - Scope::new(data, |data| {
> - let scoped = self.scoped_dir(name);
> + // Clone the parent entry so the closure doesn't need to borrow
> `self`.
> + #[cfg(CONFIG_DEBUG_FS)]
> + let parent_entry = self.0.clone();
> +
> + Scope::new(data, move |data| {
> + #[cfg(CONFIG_DEBUG_FS)]
> + let scoped = match parent_entry {
> + None => ScopedDir::empty(),
> + Some(entry) => ScopedDir {
> + entry: ManuallyDrop::new(Entry::dynamic_dir(name,
> Some(entry))),
> + _phantom: PhantomData,
> + },
> + };
> + #[cfg(not(CONFIG_DEBUG_FS))]
> + let scoped = ScopedDir::empty();
> +
> init(data, &scoped);
> scoped.into_entry()
> })
> --
> 2.52.0
>
>
