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

[Benno Lossin]

On Tue Jul 1, 2025 at 9:21 PM CEST, Danilo Krummrich wrote:
> On Tue, Jul 01, 2025 at 11:11:13AM -0700, Matthew Maurer wrote:
>> On Tue, Jul 1, 2025 at 8:10 AM Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>> >         impl Firmware {
>> >            pub fn new(&dir: debugfs::Dir, buffer: [u8]) -> impl PinInit<Self> {
>> >               pin_init!(Self {
>> >                  minor <- dir.create_file("minor", 1),
>> >                  major <- dir.create_file("major", 2),
>> >                  buffer <- dir.create_file("buffer", buffer),
>> >               })
>> >            }
>> >         }
>> >
>> >         // This is the only allocation we need.
>> >         let fw = KBox::pin_init(Firmware::new(...), GFP_KERNEL)?;
>> >
>> > With this everything is now in a single allocation and since we're using
>> > pin-init, Dir::create_file() can safely store pointers of the corresponding data
>> > in debugfs_create_file(), since this structure is guaranteed to be pinned in
>> > memory.
>> >
>> > Actually, we can also implement *only this*, since with this my previous example
>> > would just become this:
>> 
>> If we implement *only* pinned files, we run into an additional problem
>> - you can't easily extend a pinned vector. This means that you cannot
>> have dynamically created devices unless you're willing to put every
>> new `File` into its own `Box`, because you aren't allowed to move any
>> of the previously allocated `File`s for a resize.
>> 
>> Where previously you would have had
>> 
>> ```
>> debug_files: Vec<File>
>> ```
>> 
>> you would now have
>> 
>> ```
>> debug_files: Vec<PinBox<File<T>>>
>> ```
>
> Stuffing single File instances into a Vec seems like the wrong thing to do.
>
> Instead you may have instances of some data structure that is created
> dynamically in your driver that you want to expose through debugfs.
>
> Let's say you have (userspace) clients that can be registered arbitrarily, then
> you want a Vec<Client>, which contains the client instances. In order to provide
> information about the Client in debugfs you then have the client embed things as
> discussed above.
>
> 	struct Client {
> 	   id: File<ClientId>,
> 	   data: File<ClientData>,
> 	   ...
> 	}
>
> I think that makes much more sense than keeping a Vec<Arc<Client>> *and* a
> Vec<File> separately. Also, note that with the above, your Client instances
> don't need to be reference counted anymore.
>
> I think this addresses the concerns below.

You still have the issue that `Client` now needs to be pinned and the
vector can't be resized. But if you know that it's bounded, then we
could just make `Pin<Vec<T>>` work as expected (not relocating the
underlying allocation by not exposing `push`, only
`push_within_capacity`).

We also could have a `SegmentedVec<T>` that doesn't move elements.
Essentially it is
    
    enum SegmentedVec<T> {
        Cons(Segment<T>, KBox<SegmentedVec<T>>)
        Nul,
    }

    struct Segment<T> {
        elements: [T; 16]
    }

or make the segments variable-sized and grow them accordingly.

---
Cheers,
Benno