On Thu, 2014-06-19 at 08:27 -0700, Tom Stellard wrote: > On Thu, Jun 19, 2014 at 10:21:32AM -0400, Jan Vesely wrote: > > The important part is the change of the condition to <= 0. Otherwise the > > loop > > gets stuck never actually growing the pool. > > > > The change in the aux-need calculation guarantees max 2 iterations, and > > avoids wasting memory in case a smaller item can't fit into a relatively > > larger > > pool. > > > > Does this patch obsolete the XXX comment around line 292 of this file? If so, > we should remove it.
I'm not sure if the XXX comment applies to the first: if (pool->size_in_dw < allocated+unallocated) or in general. In general I think the comment is obsolete as is. There already is a logic that tries to grow the pool if allocation fails despite enough free space. The patch only changes the the condition to include free space==needed space corner case. The change in requested size is separate and does not change the situation. > > Also have tried this with patches 1-9 of this series: > http://lists.freedesktop.org/archives/mesa-dev/2014-June/061742.html I have not tried applying the patches, but patch 5/11 moves the very same logic ( if (need <0)), to a slightly different location. So I think that patch needs to be updated to include the fix. That series also removes one call to compute_memory_pool_finalize, so adding a check there now might be redundant. However, I'd still prefer to check return value of both calls to compute_memory_pool_finalize for consistency. regards, Jan > > -Tom > > > Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu> > > CC: Bruno Jimenez <brunoji...@gmail.com> > > --- > > > > This fixes hang in gegl colors.xml test > > > > src/gallium/drivers/r600/compute_memory_pool.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/src/gallium/drivers/r600/compute_memory_pool.c > > b/src/gallium/drivers/r600/compute_memory_pool.c > > index ec8c470..0b6d2da6 100644 > > --- a/src/gallium/drivers/r600/compute_memory_pool.c > > +++ b/src/gallium/drivers/r600/compute_memory_pool.c > > @@ -320,8 +320,11 @@ int compute_memory_finalize_pending(struct > > compute_memory_pool* pool, > > int64_t need = item->size_in_dw+2048 - > > (pool->size_in_dw - allocated); > > > > - if (need < 0) { > > - need = pool->size_in_dw / 10; > > + if (need <= 0) { > > + /* There's enough free space, but it's too > > + * fragmented. Assume half of the item can fit > > + * int the last chunk */ > > + need = (item->size_in_dw / 2) + ITEM_ALIGNMENT; > > } > > > > need = align(need, ITEM_ALIGNMENT); > > -- > > 1.9.3 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev -- Jan Vesely <jan.ves...@rutgers.edu>
signature.asc
Description: This is a digitally signed message part
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
