[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DB0ZOHPFJR7T.1W3OF32GDL55O@kernel.org>
Date: Tue, 01 Jul 2025 22:09:10 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Danilo Krummrich" <dakr@...nel.org>
Cc: "Matthew Maurer" <mmaurer@...gle.com>, "Greg Kroah-Hartman"
<gregkh@...uxfoundation.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>, "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>, "Dirk
Behme" <dirk.behme@...bosch.com>
Subject: Re: [PATCH v8 4/6] rust: debugfs: Support arbitrary owned backing
for File
On Tue Jul 1, 2025 at 10:03 PM CEST, Benno Lossin wrote:
> On Tue Jul 1, 2025 at 9:58 PM CEST, Danilo Krummrich wrote:
>> On 7/1/25 9:46 PM, Benno Lossin wrote:
>>> 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:
>>>>> 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.
>>
>> That sounds a lot like the perfect application for XArray. :)
>
> Haha I didn't know this already existed in the kernel :) Yeah then we
> should make XArray work for this use-case.
Ah wait, I meant for `SegmentedVec<T>` to store multiple `T` in the same
allocation, so it would only grow sometimes and amortize the allocations
just like `Vec`. It seems to me that XArray only stores pointers, so you
have to box everything (which we're trying to avoid IIUC).
---
Cheers,
Benno
Powered by blists - more mailing lists