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

[Christian König]

On 16.06.2015 10:56, Michel Dänzer wrote:
> On 16.06.2015 17:34, Christian König wrote:
>> On 16.06.2015 10:28, Michel Dänzer wrote:
>>> From: Michel Dänzer <michel.daenzer at amd.com>
>>>
>>> But only when doing so is safe according to the
>>> RADEON_INFO_VA_UNMAP_WORKING kernel query.
>>>
>>> This avoids GPU VM address range conflicts when the BO has other
>>> references than the GEM handle being closed, e.g. when the BO is shared.
>> That actually doesn't really helps, it just masks the problem for now.
>> The other GEM handle could still require the address space just unmapped
>> and freed.
> In theory, yes. In practice, I think only the Mesa driver actually uses
> GPUVM addresses, not the other parts involved in these issues (the Xorg
> radeon driver or the Android gralloc). The Mesa driver is careful not to
> have several representations of the same BO.

Checking the latest code I think you're right. When the kernel returns 
RADEON_VA_RESULT_VA_EXIST we lockup the BO using it's VA address. 
Previously we just forced the same VA on a new BO object.

> Or do you expect this might cause trouble with GL<->VDPAU or GL<->CL
> interop?

Not really, they should use the same winsys and so the same BO 
representation.

>
>
>> 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.

>
>
>> 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.

Regards,
Christian.