anv/allocator: Take the device mutex when growing a block pool
authorJason Ekstrand <jason.ekstrand@intel.com>
Tue, 15 Sep 2015 21:52:26 +0000 (14:52 -0700)
committerJason Ekstrand <jason.ekstrand@intel.com>
Fri, 18 Sep 2015 00:44:20 +0000 (17:44 -0700)
We don't have any locking issues yet because we use the pool size itself as
a mutex in block_pool_alloc to guarantee that only one thread is resizing
at a time.  However, we are about to add support for growing the block pool
at both ends.  This introduces two potential races:

 1) You could have two block_pool_alloc() calls that both try to grow the
    block pool, one from each end.

 2) The relocation handling code will now have to think about not only the
    bo that we use for the block pool but also the offset from the start of
    that bo to the center of the block pool.  It's possible that the block
    pool growing code could race with the relocation handling code and get
    a bo and offset out of sync.

Grabbing the device mutex solves both of these problems.  Thanks to (2), we
can't really do anything more granular.

src/vulkan/anv_allocator.c
src/vulkan/tests/block_pool_no_free.c
src/vulkan/tests/state_pool.c
src/vulkan/tests/state_pool_free_list_only.c
src/vulkan/tests/state_pool_no_free.c

index 121ce0392506cb50078ba0831cec1332654d69bd..201cc931cbb4040084c5eb253919047a5e687ee3 100644 (file)
@@ -287,6 +287,8 @@ anv_block_pool_grow(struct anv_block_pool *pool, uint32_t old_size)
    int gem_handle;
    struct anv_mmap_cleanup *cleanup;
 
+   pthread_mutex_lock(&pool->device->mutex);
+
    if (old_size == 0) {
       size = 32 * pool->block_size;
    } else {
@@ -295,17 +297,17 @@ anv_block_pool_grow(struct anv_block_pool *pool, uint32_t old_size)
 
    cleanup = anv_vector_add(&pool->mmap_cleanups);
    if (!cleanup)
-      return 0;
+      goto fail;
    *cleanup = ANV_MMAP_CLEANUP_INIT;
 
    if (old_size == 0)
       pool->fd = memfd_create("block pool", MFD_CLOEXEC);
 
    if (pool->fd == -1)
-      return 0;
+      goto fail;
 
    if (ftruncate(pool->fd, size) == -1)
-      return 0;
+      goto fail;
 
    /* First try to see if mremap can grow the map in place. */
    map = MAP_FAILED;
@@ -324,11 +326,11 @@ anv_block_pool_grow(struct anv_block_pool *pool, uint32_t old_size)
       cleanup->size = size;
    }
    if (map == MAP_FAILED)
-      return 0;
+      goto fail;
 
    gem_handle = anv_gem_userptr(pool->device, map, size);
    if (gem_handle == 0)
-      return 0;
+      goto fail;
    cleanup->gem_handle = gem_handle;
 
    /* Now that we successfull allocated everything, we can write the new
@@ -339,7 +341,13 @@ anv_block_pool_grow(struct anv_block_pool *pool, uint32_t old_size)
    pool->bo.map = map;
    pool->bo.index = 0;
 
+   pthread_mutex_unlock(&pool->device->mutex);
+
    return size;
+
+fail:
+   pthread_mutex_unlock(&pool->device->mutex);
+   return 0;
 }
 
 uint32_t
index 898a82b09094cc07e0b3f7eb5fc6869ba01c0d8f..d40504c4a87319b5cb030645b6275e50aa33e568 100644 (file)
@@ -51,6 +51,7 @@ static void run_test()
    struct anv_device device;
    struct anv_block_pool pool;
 
+   pthread_mutex_init(&device.mutex, NULL);
    anv_block_pool_init(&pool, &device, 16);
 
    for (unsigned i = 0; i < NUM_THREADS; i++) {
@@ -95,6 +96,7 @@ static void run_test()
    }
 
    anv_block_pool_finish(&pool);
+   pthread_mutex_destroy(&device.mutex);
 }
 
 int main(int argc, char **argv)
index e235ee9b394dd6f6de1eca2dafe71ce9ab647814..878ec19a595fee01a02607856a7bacaf56a81a1c 100644 (file)
@@ -38,6 +38,8 @@ int main(int argc, char **argv)
    struct anv_block_pool block_pool;
    struct anv_state_pool state_pool;
 
+   pthread_mutex_init(&device.mutex, NULL);
+
    for (unsigned i = 0; i < NUM_RUNS; i++) {
       anv_block_pool_init(&block_pool, &device, 256);
       anv_state_pool_init(&state_pool, &block_pool);
@@ -50,4 +52,6 @@ int main(int argc, char **argv)
       anv_state_pool_finish(&state_pool);
       anv_block_pool_finish(&block_pool);
    }
+
+   pthread_mutex_destroy(&device.mutex);
 }
index 9e89cf6425f7c98b12ec4a5e5b97f4d48308630f..2f4eb47fe45e2234e9ae64a6d99c65b26080351c 100644 (file)
@@ -37,6 +37,7 @@ int main(int argc, char **argv)
    struct anv_block_pool block_pool;
    struct anv_state_pool state_pool;
 
+   pthread_mutex_init(&device.mutex, NULL);
    anv_block_pool_init(&block_pool, &device, 4096);
    anv_state_pool_init(&state_pool, &block_pool);
 
@@ -61,4 +62,5 @@ int main(int argc, char **argv)
 
    anv_state_pool_finish(&state_pool);
    anv_block_pool_finish(&block_pool);
+   pthread_mutex_destroy(&device.mutex);
 }
index 4b3ca78974fd0a823b5f98d0c19d83f1290d9f40..4b248c2ee66b9ff7bb93918324c3671ca8b17217 100644 (file)
@@ -58,6 +58,7 @@ static void run_test()
    struct anv_block_pool block_pool;
    struct anv_state_pool state_pool;
 
+   pthread_mutex_init(&device.mutex, NULL);
    anv_block_pool_init(&block_pool, &device, 64);
    anv_state_pool_init(&state_pool, &block_pool);
 
@@ -106,6 +107,7 @@ static void run_test()
 
    anv_state_pool_finish(&state_pool);
    anv_block_pool_finish(&block_pool);
+   pthread_mutex_destroy(&device.mutex);
 }
 
 int main(int argc, char **argv)