[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8DA337FC-2395-472A-9264-C6F656A1263D@nvidia.com>
Date: Tue, 10 Feb 2026 21:43:44 +0000
From: Joel Fernandes <joelagnelf@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Maarten
Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
<airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Jonathan Corbet
<corbet@....net>, Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>, Jani Nikula
<jani.nikula@...ux.intel.com>, Joonas Lahtinen
<joonas.lahtinen@...ux.intel.com>, Vivi Rodrigo <rodrigo.vivi@...el.com>,
Tvrtko Ursulin <tursulin@...ulin.net>, Rui Huang <ray.huang@....com>, Matthew
Auld <matthew.auld@...el.com>, Matthew Brost <matthew.brost@...el.com>, Lucas
De Marchi <lucas.demarchi@...el.com>, Thomas Hellström
<thomas.hellstrom@...ux.intel.com>, Helge Deller <deller@....de>, Alice Ryhl
<aliceryhl@...gle.com>, 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 <lossin@...nel.org>, Andreas
Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, John
Hubbard <jhubbard@...dia.com>, Alistair Popple <apopple@...dia.com>, Timur
Tabi <ttabi@...dia.com>, Edwin Peer <epeer@...dia.com>, Alexandre Courbot
<acourbot@...dia.com>, Andrea Righi <arighi@...dia.com>, Andy Ritger
<ARitger@...dia.com>, Zhi Wang <zhiw@...dia.com>, Balbir Singh
<balbirs@...dia.com>, Philipp Stanner <phasta@...nel.org>, Elle Rhumsaa
<elle@...thered-steel.dev>, Daniel Almeida <daniel.almeida@...labora.com>,
"joel@...lfernandes.org" <joel@...lfernandes.org>,
"nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"rust-for-linux@...r.kernel.org" <rust-for-linux@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
"intel-xe@...ts.freedesktop.org" <intel-xe@...ts.freedesktop.org>,
"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>
Subject: Re: [PATCH -next v8 2/3] rust: gpu: Add GPU buddy allocator bindings
Agreed with all these comments, will revise accordingly.
Thanks.
> On Feb 10, 2026, at 4:10 PM, Danilo Krummrich <dakr@...nel.org> wrote:
>
> On Tue Feb 10, 2026 at 9:09 PM CET, Joel Fernandes wrote:
>>>> +impl GpuBuddyInner {
>>>> + /// Create a pin-initializer for the buddy allocator.
>>>> + fn new(params: &GpuBuddyParams) -> impl PinInit<Self, Error> {
>>>
>>> I think we can just pass them by value, they shouldn't be needed anymore after
>>> the GpuBuddy instance has been constructed.
>>
>> Dave Airlie specifically reviewed this in RFC v6 and recommended passing by
>> reference rather than by value [2]:
>>
>> "maybe we should pass them as non-mutable references, but I don't think
>> there is any point in passing them by value ever."
>>
>> The params are also reused in practice -- the doc examples show the same
>> `GpuBuddyParams` being used repeatedly. References
>> avoid unnecessary copies for this reuse pattern.
>
> Well, that's for GpuBuddyAllocParams, those are indeed reused and shouldn't be
> copied all the time.
>
> But my comment was about GpuBuddyParams, I don't see a reason those are reused
> (neither are they in the example), so it makes more sense to pass them by value,
> such that they are consumed. (I.e. I'm not asking for adding Copy/Clone.)
>
>>
>> [2] https://lore.kernel.org/all/CAPM=9tyL_Cq3+qWc4A41p7eqnNDLS1APUEeUbaQyJ46YDkipVw@mail.gmail.com/
>>
>>>> + pub fn new(params: &GpuBuddyParams) -> Result<Self> {
>>>
>>> Same here, we should be able to take this by value.
>>
>> Same reasoning as above.
>>
>>>> + pub fn alloc_blocks(&self, params: &GpuBuddyAllocParams) -> Result<Arc<AllocatedBlocks>> {
>>>
>>> Why do we force a reference count here? I think we should just return
>>> impl PinInit<AllocatedBlocks, Error> and let the driver decide where to
>>> initialize the object, no?
>>>
>>> I.e. what if the driver wants to store additional data in a driver private
>>> structure? Then we'd need two allocations otherwise and another reference count
>>> in the worst case.
>>
>> Good point. The reason I had `Arc` was to anticipate potential shared ownership
>> use cases, but at the moment there is no such use case
>> in nova-core -- each `AllocatedBlocks` has a single owner.
>
> Sure, but drivers can always pass an impl PinInit to Arc::pin_init() themselves.
>
>> I'll change this to return `impl PinInit<AllocatedBlocks, Error>` in the next
>> version. If a shared ownership use case arises later, we
>> can always add an `Arc`-returning convenience wrapper.
>
> I don't think we should, don't give drivers a reason to go for more allocations
> they actually need for convinience.
Powered by blists - more mailing lists