| Message ID | 20241022161825.228278-1-olvaffe@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] drm/syncobj: ensure progress for syncobj queries | expand |
Am 22.10.24 um 18:18 schrieb Chia-I Wu: > Userspace might poll a syncobj with the query ioctl. Call > dma_fence_enable_sw_signaling to ensure dma_fence_is_signaled returns > true in finite time. Wait a second, just querying the fence status is absolutely not guaranteed to return true in finite time. That is well documented on the dma_fence() object. When you want to poll on signaling from userspace you really need to call poll or the wait IOCTL with a zero timeout. That will also return immediately but should enable signaling while doing that. So just querying the status should absolutely *not* enable signaling. That's an intentional separation. Regards, Christian. > > Fixes: 27b575a9aa2f ("drm/syncobj: add timeline payload query ioctl v6") > Signed-off-by: Chia-I Wu <olvaffe@gmail.com> > > --- > > v2: add Signed-off-by and Fixes tags > --- > drivers/gpu/drm/drm_syncobj.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index 4fcfc0b9b386c..58c5593c897a2 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -1689,6 +1689,9 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, > DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) { > point = fence->seqno; > } else { > + /* ensure forward progress */ > + dma_fence_enable_sw_signaling(fence); > + > dma_fence_chain_for_each(iter, fence) { > if (iter->context != fence->context) { > dma_fence_put(iter);
On Tue, Oct 22, 2024 at 9:53 AM Christian König <christian.koenig@amd.com> wrote: > > Am 22.10.24 um 18:18 schrieb Chia-I Wu: > > Userspace might poll a syncobj with the query ioctl. Call > > dma_fence_enable_sw_signaling to ensure dma_fence_is_signaled returns > > true in finite time. > > Wait a second, just querying the fence status is absolutely not > guaranteed to return true in finite time. That is well documented on the > dma_fence() object. > > When you want to poll on signaling from userspace you really need to > call poll or the wait IOCTL with a zero timeout. That will also return > immediately but should enable signaling while doing that. > > So just querying the status should absolutely *not* enable signaling. > That's an intentional separation. I think it depends on what semantics DRM_IOCTL_SYNCOBJ_QUERY should have. If DRM_IOCTL_SYNCOBJ_QUERY is mainly for vulkan timeline semaphores, it is a bit heavy if userspace has to do a DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT before a query. > > Regards, > Christian. > > > > > Fixes: 27b575a9aa2f ("drm/syncobj: add timeline payload query ioctl v6") > > Signed-off-by: Chia-I Wu <olvaffe@gmail.com> > > > > --- > > > > v2: add Signed-off-by and Fixes tags > > --- > > drivers/gpu/drm/drm_syncobj.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > > index 4fcfc0b9b386c..58c5593c897a2 100644 > > --- a/drivers/gpu/drm/drm_syncobj.c > > +++ b/drivers/gpu/drm/drm_syncobj.c > > @@ -1689,6 +1689,9 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, > > DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) { > > point = fence->seqno; > > } else { > > + /* ensure forward progress */ > > + dma_fence_enable_sw_signaling(fence); > > + > > dma_fence_chain_for_each(iter, fence) { > > if (iter->context != fence->context) { > > dma_fence_put(iter); >
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 4fcfc0b9b386c..58c5593c897a2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1689,6 +1689,9 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) { point = fence->seqno; } else { + /* ensure forward progress */ + dma_fence_enable_sw_signaling(fence); + dma_fence_chain_for_each(iter, fence) { if (iter->context != fence->context) { dma_fence_put(iter);
Userspace might poll a syncobj with the query ioctl. Call dma_fence_enable_sw_signaling to ensure dma_fence_is_signaled returns true in finite time. Fixes: 27b575a9aa2f ("drm/syncobj: add timeline payload query ioctl v6") Signed-off-by: Chia-I Wu <olvaffe@gmail.com> --- v2: add Signed-off-by and Fixes tags --- drivers/gpu/drm/drm_syncobj.c | 3 +++ 1 file changed, 3 insertions(+)