Quoting Scott D Phillips (2018-05-02 20:24:01)
> Chris Wilson <ch...@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.

Yes.

> > 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.

I'm just looking at it from the pov of drm_mm which this "replaces" (not
a complete replacement as the kernel still calls into drm_mm to validate
what the user tells us, but only on first binding). The main lesson,
imho, being that allocation and linear lists are terrible.

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

Also a clear API begs for unittests ;)

> > 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.

I don't mind either way, what I care about is that the rationale for
picking one over the other is recorded. Knowing what your expectations
are helps us all live up to them.
-Chris
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to