[Mesa-dev] [PATCH] winsys/radeon: Unmap GPU VM address range when destroying BO

[Christian König]

On 17.06.2015 04:54, Michel Dänzer wrote:
> On 16.06.2015 20:39, Christian König wrote:
>> On 16.06.2015 10:56, Michel Dänzer wrote:
>>> On 16.06.2015 17:34, Christian König wrote:
>>>> What we would need to really clean that up is to make the VM mappings
>>>> per GEM handle like you suggested or allow multiple mappings per BO like
>>>> we did it for Amdgpu.
>>> What kind of time-frame / effort do you think it would take to get that?
>> Doing it like Amdgpu is actually trivial to implement, we would just
>> need to port the implementation back from Amdgpu. But the problem is
>> that it would break the userspace ABI and some older mesa version
>> wouldn't work any more because of this cause they couldn't detect any
>> more that it's the same BO.
>>
>> Doing the mapping per GEM handle might be an idea worth looking into,
>> but I'm not sure how to approach that.
> Allowing multiple mappings per BO but not tracking them per GEM handle
> wouldn't solve the problem without this Mesa patch anyway, because the
> kernel would remember all mappings until the BO is destroyed, right? I
> think that might also indeed break older Mesa.
>
> However, I think it should be safe to allow multiple mappings if they
> are tracked per GEM handle. That should avoid this kind of problem even
> with old Mesa without this patch. Not sure that's worth the effort though.
>
>
>>>> Another alternative is to teach radeon_bomgr_free_va that a certain
>>>> address range could be allocated to more than one handle.
>>> What exactly do you have in mind for that? I don't think the winsys has
>>> several representations of the same BO in the cases fixed by this patch,
>>> and I'm not sure how it could account for external references.
>> Ah, right the BO could still be open by somebody else outside of mesa.
>>
>> Since the code now looks up the BOs by VA we indeed shouldn't have any
>> problems any more and your proposed solution should work fine.
> Does that mean I get your Reviewed-by: or at least Acked-by: tag? :)

Yes, patches are Reviewed-by: Christian König <christian.koenig at amd.com>

A better solution for detecting if it's save to unmap the BO would be 
nice, but I don't see one of hand either.

Regards,
Christian.