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@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>

diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c
index 535657c45d8f8a8dbcd7cf1a55c5407752f59068..010da4b8c4e70da9ddb9b17609bef3e5ac93fa86 100644 (file)
--- 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 863b527255ad3ebce654748bf9b355a21ef547b3..d4e237025d0b09050dcaa9902e5462bdd03c1208 100644 (file)
--- 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