On ti, 2017-01-10 at 21:55 +0000, Chris Wilson wrote:
> Performing an eviction search can be very, very slow especially for a
> range restricted replacement. For example, a workload like
> gem_concurrent_blit will populate the entire GTT and then cause aperture
> thrashing. Since the GTT is a mix of active and inactive tiny objects,
> we have to search through almost 400k objects before finding anything
> inside the mappable region, and as this search is required before every
> operation performance falls off a cliff.
> 
> Instead of performing the full search, we do a trial replacement of the
> node at a random location fitting the specified restrictions. We lose
> the strict LRU property of the GTT in exchange for avoiding the slow
> search (several orders of runtime improvement for gem_concurrent_blit
> 4KiB-global-gtt, e.g. from 5000s to 20s). The loss of LRU replacement is
> (later) mitigated firstly by only doing replacement if we find no
> freespace and secondly by execbuf doing a PIN_NONBLOCK search first before
> it starts thrashing (i.e. the random replacement will only occur from the
> already inactive set of objects).
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>

<SNIP>

> +static u64 random_offset(u64 start, u64 end, u64 len, u64 align)
> +{

The usual GEM_BUG_ON dance to make sure the inputs make some sense. Or
are you relying on the upper level callers?

> +     u64 range, addr;
> +
> +     if (align == 0)
> +             align = I915_GTT_MIN_ALIGNMENT;
> +
> +     range = round_down(end - len, align) - round_up(start, align);

For example this may cause an odd result.

> @@ -3629,6 +3655,16 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
>       if (err != -ENOSPC)
>               return err;
>  
> +     /* No free space, pick a slot at random */
> +     err = i915_gem_gtt_reserve(vm, node,
> +                                size,
> +                                random_offset(start, end, size, alignment),

I'd pull this to a line above just to make it more humane to read.

> +                                color,
> +                                flags);
> +     if (err != -ENOSPC)
> +             return err;
> +
> +     /* Randomly selected placement is pinned, do a search */
>       err = i915_gem_evict_something(vm, size, alignment, color,
>                                      start, end, flags);
>       if (err)

I'm bit unsure why it would make such a big difference, but if you've
been running the numbers. Code itself is all good, so this is;

Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to