Re: [Mesa-dev] [PATCH] anv/allocator: Don't srink either end of the block pool
On Mon, Apr 23, 2018 at 8:33 AM, Scott D Phillips < scott.d.phill...@intel.com> wrote: > Jason Ekstrandwrites: > > > Previously, we only tried to ensure that we didn't shrink either end > > below what was already handed out. However, due to the way we handle > > relocations with block pools, we can't shrink the back end at all. It's > > probably best to not shrink in either direction. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105374 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106147 > > Cc: mesa-sta...@lists.freedesktop.org > > --- > > src/intel/vulkan/anv_allocator.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_ > allocator.c > > index f884ac3..642e161 100644 > > --- a/src/intel/vulkan/anv_allocator.c > > +++ b/src/intel/vulkan/anv_allocator.c > > @@ -508,12 +508,12 @@ anv_block_pool_grow(struct anv_block_pool *pool, > struct anv_block_state *state) > > >assert(center_bo_offset >= back_used); > > > >/* Make sure we don't shrink the back end of the pool */ > > - if (center_bo_offset < pool->back_state.end) > > - center_bo_offset = pool->back_state.end; > > + if (center_bo_offset < back_required) > > + center_bo_offset = back_required; > >/* Make sure that we don't shrink the front end of the pool */ > > - if (size - center_bo_offset < pool->state.end) > > - center_bo_offset = size - pool->state.end; > > + if (size - center_bo_offset < front_required) > > + center_bo_offset = size - front_required; > > Reading through the function, it's not clear to me what condition will > lead to a possible shrinking of one side or the other here. Regardless, > any calculation here based on .end, the old size, seems like it would > have to be wrong because we're trying to satisfy the post condition that > there is enough room for .next on each side. So, > The problem is that he balancing algorithm above this isn't perfect and can accidentally try to shrink one side or the other. The logic fixed here was intended to prevent that but didn't prevent it hard enough. > Reviewed-by: Scott D Phillips > Thanks! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/allocator: Don't srink either end of the block pool
Jason Ekstrandwrites: > Previously, we only tried to ensure that we didn't shrink either end > below what was already handed out. However, due to the way we handle > relocations with block pools, we can't shrink the back end at all. It's > probably best to not shrink in either direction. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105374 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106147 > Cc: mesa-sta...@lists.freedesktop.org > --- > src/intel/vulkan/anv_allocator.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/intel/vulkan/anv_allocator.c > b/src/intel/vulkan/anv_allocator.c > index f884ac3..642e161 100644 > --- a/src/intel/vulkan/anv_allocator.c > +++ b/src/intel/vulkan/anv_allocator.c > @@ -508,12 +508,12 @@ anv_block_pool_grow(struct anv_block_pool *pool, struct > anv_block_state *state) >assert(center_bo_offset >= back_used); > >/* Make sure we don't shrink the back end of the pool */ > - if (center_bo_offset < pool->back_state.end) > - center_bo_offset = pool->back_state.end; > + if (center_bo_offset < back_required) > + center_bo_offset = back_required; >/* Make sure that we don't shrink the front end of the pool */ > - if (size - center_bo_offset < pool->state.end) > - center_bo_offset = size - pool->state.end; > + if (size - center_bo_offset < front_required) > + center_bo_offset = size - front_required; Reading through the function, it's not clear to me what condition will lead to a possible shrinking of one side or the other here. Regardless, any calculation here based on .end, the old size, seems like it would have to be wrong because we're trying to satisfy the post condition that there is enough room for .next on each side. So, Reviewed-by: Scott D Phillips > } > > assert(center_bo_offset % PAGE_SIZE == 0); > -- > 2.5.0.400.gff86faf > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/allocator: Don't srink either end of the block pool
Hi, On 21.04.2018 08:15, Jason Ekstrand wrote: Previously, we only tried to ensure that we didn't shrink either end below what was already handed out. However, due to the way we handle relocations with block pools, we can't shrink the back end at all. It's probably best to not shrink in either direction. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105374 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106147 Verified that "texture3d" demo works with this patch, crashes without it. Tested-by: Eero Tamminen- Eero Cc: mesa-sta...@lists.freedesktop.org --- src/intel/vulkan/anv_allocator.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c index f884ac3..642e161 100644 --- a/src/intel/vulkan/anv_allocator.c +++ b/src/intel/vulkan/anv_allocator.c @@ -508,12 +508,12 @@ anv_block_pool_grow(struct anv_block_pool *pool, struct anv_block_state *state) assert(center_bo_offset >= back_used); /* Make sure we don't shrink the back end of the pool */ - if (center_bo_offset < pool->back_state.end) - center_bo_offset = pool->back_state.end; + if (center_bo_offset < back_required) + center_bo_offset = back_required; /* Make sure that we don't shrink the front end of the pool */ - if (size - center_bo_offset < pool->state.end) - center_bo_offset = size - pool->state.end; + if (size - center_bo_offset < front_required) + center_bo_offset = size - front_required; } assert(center_bo_offset % PAGE_SIZE == 0); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv/allocator: Don't srink either end of the block pool
Previously, we only tried to ensure that we didn't shrink either end below what was already handed out. However, due to the way we handle relocations with block pools, we can't shrink the back end at all. It's probably best to not shrink in either direction. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105374 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106147 Cc: mesa-sta...@lists.freedesktop.org --- src/intel/vulkan/anv_allocator.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c index f884ac3..642e161 100644 --- a/src/intel/vulkan/anv_allocator.c +++ b/src/intel/vulkan/anv_allocator.c @@ -508,12 +508,12 @@ anv_block_pool_grow(struct anv_block_pool *pool, struct anv_block_state *state) assert(center_bo_offset >= back_used); /* Make sure we don't shrink the back end of the pool */ - if (center_bo_offset < pool->back_state.end) - center_bo_offset = pool->back_state.end; + if (center_bo_offset < back_required) + center_bo_offset = back_required; /* Make sure that we don't shrink the front end of the pool */ - if (size - center_bo_offset < pool->state.end) - center_bo_offset = size - pool->state.end; + if (size - center_bo_offset < front_required) + center_bo_offset = size - front_required; } assert(center_bo_offset % PAGE_SIZE == 0); -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev