[Mesa-dev] [PATCH] anv/allocator: Avoid race condition in anv_block_pool_map.
Jason Ekstrand
jason at jlekstrand.net
Wed Jan 23 22:20:39 UTC 2019
On Wed, Jan 23, 2019 at 2:45 PM Rafael Antognolli <
rafael.antognolli at intel.com> wrote:
> Accessing bo->map and then pool->center_bo_offset without a lock is
> racy. One way of avoiding such race condition is to store the bo->map +
> center_bo_offset into pool->map at the time the block pool is growing,
> which happens within a lock.
>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> Reported-by: Ian Romanick <idr at freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109442
> Fixes: fc3f58832015cbb177179e7f3420d3611479b4a9
> ---
> src/intel/vulkan/anv_allocator.c | 10 ++++++++--
> src/intel/vulkan/anv_private.h | 13 +++++++++++++
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_allocator.c
> b/src/intel/vulkan/anv_allocator.c
> index 89f26789c85..0bfe55bf684 100644
> --- a/src/intel/vulkan/anv_allocator.c
> +++ b/src/intel/vulkan/anv_allocator.c
> @@ -437,6 +437,7 @@ anv_block_pool_init(struct anv_block_pool *pool,
> pool->nbos = 0;
> pool->size = 0;
> pool->start_address = gen_canonical_address(start_address);
> + pool->map = NULL;
>
> /* This pointer will always point to the first BO in the list */
> pool->bo = &pool->bos[0];
> @@ -575,6 +576,7 @@ anv_block_pool_expand_range(struct anv_block_pool
> *pool,
>
> /* Now that we successfull allocated everything, we can write the new
> * center_bo_offset back into pool. */
> + pool->map = map + center_bo_offset;
>
We should only set this when we are NOT using softpin. Otherwise, we'll
have a non-NULL map pointer that doesn't do what it looks like it does.
--Jason
> pool->center_bo_offset = center_bo_offset;
>
> /* For block pool BOs we have to be a bit careful about where we place
> them
> @@ -670,8 +672,12 @@ anv_block_pool_get_bo(struct anv_block_pool *pool,
> int32_t *offset)
> void*
> anv_block_pool_map(struct anv_block_pool *pool, int32_t offset)
> {
> - struct anv_bo *bo = anv_block_pool_get_bo(pool, &offset);
> - return bo->map + pool->center_bo_offset + offset;
> + if (pool->bo_flags & EXEC_OBJECT_PINNED) {
> + struct anv_bo *bo = anv_block_pool_get_bo(pool, &offset);
> + return bo->map + offset;
> + } else {
> + return pool->map + offset;
> + }
> }
>
> /** Grows and re-centers the block pool.
> diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> index 3889065c93c..110b2ccf023 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -663,6 +663,19 @@ struct anv_block_pool {
> */
> uint32_t center_bo_offset;
>
> + /* Current memory map of the block pool. This pointer may or may not
> + * point to the actual beginning of the block pool memory. If
> + * anv_block_pool_alloc_back has ever been called, then this pointer
> + * will point to the "center" position of the buffer and all offsets
> + * (negative or positive) given out by the block pool alloc functions
> + * will be valid relative to this pointer.
> + *
> + * In particular, map == bo.map + center_offset
> + *
> + * DO NOT access this pointer directly. Use anv_block_pool_map()
> instead,
> + * since it will handle the softpin case as well, where this points to
> NULL.
> + */
> + void *map;
> int fd;
>
> /**
> --
> 2.17.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190123/32c58b6a/attachment.html>
More information about the mesa-dev
mailing list