On Thu, 2014-06-19 at 18:22 +0200, Bruno Jimenez wrote: > On Thu, 2014-06-19 at 11:58 -0400, Jan Vesely wrote: > > 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. > > Hi, > > As far as I understand it, this patch doesn't make obsolete that > comment. But that may be because I understant that that comment only > refers to the first grow of the pool, and the rest is a workaround to > continue growing the pool if we can't fit in the new items.
Hi, thanks for clarification. Since the pool is potentially grown to fit every item, is the initial compute_memory_grow_pool(pool, pipe, allocated+unallocated); needed at all? regards, Jan > > > > > > > > > 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. > > You are right, this patch is still needed in my series. > > > 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. > > I'm OK with adding both calls, and I don't think it is redundant. > Imagine the case where my series don't land, we would have one call > checked and the other not. > > I'll try to rebase my series on top of yours. > > Thanks! > Bruno > > > > > > > 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
