Skip to content

Commit 3761baa

Browse files
gfxstrandrodrigovivi
authored andcommitted
Revert "drm/i915: Propagate errors on awaiting already signaled fences"
This reverts commit 9e31c1f. 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: 9e31c1f ("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 93a2711) Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
1 parent c9d9fdb commit 3761baa

File tree

1 file changed

+2
-6
lines changed

1 file changed

+2
-6
lines changed

drivers/gpu/drm/i915/i915_request.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,10 +1426,8 @@ i915_request_await_execution(struct i915_request *rq,
14261426

14271427
do {
14281428
fence = *child++;
1429-
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
1430-
i915_sw_fence_set_error_once(&rq->submit, fence->error);
1429+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
14311430
continue;
1432-
}
14331431

14341432
if (fence->context == rq->fence.context)
14351433
continue;
@@ -1527,10 +1525,8 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence)
15271525

15281526
do {
15291527
fence = *child++;
1530-
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
1531-
i915_sw_fence_set_error_once(&rq->submit, fence->error);
1528+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
15321529
continue;
1533-
}
15341530

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

0 commit comments

Comments
 (0)