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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Regards,
Christian.


Gustavo


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux