[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DAAG6HNJ42IJ.1AWL4SUJDUGO5@kernel.org>
Date: Sat, 31 May 2025 17:23:00 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Danilo Krummrich" <dakr@...nel.org>
Cc: <gregkh@...uxfoundation.org>, <rafael@...nel.org>, <ojeda@...nel.org>,
<alex.gaynor@...il.com>, <boqun.feng@...il.com>, <gary@...yguo.net>,
<bjorn3_gh@...tonmail.com>, <benno.lossin@...ton.me>,
<a.hindborg@...nel.org>, <aliceryhl@...gle.com>, <tmgross@...ch.edu>,
<chrisi.schrefl@...il.com>, <rust-for-linux@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 6/7] rust: miscdevice: expose the parent device as
&Device<Bound>
On Sat May 31, 2025 at 2:39 PM CEST, Danilo Krummrich wrote:
> On Sat, May 31, 2025 at 02:10:08PM +0200, Benno Lossin wrote:
>> On Sat May 31, 2025 at 12:46 PM CEST, Danilo Krummrich wrote:
>> > But, I agree that we should not name it `Ptr`, probably should never have been
>> > named `Ptr`, but `Data`, `Private` or similar.
>> >
>> >> Would that also make sense to use as a general change? So don't store
>> >> `Self::Ptr`, but `Self` directly?
>> >
>> > I think it can't be `Self`, see above.
>>
>> The rust_misc_device example would still work if we changed this to
>> `Self`. Now it's not a complicated user of the API and someone might
>> want to store `Self` in an `Arc` and then store that as the private
>> data, as the MiscDevice is also referenced from somewhere else. But I
>> don't know if that is common or an intended use-case :)
>>
>> For simple use-cases however, I think that `Self` definitely is the
>> right choice (as opposed to `Pin<KBox<Self>>` for example, as that has
>> an extra allocation :)
>
> The data returned by open() can be anything. It can also be some arbitrary
> Arc<T> that already exists and is looked up in open(). It can also be something
> new that is created within open() and requires in-place initialization.
>
> So, if we want to change this, we could return an `impl PinInit<Self, Error>` as
> you suggest above and initialize it in-place in
> `RawDeviceRegistration::private`.
>
> I agree that this is the correct thing to do, but that really sounds like a
> subsequent patch.
Sounds good.
---
Cheers,
Benno
Powered by blists - more mailing lists