From 896a7c28eb4b993751bb49659de7511b3fc68756 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Thu, 26 Mar 2020 12:57:36 -0500 Subject: [PATCH] anv/allocator: Use util_dynarray for blocks in anv_state_stream When we originally wrote a bunch of the allocation data structures, we re-used the GPU memory for CPU-side data structures. It's a bit more memory efficient and usually ok. However, this has a couple of problems: 1. It makes it MUCH more likely that the GPU will accidentlly stomp CPU-side data structures and cause nearly impossible to debug crashes. 2. With discrete GPUs, the memory will be mapped somehow and that map may be across the BAR so it could have horribly slow CPU access. This is bad for our CPU-side data structures. In the case of anv_state_stream, it also made the data structure massively more complex than it needed to be. Reviewed-by: Lionel Landwerlin Tested-by: Marge Bot Part-of: --- src/intel/vulkan/anv_allocator.c | 56 ++++++++++++-------------------- src/intel/vulkan/anv_private.h | 4 +-- 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c index 535657c45d8..010da4b8c4e 100644 --- a/src/intel/vulkan/anv_allocator.c +++ b/src/intel/vulkan/anv_allocator.c @@ -1190,27 +1190,25 @@ anv_state_stream_init(struct anv_state_stream *stream, stream->block = ANV_STATE_NULL; - stream->block_list = NULL; - /* Ensure that next + whatever > block_size. This way the first call to * state_stream_alloc fetches a new block. */ stream->next = block_size; + util_dynarray_init(&stream->all_blocks, NULL); + VG(VALGRIND_CREATE_MEMPOOL(stream, 0, false)); } void anv_state_stream_finish(struct anv_state_stream *stream) { - struct anv_state_stream_block *next = stream->block_list; - while (next != NULL) { - struct anv_state_stream_block sb = VG_NOACCESS_READ(next); - VG(VALGRIND_MEMPOOL_FREE(stream, sb._vg_ptr)); - VG(VALGRIND_MAKE_MEM_UNDEFINED(next, stream->block_size)); - anv_state_pool_free_no_vg(stream->state_pool, sb.block); - next = sb.next; + util_dynarray_foreach(&stream->all_blocks, struct anv_state, block) { + VG(VALGRIND_MEMPOOL_FREE(stream, block->map)); + VG(VALGRIND_MAKE_MEM_NOACCESS(block->map, block->alloc_size)); + anv_state_pool_free_no_vg(stream->state_pool, *block); } + util_dynarray_fini(&stream->all_blocks); VG(VALGRIND_DESTROY_MEMPOOL(stream)); } @@ -1226,28 +1224,21 @@ anv_state_stream_alloc(struct anv_state_stream *stream, uint32_t offset = align_u32(stream->next, alignment); if (offset + size > stream->block.alloc_size) { - uint32_t min_block_size = size + sizeof(struct anv_state_stream_block); uint32_t block_size = stream->block_size; - if (block_size < min_block_size) - block_size = round_to_power_of_two(min_block_size); + if (block_size < size) + block_size = round_to_power_of_two(size); stream->block = anv_state_pool_alloc_no_vg(stream->state_pool, block_size, PAGE_SIZE); + util_dynarray_append(&stream->all_blocks, + struct anv_state, stream->block); + VG(VALGRIND_MAKE_MEM_NOACCESS(stream->block.map, block_size)); - struct anv_state_stream_block *sb = stream->block.map; - VG_NOACCESS_WRITE(&sb->block, stream->block); - VG_NOACCESS_WRITE(&sb->next, stream->block_list); - stream->block_list = sb; - VG(VG_NOACCESS_WRITE(&sb->_vg_ptr, NULL)); - - VG(VALGRIND_MAKE_MEM_NOACCESS(stream->block.map, stream->block_size)); - - /* Reset back to the start plus space for the header */ - stream->next = sizeof(*sb); - - offset = align_u32(stream->next, alignment); + /* Reset back to the start */ + stream->next = offset = 0; assert(offset + size <= stream->block.alloc_size); } + const bool new_block = stream->next == 0; struct anv_state state = stream->block; state.offset += offset; @@ -1256,22 +1247,17 @@ anv_state_stream_alloc(struct anv_state_stream *stream, stream->next = offset + size; -#ifdef HAVE_VALGRIND - struct anv_state_stream_block *sb = stream->block_list; - void *vg_ptr = VG_NOACCESS_READ(&sb->_vg_ptr); - if (vg_ptr == NULL) { - vg_ptr = state.map; - VG_NOACCESS_WRITE(&sb->_vg_ptr, vg_ptr); - VALGRIND_MEMPOOL_ALLOC(stream, vg_ptr, size); + if (new_block) { + assert(state.map == stream->block.map); + VG(VALGRIND_MEMPOOL_ALLOC(stream, state.map, size)); } else { - void *state_end = state.map + state.alloc_size; /* This only updates the mempool. The newly allocated chunk is still * marked as NOACCESS. */ - VALGRIND_MEMPOOL_CHANGE(stream, vg_ptr, vg_ptr, state_end - vg_ptr); + VG(VALGRIND_MEMPOOL_CHANGE(stream, stream->block.map, stream->block.map, + stream->next)); /* Mark the newly allocated chunk as undefined */ - VALGRIND_MAKE_MEM_UNDEFINED(state.map, state.alloc_size); + VG(VALGRIND_MAKE_MEM_UNDEFINED(state.map, state.alloc_size)); } -#endif return state; } diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 863b527255a..d4e237025d0 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -885,8 +885,6 @@ struct anv_state_pool { struct anv_fixed_size_state_pool buckets[ANV_STATE_BUCKETS]; }; -struct anv_state_stream_block; - struct anv_state_stream { struct anv_state_pool *state_pool; @@ -900,7 +898,7 @@ struct anv_state_stream { uint32_t next; /* List of all blocks allocated from this pool */ - struct anv_state_stream_block *block_list; + struct util_dynarray all_blocks; }; /* The block_pool functions exported for testing only. The block pool should -- 2.30.2