Skip to content

ANV: Allocate everything from the BO cache

Faith Ekstrand requested to merge gfxstrand/mesa:wip/anv-rework-bos into master

This MR is marked WIP because it's still pretty early and I've not yet completed it but I wanted to post it so that people (particularly @llandwerlin) can see where my mind is going and provide feedback. The basic idea here is to make ANV start allocating absolutely everything through the BO cache so that every BO in existence lives in a single struct util_sparse_array indexed by GEM handle. There are a number of reasons for this:

  1. We've had issues with the relocation path where the BO goes away between the execbuf and when we try to update anv_bo::offset. @llandwerlin has some patches which fix this for the batch buffers in his VK_KHR_timeline_semaphore MR (!2236 (merged)) but it also, in theory, exists for descriptor sets, query pools, and possibly other things. If everything is allocated from the BO cache and all struct anv_bo live in a sparse array, the data for anv_bo::offset exists for all of time the first time a BO with that GEM handle is allocated so the relocation update can't crash. There's still a data race but it's harmless.

  2. Right now, our resident BO tracking uses a hash set and this does actually show up on perf traces because hash sets aren't nearly as free as they teach you in CS school. If all BOs ever live in a sparse array, we can replace this hash set with a simple bitset indexed by GEM handle. This will be much more performant.

  3. The current algorithm used for building the validation list to hand off to the kernel relies on anv_bo::index which means we have to take a lock in order to build that list. If we have all BOs ever in an array, we can use a bitset to ensure uniqueness in the validation list. This should let us completely drop the lock from anv_queue_execbuf() in the softpin case.

  4. It's generally a positive cleanup. The more I've been looking at things, the more cases I've found of someone not setting the right BO flags or similar issues. If everything goes though the BO cache, it gives us the opportunity to sanitize things a bit.

Please give it a read if you're interested in Vulkan core internals and have comments. I welcome any feedback. I plan to pick this up on Monday in the hopes of (assuming @llandwerlin likes the direction) landing it along with timeline semaphores.

Edited by Faith Ekstrand

Merge request reports