[PATCH] dma-buf/fence-array: get signaled state when signaling is disabled

[Christian König]

Am 23.09.2016 um 13:30 schrieb Gustavo Padovan:
> 2016-09-22 Christian König <christian.koenig at amd.com>:
>
>> Am 22.09.2016 um 13:16 schrieb Gustavo Padovan:
>>> 2016-09-22 Christian König <christian.koenig at amd.com>:
>>>
>>>> Dropping the rest of the patch, cause that really doesn't make sense any
>>>> more.
>>>>
>>>> Am 22.09.2016 um 12:40 schrieb Gustavo Padovan:
>>>>>> E.g. for example it is illegal to do something like
>>>>>>> "while(!fence_is_signaled(f)) sleep();" without enabling signaling before
>>>>>>> doing this.
>>>>>>>
>>>>>>> Could just be a misunderstanding, but the comments on your patch actually
>>>>>>> sounds a bit like somebody is trying to do exactly that.
>>>>> I think the usecase in mind here is poll(fd, timeout=0)
>>>> Exactly as I feared. Even if userspace polls with timeout=0 you still need
>>>> to call enable_signaling().
>>>>
>>>> Otherwise you can run into a situation where userspace only uses timeout=0
>>>> and so never activates the signaling check in the driver.
>>>>
>>>> This would in turn result in an endless loop on implementations where the
>>>> driver never signals fences on their own.
>>> Polling is optional, userspace may never call it. And DRM/KMS or GPU
>>> drivers will be doing fence_wait() themselves so signaling is enabled at
>>> some point.
>> No they won't. We have an use case where we clearly want to avoid that as
>> much as possible because and so the driver never calls enable_signaling() on
>> it's own.
>>
>> Exposing this poll function to userspace without enabling signaling is a
>> clear NAK from my side.
> Okay. So you are NAK'ing the does_not_pool_wait change. Should we revert
> that one then? It is already broken.

Yeah, that would probably a good idea. The AMD driver changes which 
really rely on this aren't upstream yet, so if you point me to the 
commit hash I could revert that as well when we send that out.

On the other hand the idea behind fence_is_signaled() is really that you 
check the status multiple times after enabling signaling. So I would 
prefer if you would revert this change preliminary.

Double checking this patch (and thinking about it a bit more) reveals 
that it is most likely correct. So feel free to commit this one if it is 
still needed for something.

Regards,
Christian.

>
> Gustavo
>