Re: [PATCH v5 4/4] rust: samples: Add debugfs sample

[Alice Ryhl <aliceryhl@google.com>]

On Thu, May 22, 2025 at 08:25:25AM +0200, Danilo Krummrich wrote:
> On Wed, May 21, 2025 at 10:43:26PM +0000, Alice Ryhl wrote:
> > > > * Possibly we have a way to drop a Rust object without removing it from
> > > >   the file system. In this case, it can never be accessed from Rust
> > > >   again, and the only way to remove it is to drop its parent directory.
> > > 
> > > I'm not sure about this one.
> > > 
> > > IIUC, this basically brings back the "keep() logic", which I still think is a
> > > footgun and should be avoided.
> > > 
> > > Also, we only needed this, since due to the borrowing design we couldn't store
> > > parent and child nodes in the same structure. With reference counting (or the
> > > logic above) this goes away.
> > > 
> > > I wrote the following in a previous conversation [1].
> > > 
> > > --
> > > 
> > > What I see more likely to happen is a situation where the "root" directory
> > > (almost) lives forever, and hence subsequent calls, such as
> > > 
> > > 	root.subdir("foo").keep()
> > > 
> > > effectively are leaks.
> > > 
> > > One specific example for that would be usb_debug_root, which is created in the
> > > module scope of usb-common and is used by USB host / gadget / phy drivers.
> > > 
> > > The same is true for other cases where the debugfs "root" is created in the
> > > module scope, but subsequent entries are created by driver instances. If a
> > > driver would use keep() in such a case, we'd effectively leak memory everytime a
> > > device is unplugged (or unbound in general).
> > 
> > Where is the leak? If the file is still visible in the file system, then
> > it's not a leak to still have the memory. If the file got removed by
> > removing its parent, then my intent is that this should free the memory
> > of the child object.
> 
> Well, take the case I described above, where the debugfs "root" is created in
> the module scope, but subsequent entries are created by driver instances. If a
> driver would use keep() in such a case, we'd effectively the file / directory
> (and subsequently also the corresponding memory) everytime a device is unplugged
> (or unbound in general)."
> 
> If the module is built-in the directory from the module scope is *never*
> removed, but the entries the driver e.g. creates in probe() for a particular
> device with keep() will pile up endlessly, especially for hot-pluggable devices.
> 
> (It's getting even worse when there's data bound to such a leaked file, that
> might even contain a vtable that is entered from any of the fops of the file.)
> 
> That'd be clearly a bug, but for the driver author calling keep() seems like a
> valid thing to do -- to me that's clearly a built-in footgun.

I mean, for cases such as this, I could imagine that you use `keep()` on
the files stored inside of the driver directory, but don't use it on the
directory. That way, you only have to keep a single reference to an
entire directory around, which may be more convenient.

But I also see your point. I don't think keep() is a critical part of
what I think the approach should be.

Alice