lkml.org 
[lkml]   [2021]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 154/167] Revert "drm/i915: Propagate errors on awaiting already signaled fences"
    Date
    From: Jason Ekstrand <jason@jlekstrand.net>

    commit 3761baae908a7b5012be08d70fa553cc2eb82305 upstream.

    This reverts commit 9e31c1fe45d555a948ff66f1f0e3fe1f83ca63f7. Ever
    since that commit, we've been having issues where a hang in one client
    can propagate to another. In particular, a hang in an app can propagate
    to the X server which causes the whole desktop to lock up.

    Error propagation along fences sound like a good idea, but as your bug
    shows, surprising consequences, since propagating errors across security
    boundaries is not a good thing.

    What we do have is track the hangs on the ctx, and report information to
    userspace using RESET_STATS. That's how arb_robustness works. Also, if my
    understanding is still correct, the EIO from execbuf is when your context
    is banned (because not recoverable or too many hangs). And in all these
    cases it's up to userspace to figure out what is all impacted and should
    be reported to the application, that's not on the kernel to guess and
    automatically propagate.

    What's more, we're also building more features on top of ctx error
    reporting with RESET_STATS ioctl: Encrypted buffers use the same, and the
    userspace fence wait also relies on that mechanism. So it is the path
    going forward for reporting gpu hangs and resets to userspace.

    So all together that's why I think we should just bury this idea again as
    not quite the direction we want to go to, hence why I think the revert is
    the right option here.

    For backporters: Please note that you _must_ have a backport of
    https://lore.kernel.org/dri-devel/20210602164149.391653-2-jason@jlekstrand.net/
    for otherwise backporting just this patch opens up a security bug.

    v2: Augment commit message. Also restore Jason's sob that I
    accidentally lost.

    v3: Add a note for backporters

    Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
    Reported-by: Marcin Slusarz <marcin.slusarz@intel.com>
    Cc: <stable@vger.kernel.org> # v5.6+
    Cc: Jason Ekstrand <jason.ekstrand@intel.com>
    Cc: Marcin Slusarz <marcin.slusarz@intel.com>
    Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3080
    Fixes: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already signaled fences")
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Link: https://patchwork.freedesktop.org/patch/msgid/20210714193419.1459723-3-jason@jlekstrand.net
    (cherry picked from commit 93a2711cddd5760e2f0f901817d71c93183c3b87)
    Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/gpu/drm/i915/i915_request.c | 8 ++------
    1 file changed, 2 insertions(+), 6 deletions(-)

    --- a/drivers/gpu/drm/i915/i915_request.c
    +++ b/drivers/gpu/drm/i915/i915_request.c
    @@ -1285,10 +1285,8 @@ i915_request_await_execution(struct i915

    do {
    fence = *child++;
    - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
    - i915_sw_fence_set_error_once(&rq->submit, fence->error);
    + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
    continue;
    - }

    if (fence->context == rq->fence.context)
    continue;
    @@ -1386,10 +1384,8 @@ i915_request_await_dma_fence(struct i915

    do {
    fence = *child++;
    - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
    - i915_sw_fence_set_error_once(&rq->submit, fence->error);
    + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
    continue;
    - }

    /*
    * Requests on the same timeline are explicitly ordered, along

    \
     
     \ /
      Last update: 2021-07-26 18:44    [W:4.645 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site