[PATCH 2/2] dma-buf: cleanup shared fence removal

Koenig, Christian Christian.Koenig at amd.com
Fri Jun 28 06:32:15 UTC 2019


Am 27.06.19 um 21:57 schrieb Daniel Vetter:
> [SNIP]
>> Again, the reason to remove the fence from one reservation object is
>> simply that it is faster to remove it from one object than to attach a
>> new fence to all other objects.
> Hm I guess I was lead astray by the eviction_fence invalidation thing
> in enable_signaling, and a few other places that freed the bo right
> afters (like amdgpu_amdkfd_gpuvm_free_memory_of_gpu), where removing
> the fences first and then freeing the bo is kinda pointless.

AH! Now I know where your missing puzzle piece is.

See when we free a buffer object TTM puts it on the delayed delete list 
to make sure that it gets freed up only after all fences are signaled.

So this is essentially to make sure the BO gets freed up immediately.

> Now with your insistence that I'm getting something wrong I guess the
> you're talking about the unbind case, where the bo survives, but it's
> mapping disappears, and hence that specific eviction_fence needs to be
> removed.
> And yeah there I guess just removing the magic eviction fence is
> cheaper than replacing all the others.

If possible I actually want to apply this to the general case of freeing 
up per process resources.

In other words when we don't track resource usage on a per submission 
basis freeing up resources is costly because we always have to wait for 
the last submission.

But if we can prevent further access to the resource using page tables 
it is perfectly valid to free it as soon as the page tables are up to date.

Regards,
Christian.

>
> Now I guess I understand the mechanics of this somewhat, and what
> you're doing, and lit ooks even somewhat safe. But I have no idea what
> this is supposed to achieve. It feels a bit like ->notify_move, but
> implemented in the most horrible way possible. Or maybe something
> else.
>
> Really no idea.
>
> And given that we've wasted I few pages full of paragraphs already on
> trying to explain what your new little helper is for, when it's safe
> to use, when it's maybe not a good idea, and we still haven't even
> bottomed out on what this is for ... well I really don't think it's a
> good idea to inflict this into core code. Because just blindly
> removing normal fences is not safe.
>
> Especially with like half a sentence of kerneldoc that explains
> nothing of all this complexity.
> -Daniel



More information about the dri-devel mailing list