2016-09-23 Christian König <christian.koenig@xxxxxxx>: > Am 23.09.2016 um 13:30 schrieb Gustavo Padovan: > > 2016-09-22 Christian König <christian.koenig@xxxxxxx>: > > > > > Am 22.09.2016 um 13:16 schrieb Gustavo Padovan: > > > > 2016-09-22 Christian König <christian.koenig@xxxxxxx>: > > > > > > > > > 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. It is this patch: ecebca7 dma-buf/sync-file: Avoid enable fence signaling if poll(.timeout=0) But if we revert it as you are a proposing we don't need my patch here anymore. However we would need to revert it now because it is broken. Shall I send a revert part? Gustavo _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
