Message-ID: <CAGSQo02XEuHGJ8YMvBp=S=3sWt=0R-wEkdkTp7aEsnaCBEB1wg@mail.gmail.com>
Date: Tue, 10 Jun 2025 10:54:24 -0700
From: Matthew Maurer <mmaurer@...gle.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Alice Ryhl <aliceryhl@...gle.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Benno Lossin <lossin@...nel.org>, 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>, 
	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

I'm going to try to rework the patch based on the feedback here, and
have a few things I want to check on to reduce cycling:

1. Every approach described in this thread will result in the creation
APIs (create a root dir, create a subdir, create a file) being
fallible (returns `Result<>`) because they use `Arc` in the
implementation which fundamentally allocates. I previously got the
feedback that the debugfs API should have no possibility of handleable
errors. This would *not* mean that we were signaling the errors out of
the raw debugfs bindings into Rust, but Rust would be able to detect
`Arc` allocation failures unless you really want me to be hiding
allocation failures (I could theoretically do this by generating a
dummy object on allocation failure, but this would add complexity and
seems questionable).
2. If we're not using borrowing to statically prevent entries from
outliving their parents, there are two possible semantics for what to
do when an entry *does* outlive its parent:
  a. The parent doesn't actually get `debugfs_remove` called until all
its child objects have left scope too. This can be accomplished by
just having a `_parent: Option<Arc<Entry>>` in each `Entry` structure.
  b. We get ghost objects, e.g. the child object is still around, but
it is somehow able to discover that its `dentry` pointer is no longer
valid and avoids trying to use it. The best solution I have to this is
to again maintain an optional parent `Arc`, but also have `Entry` hold
an additional refcount member. When you want to create a subdir, a
file in a directory, or delete any of those, you'd walk the spine of
parents, trying to claim the extra refcount on everything. Once you've
got it, you can safely delete yourself or create a subdirectory or
file. Then, whoever is last out of the extra refcount (either the
owning object or someone holding a reference while they perform an
operation on themselves) can try to clean up, again by claiming the
spine above them.
3. If we're not supporting `leak` or lifetime-based `File` anymore,
then I don't think my method of attaching data is needed anymore. It
was complex primarily in order to support statically enforced
lifetimes on dirs/subdirs/files, and several of the tricks I was using
don't work without a lifetime on the `File` type. Instead, I'll make
it so that `File` objects can attach a `ForeignOwnable`, with the
expected common cases being either `&'static MyData` or `Arc<MyData>`.

Unless I get additional feedback, I'm currently implementing 2-a,
using `ForeignOwnable` based attachments of data to files, and
returning `Result` where the only actual failure possibility is
allocation failure.

Powered by blists - more mailing lists