venus: ensure object id is unique

Currently driver side heap alloc obj ptr is used as object id, which is
used on the renderer side for actual vk obj mapping. However, this adds
an implicit dependency between any driver obj destroy/free and new obj
create/allocate because the heap obj freed up can be immediately
reallocated out.

With venus moving to multi-ring, the ordering between asynchronous obj
destroy/free and new obj create/allocate has to be guaranteed via driver
side non-primary ring submission always waiting for primary ring idle.
This can defeat the purpose of multi-ring in certain scenarios. So this
change adds a way to assign unique id to object.

Even before multi-ring, the unique object id can make device and queue
object alloc/free more robust without hidden ordering requirements. This
also fixes some oom cts which can intentionally fail the submission of
an object destroy (renderer side obj is still present) while the driver
side freed object ptr being reused for another object creating, causing
object id reuse at renderer side object table.

Signed-off-by: Yiwei Zhang <zzyiwei@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27083>

src/virtio/vulkan/vn_command_buffer.c

@@ -719,11 +719,6 @@ vn_DestroyCommandPool(VkDevice device,
 
    alloc = pAllocator ? pAllocator : &pool->allocator;
 
-   /* We must emit vkDestroyCommandPool before freeing the command buffers in
-    * pool->command_buffers.  Otherwise, another thread might reuse their
-    * object ids while they still refer to the command buffers in the
-    * renderer.
-    */
    vn_async_vkDestroyCommandPool(dev->primary_ring, device, commandPool,
                                  NULL);
 

src/virtio/vulkan/vn_common.c

@@ -57,6 +57,7 @@ static const struct debug_control vn_perf_options[] = {
    /* clang-format on */
 };
 
+uint64_t vn_next_obj_id = 1;
 struct vn_env vn_env;
 
 static void

src/virtio/vulkan/vn_common.h

@@ -34,6 +34,7 @@
 #include "util/os_time.h"
 #include "util/perf/cpu_trace.h"
 #include "util/simple_mtx.h"
+#include "util/u_atomic.h"
 #include "util/u_math.h"
 #include "util/xmlconfig.h"
 #include "vk_alloc.h"
@@ -302,6 +303,14 @@ vn_refcount_dec(struct vn_refcount *ref)
    return old == 1;
 }
 
+extern uint64_t vn_next_obj_id;
+
+static inline uint64_t
+vn_get_next_obj_id(void)
+{
+   return p_atomic_fetch_add(&vn_next_obj_id, 1);
+}
+
 uint32_t
 vn_extension_get_spec_version(const char *name);
 
@@ -356,7 +365,7 @@ vn_instance_base_init(
 {
    VkResult result = vk_instance_init(&instance->base, supported_extensions,
                                       dispatch_table, info, alloc);
-   instance->id = (uintptr_t)instance;
+   instance->id = vn_get_next_obj_id();
    return result;
 }
 
@@ -376,7 +385,7 @@ vn_physical_device_base_init(
    VkResult result = vk_physical_device_init(
       &physical_dev->base, &instance->base, supported_extensions, NULL, NULL,
       dispatch_table);
-   physical_dev->id = (uintptr_t)physical_dev;
+   physical_dev->id = vn_get_next_obj_id();
    return result;
 }
 
@@ -395,7 +404,7 @@ vn_device_base_init(struct vn_device_base *dev,
 {
    VkResult result = vk_device_init(&dev->base, &physical_dev->base,
                                     dispatch_table, info, alloc);
-   dev->id = (uintptr_t)dev;
+   dev->id = vn_get_next_obj_id();
    return result;
 }
 
@@ -413,7 +422,7 @@ vn_queue_base_init(struct vn_queue_base *queue,
 {
    VkResult result =
       vk_queue_init(&queue->base, &dev->base, queue_info, queue_index);
-   queue->id = (uintptr_t)queue;
+   queue->id = vn_get_next_obj_id();
    return result;
 }
 
@@ -429,7 +438,7 @@ vn_object_base_init(struct vn_object_base *obj,
                     struct vn_device_base *dev)
 {
    vk_object_base_init(&dev->base, &obj->base, type);
-   obj->id = (uintptr_t)obj;
+   obj->id = vn_get_next_obj_id();
 }
 
 static inline void

src/virtio/vulkan/vn_descriptor_set.c

@@ -411,10 +411,6 @@ vn_DestroyDescriptorPool(VkDevice device,
 
    alloc = pAllocator ? pAllocator : &pool->allocator;
 
-   /* We must emit vkDestroyDescriptorPool before freeing the sets in
-    * pool->descriptor_sets.  Otherwise, another thread might reuse their
-    * object ids while they still refer to the sets in the renderer.
-    */
    vn_async_vkDestroyDescriptorPool(dev->primary_ring, device, descriptorPool,
                                     NULL);
 

src/virtio/vulkan/vn_device.c

@@ -603,10 +603,6 @@ vn_DestroyDevice(VkDevice device, const VkAllocationCallbacks *pAllocator)
 
    vn_device_memory_report_fini(dev);
 
-   /* We must emit vkDestroyDevice before freeing dev->queues.  Otherwise,
-    * another thread might reuse their object ids while they still refer to
-    * the queues in the renderer.
-    */
    vn_async_vkDestroyDevice(dev->primary_ring, device, NULL);
 
    /* We must emit vn_call_vkDestroyDevice before releasing bound ring_idx.

src/virtio/vulkan/vn_device_memory.c

@@ -120,7 +120,7 @@ vn_device_memory_pool_grow_alloc(struct vn_device *dev,
    if (!mem)
       return VK_ERROR_OUT_OF_HOST_MEMORY;
 
-   vn_object_set_id(mem, (uintptr_t)mem, VK_OBJECT_TYPE_DEVICE_MEMORY);
+   vn_object_set_id(mem, vn_get_next_obj_id(), VK_OBJECT_TYPE_DEVICE_MEMORY);
 
    VkResult result = vn_device_memory_alloc_simple(dev, mem, &alloc_info);
    if (result != VK_SUCCESS)
@@ -585,7 +585,7 @@ vn_AllocateMemory(VkDevice device,
    if (!mem)
       return vn_error(dev->instance, VK_ERROR_OUT_OF_HOST_MEMORY);
 
-   vn_object_set_id(mem, (uintptr_t)mem, VK_OBJECT_TYPE_DEVICE_MEMORY);
+   vn_object_set_id(mem, vn_get_next_obj_id(), VK_OBJECT_TYPE_DEVICE_MEMORY);
 
    VkResult result;
    if (mem->base.base.ahardware_buffer) {

src/virtio/vulkan/vn_image.c

@@ -448,7 +448,7 @@ vn_image_create(struct vn_device *dev,
    if (!img)
       return VK_ERROR_OUT_OF_HOST_MEMORY;
 
-   vn_object_set_id(img, (uintptr_t)img, VK_OBJECT_TYPE_IMAGE);
+   vn_object_set_id(img, vn_get_next_obj_id(), VK_OBJECT_TYPE_IMAGE);
 
    VkResult result = vn_image_init(dev, create_info, img);
    if (result != VK_SUCCESS) {
@@ -482,7 +482,7 @@ vn_image_create_deferred(struct vn_device *dev,
    if (!img)
       return VK_ERROR_OUT_OF_HOST_MEMORY;
 
-   vn_object_set_id(img, (uintptr_t)img, VK_OBJECT_TYPE_IMAGE);
+   vn_object_set_id(img, vn_get_next_obj_id(), VK_OBJECT_TYPE_IMAGE);
 
    VkResult result = vn_image_deferred_info_init(img, create_info, alloc);
    if (result != VK_SUCCESS) {

src/virtio/vulkan/vn_pipeline.c

@@ -461,8 +461,6 @@ vn_get_target_ring(struct vn_device *dev)
        * ready on the renderer side.
        *
        * TODO:
-       * - For pipeline objects, avoid object id re-use between async pipeline
-       *   destroy on the primary ring and sync pipeline create on TLS ring.
        * - For pipeline create, track ring seqnos of layout and renderpass
        *   objects it depends on, and only wait for those seqnos once.
        * - For pipeline cache retrieval, track ring seqno of pipeline cache