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+qwc4a41p7eqnndls1apueeubaqyj46ydki...@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.
