Re: [PATCH] drm/syncobj: return meaningful value to user space

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

 





On 2019年07月19日 16:13, Lionel Landwerlin wrote:
On 18/07/2019 17:33, Chunming Zhou wrote:
在 2019/7/18 22:08, Lionel Landwerlin 写道:
On 18/07/2019 16:02, Chunming Zhou wrote:
在 2019/7/18 19:31, Lionel Landwerlin 写道:
On 18/07/2019 14:13, Chunming Zhou wrote:
if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence
on syncobj,
then return non-block error code to user sapce.

Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx>
---
    drivers/gpu/drm/drm_syncobj.c | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index 361a01a08c18..929f7c64f9a2 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -252,7 +252,7 @@ int drm_syncobj_find_fence(struct drm_file
*file_private,
                return 0;
            dma_fence_put(*fence);
        } else {
-        ret = -EINVAL;
+        ret = -ENOTBLK;
        }
          if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
@@ -832,7 +832,7 @@ static signed long
drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
                if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
                    continue;
                } else {
-                timeout = -EINVAL;
+                timeout = -ENOTBLK;
                    goto cleanup_entries;
                }
            }
This would break existing tests for binary syncobjs.
How does this breaks binary syncobj?

This is used in the submission path of several drivers.

Changing the error code will change what the drivers are reporting to
userspace and could break tests.


i915 doesn't use that function so it's not affected but
lima/panfrost/vc4 seem to be.

anyone from vc4 can confirm this? There are many place in wait_ioctl
being able to return previous EINVAL, not sure what they use to.




Is this really what we want?
I want to use this meaningful return value to judge if WaitBeforeSignal
happens.

I think this is the cheapest change for that.

I thought the plan was to add a new ioctl to query the last submitted
value.
Yes, that is optional way too.  I just thought changing return value is
very cheap, isn't it?


-David


I could be misguided but I thought the kernel policy was to never break userspace.
But no one exactly points how to break userspace, doesn't it?

-David

I'm not sure where this sits :/


-Lionel



Did I misunderstand?


Thanks,


-Lionel


-David


-Lionel




_______________________________________________
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