Mesa (master): anv/allocator: Use util_dynarray for blocks in anv_state_stream

[GitLab Mirror]

Module: Mesa
Branch: master
Commit: 896a7c28eb4b993751bb49659de7511b3fc68756
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=896a7c28eb4b993751bb49659de7511b3fc68756

Author: Jason Ekstrand <jason at jlekstrand.net>
Date:   Thu Mar 26 12:57:36 2020 -0500

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 <lionel.g.landwerlin at intel.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4336>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4336>

---

 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