diff options
author | Jason Ekstrand <[email protected]> | 2020-03-26 12:57:36 -0500 |
---|---|---|
committer | Marge Bot <[email protected]> | 2020-03-31 08:12:07 +0000 |
commit | 896a7c28eb4b993751bb49659de7511b3fc68756 (patch) | |
tree | 5dde5918dafbf11f6010ce57f74229d72ab035ec | |
parent | 63bec07e14d1cd8e01bf45bcda341bb364620cfc (diff) |
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 <[email protected]>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4336>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4336>
-rw-r--r-- | src/intel/vulkan/anv_allocator.c | 56 | ||||
-rw-r--r-- | 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 |