[Intel-gfx] [PATCH 10/42] drm/i915: Defer active reference until required

[Tvrtko Ursulin]

On 07/10/2016 17:58, Chris Wilson wrote:
> On Fri, Oct 07, 2016 at 05:35:38PM +0100, Tvrtko Ursulin wrote:
>> On 07/10/2016 10:46, Chris Wilson wrote:
>>> We only need the active reference to keep the object alive after the
>>> handle has been deleted (so as to prevent a synchronous gem_close). Why
>>> then pay the price of a kref on every execbuf when we can insert that
>>> final active ref just in time for the handle deletion?
>> I really dislike this.  Where there was elegance with obj/vma_put,
>> it is now replaced with out of place looking
>> __i915_gem_object_release_unless_active. I don't see why would
>> higher level layers have to concern themselves with calling
>> something with such a low-level sounding name.
>>
>> How much does this influence performance and in what cases? If
>> significant, could we try to come up with something similar but more
>> elegant?
> Back in the day, this was one of the most frequent atomic operations we
> did. And whilst perf overemphasizes the stalls from locked instructions,
> the sheer numbers of them we do are significant (since we do one at the
> start and end of every execbuf for every object in typical conditions).
> Whilst it is less significant in the face of obj->resv undoing all of the
> gains, it is still a deep paper cut. (At the GL level, consider about 100
> objects per batch, several thousand times a second x 2, these ops are low
> hanging fruit.)

I understand there is a lot of them (same is true for many other 
operations we do), but how does that translate to some benchmarks?

> What's needed is a function to take the place of the close_object for
> internally allocated objects. It is also worth noting that they are either
> already part of a cache, or are suitable for caching....

Ok but those are not x 100 per batch.

Regards,

Tvrtko