Shared semaphores for amdgpu

[Christian König]

Am 09.03.2017 um 10:43 schrieb zhoucm1:
>
>
> On 2017年03月09日 17:12, Christian König wrote:
>> Am 09.03.2017 um 09:15 schrieb Dave Airlie:
>>> On 9 March 2017 at 17:38, Christian König <christian.koenig at amd.com> 
>>> wrote:
>>>>> I do wonder if we need the separate sem signal/wait interface, I 
>>>>> think
>>>>> we should just add
>>>>> semaphore chunks to the CS interface.
>>>> Yeah, that's what I've said as well from the very first beginning.
>>>>
>>>> Another question is if we should really create another 
>>>> implementation to
>>>> share semaphores between processes.
>>>>
>>>> In other words putting the current fences inside the semaphore into a
>>>> sync_file with the signal_on_any bit set would have pretty much the 
>>>> same
>>>> effect, except that the resulting object then had the sync_file 
>>>> semantics
>>>> for adding new fences and can be used in the atomic IOCTLs as well.
>>> So the vulkan external semaphore spec has two different type of 
>>> semaphore
>>> semantics, I'm not sure the sync_file semantics match the first type,
>>> only the second.
>>
>> I haven't completely read that part of the spec yet, but from what I 
>> know the first semantics is actually a bit scary and I'm not sure if 
>> we want to fully support that.
>>
>> Especially that you can wait on a semaphore object which is not 
>> signaled yet can easily lead to deadlocks and bound resources in the 
>> kernel and windowing system.
>>
>> Imagine that you send a command submission to the kernel with the 
>> request to wait for a semaphore object and then never signal that 
>> semaphore object. At least for amdgpu the kernel driver would accept 
>> that CS and push it into the scheduler. This operation needs memory, 
>> so by doing this the application would bind kernel memory without the 
>> prospect of releasing it anytime soon.
>>
>> We could of course try to limit the amounts of waiting CS in the 
>> kernel, but then we have the problem of deadlocks again. E.g. the 
>> signaling CS wouldn't be accepted by the kernel because we have so 
>> many waiters.
>>
>> Additional to that you can easily build deadlocks in the form CS A 
>> depends on CS B and CS B depends on CS A. The exact same problem for 
>> Android fences where discussed on the list as well, but the semantic 
>> there is especially designed so that you can't build deadlocks with it.
> Forbidding to wait un-sginaled sem will be enough for your this concern.

Completely agree, problem here is that this isn't documented like this 
in the Vulkan specification as far as I know.

>
>>
>>> I think we would still need separate objects to do the first type,
> Agreed, the implementation indeed do this.
>>> which I want for VR stuff..
>>
>> Which is perfectly reasonable, sharing the object between processes 
>> takes time. So you only want to do this once.
>>
>> As a possible solution what do you think about adding some new 
>> functionality to the sync file IOCTLs?
>>
>> IIRC we currently only support adding new fences to the sync file and 
>> then waiting for all of the in the CS/Atomic page flip.
>>
>> But what if we also allow replacing the fence(s) in the sync file? 
>> And then additional to that consuming the fence in the CS/Atomic page 
>> flip IOCTL?
> I feel the new sem implementation of what I attached is very good, at 
> least a good start, maybe we could discuss from there, not talk in the 
> fly, so that any problem we can improve it.

We first need to find a consensus if we can't implement that by 
improving the existing synchronization primitives instead of inventing 
new ones.

I'm clearly not favoring any approach just because it is already 
implemented.

See sync files are already upstream and well supported in for example 
the atomic page flipping IOCTLs and adding the extra functionality 
needed to support the semantics of Vulkan semaphores looks rather 
trivial to me.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> That's trivial to implement and should give us pretty much the same 
>> semantics as the shared semaphore object in Vulkan.
>>
>> Christian.
>>
>>>
>>> I'll try and think about it a bit harder tomorrow.
>>>
>>> Dave.
>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx