Quoting Daniel Vetter (2018-02-19 15:52:34)
> On Mon, Feb 19, 2018 at 02:43:59PM +0000, Chris Wilson wrote:
> > Quoting Joonas Lahtinen (2018-02-19 14:40:32)
> > > Quoting Chris Wilson (2018-02-19 13:35:43)
> > > > +++ b/drivers/gpu/drm/drm_mm.c
> > > > @@ -836,9 +836,24 @@ struct drm_mm_node *drm_mm_scan_color_evict(struct drm_mm_scan *scan)
> > > > if (!mm->color_adjust)
> > > > return NULL;
> > > >
> > > > - hole = list_first_entry(&mm->hole_stack, typeof(*hole), hole_stack);
> > > > - hole_start = __drm_mm_hole_node_start(hole);
> > > > - hole_end = hole_start + hole->hole_size;
> > > > + /*
> > > > + * The hole found during scanning should ideally be the first element
> > > > + * in the hole_stack list, but due to side-effects in the driver it
> > > > + * may not be.
> > > > + */
> > > > + list_for_each_entry(hole, &mm->hole_stack, hole_stack) {
> > > > + hole_start = __drm_mm_hole_node_start(hole);
> > > > + hole_end = hole_start + hole->hole_size;
> > > > +
> > > > + if (hole_start <= scan->hit_start &&
> > > > + hole_end >= scan->hit_end)
> > >
> > > How about some likely() here?
> >
> > I felt that at the point where we admit we need a search, likely() went
> > out of the window. Given that I think we'll need 2 or 3 iterations at
> > most, that probably means in practice this is around the 50% mark.
> > Claiming that it's likely() may be misleading to the reader.
>
> Concurred.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Not too surprising that no one else had an opinion since only i915 uses
the node coloring at present. Pushed to drm-misc-fixes.
> I spent some brain cycles trying to come up with clever tricks to avoid
> the need to this function outright, but failed.
I think it's not too bad overall. When scanning we include the necessary
bookends in the hole calculation, so we know that if we do need to evict
a bookend then it is available and was included in our LRU ordering. It
is just that during the scan, the intermediate state is inconsistent
with the final arrangement and so instead of always evicting those
bookends we make a final call on whether the new node can fit into the
smaller hole or needs the enlargement. It should only be called a maximum
of two times to check on the two bookends, and have no impact for when
coloring is not in effect.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
