Message-Id: <D9WR11ILWC2X.2TIYICAG4H1Q1@kernel.org>
Date: Thu, 15 May 2025 14:55:46 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Danilo Krummrich" <dakr@...nel.org>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>
Cc: "Matthew Maurer" <mmaurer@...gle.com>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <benno.lossin@...ton.me>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
 "Rafael J. Wysocki" <rafael@...nel.org>, "Sami Tolvanen"
 <samitolvanen@...gle.com>, "Timur Tabi" <ttabi@...dia.com>,
 <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v5 4/4] rust: samples: Add debugfs sample

On Thu May 15, 2025 at 2:37 PM CEST, Danilo Krummrich wrote:
> On Thu, May 15, 2025 at 01:43:09PM +0200, Greg Kroah-Hartman wrote:
>> On Thu, May 15, 2025 at 10:59:44AM +0200, Benno Lossin wrote:
>> > On Wed May 14, 2025 at 11:55 PM CEST, Matthew Maurer wrote:
>> > > On Wed, May 14, 2025 at 2:07 AM Danilo Krummrich <dakr@...nel.org> wrote:
>> > >> However, I really think we should keep the code as it is in this version and
>> > >> just don't provide an example that utilizes ManuallyDrop and forget().
>> > >>
>> > >> I don't see how the idea of "manually dropping" (sub-)directories and files
>> > >> provides any real value compared to just storing their instance in a driver
>> > >> structure as long as they should stay alive, which is much more intuitive
>> > >> anyways.
>> > >
>> > > We can't easily do this, because dropping a root directory recursively
>> > > drops everything underneath it. This means that if I have
>> > >
>> > > foo/
>> > >   - bar/
>> > >   - baz/
>> > >
>> > > Then my directory handle for `bar` have to be guaranteed to outlive my
>> > > directory handle for `foo` so that I know it's didn't get deleted
>> > > under me. This is why they have a borrow onto their parent directory.
>> > > This borrow means that you can't (without `unsafe`, or something like
>> > > `yoke`) keep handles to `foo` and `bar` in the same struct.
>> > 
>> > Is there no refcount that we can use instead of borrowing? I guess not,
>> > since one can call `debugfs_remove`. What about a refcount on the rust
>> > side? or is debugfs not used for "debugging" and needs to have the
>> > performance of no refcount?
>> 
>> debugfs should never have any performance issues (i.e. you don't use it
>> for performant things.)
>> 
>> So refcount away!  That should never be an issue.
>
> Reference counting (parent) directories should lead to a much cleaner solution.
>
> I mentioned that previously, but also said in that context that it's a bit
> contrary to how the C API is utilized currently, which usually isn't desired.

We could also change the C side to use refcounting :) It is probably a
bigger change (I have no idea how common the usage of debugfs is).

In my mind, it would also allow the C side to benefit from the same
"drop the dirs that you don't need anymore and all subdirs will be
removed if they also aren't referenced any longer" thing.

> However, if we're fine with that I think it's superior to the borrowing
> solution, which requires keep(). IMHO keep() is a footgun in general, even if
> not callable for "root" directories.

I would prefer refcounting over forgetting, it much more clearly shows
who owns the debugfs dirs. Also, in which cases would one not call
`.keep()`? The USB example from the other thread comes to mind, but
there you might be able to borrow a `Dir<'static` for `'static`, are
there other cases?

---
Cheers,
Benno

Powered by blists - more mailing lists