Re: [PATCH v3 2/9] rust: debugfs: add lookup contructor

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> Add a new constructor for Dir that uses the debugfs_lookup() API to
> obtain a reference to an existing debugfs directory entry.
>
> The key difference from Dir::new() and Dir::subdir() is the cleanup
> semantics: when a Dir obtained via lookup() is dropped, it calls
> dput() to release the reference rather than debugfs_remove() which
> would delete the directory.
>
> To implement this cleanup distinction, the Entry class now includes
> an is_lookup boolean that specifies how the entry was created and
> therefore how it should be dropped.

I have two main comments about this.

(1) I think it would be better to wrap "lookup entries" into a new type in order
to account for the fact that they do not behave like regular entries. I.e. when
a "lookup entry" is dropped, it does not cause the entry to disappear from the
file system. Analogously, when the regular entry is dropped it disappears from
the file system regardless of whether a "lookup entry" still has a reference
count.

(2) The commit message lacks the motiviation for adding this functionality,
which is only for the purpose of a workaround of an unrelated limitation with
accessing the fields of a module structure (which Gary already works on a
solution for).

I understand it might take a while until the solution Gary works on is ready and
I'm perfectly fine with a temporary workaround. But, this workaround has to be
on nova-core only and not introduce a feature in core code that in the near
future becomes dead code.

If we have a use-case for lookup() elsewhere, I'd like to see it first. But if
we don't I prefer not to add this feature as a pure workaround for something
else.

In fact, I think that debugfs_lookup() rarely is the correct solution and more
often indicates some kind of (design) issue, like in this case.

- Danilo