[Intel-gfx] [PATCH] drm/i915: Clean up associated VMAs on context destruction

[Tvrtko Ursulin]

On 09/11/2015 03:56 PM, Chris Wilson wrote:
> On Fri, Sep 11, 2015 at 03:31:33PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Prevent leaking VMAs and PPGTT VMs when objects are imported
>> via flink.
>>
>> Scenario is that any VMAs created by the importer will be left
>> dangling after the importer exits, or destroys the PPGTT context
>> with which they are associated.
>>
>> This is caused by object destruction not running when the
>> importer closes the buffer object handle due the reference held
>> by the exporter. This also leaks the VM since the VMA has a
>> reference on it.
>>
>> In practice these leaks can be observed by stopping and starting
>> the X server on a kernel with fbcon compiled in. Every time
>> X server exits another VMA will be leaked against the fbcon's
>> frame buffer object.
>>
>> Also on systems where flink buffer sharing is used extensively,
>> like Android, this leak has even more serious consequences.
>>
>> This version is takes a general approach from the  earlier work
>> by Rafael Barabalho (drm/i915: Clean-up PPGTT on context
>> destruction) and tries to incorporate the subsequent discussion
>> between Chris Wilson and Daniel Vetter.
>>
>> On context destruction a VM is marked as closed and a worker
>> thread scheduled to unbind all inactive VMAs for this VM. At
>> the same time, active VMAs retired on this closed VM are
>> unbound immediately.
>
> You don't need a worker, since you just can just drop the vma from the
> retirement.

I was thinking that retirement does not necessarily happen - maybe both 
VMAs are already inactive at the time of context destruction. Which is 
then a question is it OK to wait for the next retirement on the same 
object to clean it up. I wasn't sure so thought it is safer to clean it 
up immediately since it is not a lot of code.

> http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=nightly&id=9d4020dce054cca23bd1fea72092d036f0a3ea13
>
> That patch is as old as the test case, just waiting for some review on
> earlier code.

Plus my patch doesn't fix flink-and-close-vma-leak, but a new one I also 
posted, flink-and-exit-vma-leak. The latter is what was affecting 
Android, and can be seen with X.org and fbcon.

Your cleanup and handle close is more complete but a question is how 
long will "just waiting for some review on earlier code" take :), 
especially considering it depends on a bigger rewrite of the core.

Regards,

Tvrtko