[Mesa-dev] [PATCH 5/9] anv: Add vma_heap allocators in anv_device

[Scott D Phillips]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Quoting Scott D Phillips (2018-05-02 17:01:05)
>> +bool
>> +anv_vma_alloc(struct anv_device *device, struct anv_bo *bo)
>> +{
>> +   if (!(bo->flags & EXEC_OBJECT_PINNED))
>> +      return true;
>> +
>> +   pthread_mutex_lock(&device->vma_mutex);
>> +
>> +   bo->offset = 0;
>
> So bo are device scoped. There can only be a single vma per bo, so why
> not store the vma node inside the bo? No extra allocations, no
> searching in anv_vma_close() (a linear walk!!! Granted you have the
> excuse of doing a full walk for list validation on top of that).

ah, I see what you're saying, something like:

bool util_vma_heap_alloc(struct util_vma_heap *heap, uint64_t size,
   uint64_t alignment, struct util_vma_heap_node *node);

to place the node in the vma, where node has a list_head and uint64_t
addr. And then free becomes a few data structure twiddles.

> I guess you don't have much that stresses the vma manager :)

Right, Jason's advice is that the "good enough" point for vulkan is
fairly low, where the model is that memory allocations are not happening
dynamically and are few in number (~100s, I guess). There's lots of room
for optimization in the allocator, for sure.

The hope is that that's localized behind the allocator interface and
everything else is fine in the series.

> The decision to split low/high ranges rather than have a up/down
> allocator wants a few words of explanation.

I suppose I saw that as a six-of-one/half-dozen-of-another type
thing. It would save having two allocators, but low-memory allocation
would still be funny with {mem=alloc; if (it wasn't actually low)
{free;fail;}}. Maybe that logic could get hoisted into the low-side
allocation function?

That being said, I don't really mind changing to a double sided
allocator if there's a good argument in favor.

> -Chris