Re: [PATCH v8 4/6] rust: debugfs: Support arbitrary owned backing for File

[Matthew Maurer]

On Mon, Jun 30, 2025 at 10:39 AM Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>
> On 6/30/25 7:34 PM, Matthew Maurer wrote:
> > On Mon, Jun 30, 2025 at 10:30 AM Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
> >>
> >> On 6/28/25 1:18 AM, Matthew Maurer wrote:
> >>> +    fn create_file<D: ForeignOwnable>(&self, _name: &CStr, data: D) -> File
> >>> +    where
> >>> +        for<'a> D::Borrowed<'a>: Display,
> >>> +    {
> >>> +        File {
> >>> +            _foreign: ForeignHolder::new(data),
> >>> +        }
> >>>        }
> >>
> >> What's the motivation for the ForeignHolder abstraction? Why not just make it
> >> File<D> and store data directly?
> >
> > 1. A `File<D>` can't be held in collection data structures as easily
> > unless all your files contain the *same* backing type.
>
> That sounds reasonable.
>
> > 2. None of the APIs or potential APIs for `File` care about which type
> > it's wrapping, nor are they supposed to. If nothing you can do with a
> > `File` is different depending on the backing type, making it
> > polymorphic is just needlessly confusing.
>
> What if I want to access file.data() and do something with the data? Then I'd
> necessarily need to put my data in an Arc and reference count it to still be
> able to access it.
>
> That doesn't seem like a reasonable requirement to be able to access data
> exposed via debugfs.

`pub fn data(&self) -> D` would go against my understanding of Greg's
request for DebugFS files to not really support anything other than
delete. I was even considering making `D` not be retained in the
disabled debugfs case, but left it in for now for so that the
lifecycles wouldn't change.

If you want a `.data()` function, I can add it in, but I don't think
it'll improve flexibility in most cases. If you want to do something
with the data and it's not in an `Arc` / behind a handle of some kind,
you'll need something providing threadsafe interior mutability in the
data structure. If that's a lock, then I have a hard time believing
that `Arc<Mutex<T>>`(or if it's a global, a `&'static Mutex<T>`, which
is why I added that in the stack) is so much more expensive than
`Box<Mutex<T>>` that it's worth a more complex API. If it's an atomic,
e.g. `Arc<AtomicU8>`, then I can see the benefit to having
`Box<AtomicU8>` over that, but it still seems so slim that I think the
simpler "`File` is just a handle to how long the file stays alive, it
doesn't let you do anything else" API makes sense.