[PATCH] drm/i915: skip __i915_request_skip() for already signaled requests

[PATCH] drm/i915: skip __i915_request_skip() for already signaled requests

[Sebastian Brzezinka]

After a GPU reset the HWSP is zeroed, so previously completed
requests appear incomplete. If such a request is picked up during
reset_rewind() and marked guilty, i915_request_set_error_once()
returns early (fence already signaled), leaving fence.error without
a fatal error code. The subsequent __i915_request_skip() then hits:
```
GEM_BUG_ON(!fatal_error(rq->fence.error))
```

Fixes a kernel BUG observed on Sandy Bridge (Gen6) during
heartbeat-triggered engine resets.
```
kernel BUG at drivers/gpu/drm/i915/i915_request.c:556!
RIP: __i915_request_skip+0x15e/0x1d0 [i915]
...
__i915_request_reset+0x212/0xa70 [i915]
reset_rewind+0xe4/0x280 [i915]
intel_gt_reset+0x30d/0x5b0 [i915]
heartbeat+0x516/0x530 [i915]
```

Guard __i915_request_skip() with i915_request_signaled(), if the
fence is already signaled, the ring content is committed and there
is nothing left to skip.

Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/13729
Fixes: 36e191f0644b ("drm/i915: Apply i915_request_skip() on submission")
Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_reset.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 37272871b0f2..b728a5171e93 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -133,7 +133,8 @@ void __i915_request_reset(struct i915_request *rq, bool guilty)
 	rcu_read_lock(); /* protect the GEM context */
 	if (guilty) {
 		i915_request_set_error_once(rq, -EIO);
-		__i915_request_skip(rq);
+		if (!i915_request_signaled(rq))
+			__i915_request_skip(rq);
 		banned = mark_guilty(rq);
 	} else {
 		i915_request_set_error_once(rq, -EAGAIN);
-- 
2.53.0

Re: [PATCH] drm/i915: skip __i915_request_skip() for already signaled requests

[Krzysztof Karas]

Hi Sebastian,

On 2026-04-16 at 13:31:18 +0200, Sebastian Brzezinka wrote:
> After a GPU reset the HWSP is zeroed, so previously completed
> requests appear incomplete. If such a request is picked up during
> reset_rewind() and marked guilty, i915_request_set_error_once()
> returns early (fence already signaled), leaving fence.error without
> a fatal error code. The subsequent __i915_request_skip() then hits:
> ```
> GEM_BUG_ON(!fatal_error(rq->fence.error))
> ```
> 
> Fixes a kernel BUG observed on Sandy Bridge (Gen6) during
By "Fixes" do you mean this patch? Or are you referring to the
tag "Fixes:" below? If former would be the case, then imperative
form might be better: Fix.

In any case the patch looks sane:
Reviewed-by: Krzysztof Karas <krzysztof.karas@intel.com>

> heartbeat-triggered engine resets.
> ```
> kernel BUG at drivers/gpu/drm/i915/i915_request.c:556!
> RIP: __i915_request_skip+0x15e/0x1d0 [i915]
> ...
> __i915_request_reset+0x212/0xa70 [i915]
> reset_rewind+0xe4/0x280 [i915]
> intel_gt_reset+0x30d/0x5b0 [i915]
> heartbeat+0x516/0x530 [i915]
> ```
> 
> Guard __i915_request_skip() with i915_request_signaled(), if the
> fence is already signaled, the ring content is committed and there
> is nothing left to skip.
> 
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/13729
> Fixes: 36e191f0644b ("drm/i915: Apply i915_request_skip() on submission")
> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_reset.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 37272871b0f2..b728a5171e93 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -133,7 +133,8 @@ void __i915_request_reset(struct i915_request *rq, bool guilty)
>  	rcu_read_lock(); /* protect the GEM context */
>  	if (guilty) {
>  		i915_request_set_error_once(rq, -EIO);
> -		__i915_request_skip(rq);
> +		if (!i915_request_signaled(rq))
> +			__i915_request_skip(rq);
>  		banned = mark_guilty(rq);
>  	} else {
>  		i915_request_set_error_once(rq, -EAGAIN);
> -- 
> 2.53.0
> 

-- 
Best Regards,
Krzysztof

Re: [PATCH] drm/i915: skip __i915_request_skip() for already signaled requests

[Andi Shyti]

Hi Sebastian,

On Mon, Apr 20, 2026 at 09:18:03AM +0000, Krzysztof Karas wrote:
> On 2026-04-16 at 13:31:18 +0200, Sebastian Brzezinka wrote:
> > After a GPU reset the HWSP is zeroed, so previously completed
> > requests appear incomplete. If such a request is picked up during
> > reset_rewind() and marked guilty, i915_request_set_error_once()
> > returns early (fence already signaled), leaving fence.error without
> > a fatal error code. The subsequent __i915_request_skip() then hits:
> > ```
> > GEM_BUG_ON(!fatal_error(rq->fence.error))
> > ```
> > 
> > Fixes a kernel BUG observed on Sandy Bridge (Gen6) during
> By "Fixes" do you mean this patch? Or are you referring to the
> tag "Fixes:" below? If former would be the case, then imperative
> form might be better: Fix.

Pour parler: the imperative is used in the last paragraph:
"Guard" :-)

> 
> In any case the patch looks sane:
> Reviewed-by: Krzysztof Karas <krzysztof.karas@intel.com>
> 
> > heartbeat-triggered engine resets.
> > ```
> > kernel BUG at drivers/gpu/drm/i915/i915_request.c:556!
> > RIP: __i915_request_skip+0x15e/0x1d0 [i915]
> > ...
> > __i915_request_reset+0x212/0xa70 [i915]
> > reset_rewind+0xe4/0x280 [i915]
> > intel_gt_reset+0x30d/0x5b0 [i915]
> > heartbeat+0x516/0x530 [i915]
> > ```
> > 
> > Guard __i915_request_skip() with i915_request_signaled(), if the
> > fence is already signaled, the ring content is committed and there
> > is nothing left to skip.
> > 
> > Cc: stable@vger.kernel.org
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/13729
> > Fixes: 36e191f0644b ("drm/i915: Apply i915_request_skip() on submission")
> > Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi