Re: [PATCH v2 1/2] drm/i915/guc: Complete all requests of a non-schedulable context

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Wed, 2026-11-11 at 13:57 +0000, Javier MV wrote:
> From: Javier MV <javiermv@duck.com>
> 
> When the context of a request being submitted is banned or exiting,
> __guc_add_request() marks the request as complete with -EIO instead
> of
> submitting it. But guc_dequeue_one_context() merges all queued
> requests
> of a context and only passes the last one to __guc_add_request(). The
> earlier ones were moved to ce->guc_state.requests by
> __i915_request_submit(), which only skips their payload: they still
> need the GPU to run the context and write their breadcrumbs.
> 
> After a full GT reset the banned context is no longer registered with
> GuC and is never registered again, so those requests never complete.
> Nothing retires them, the buffers they reference stay pinned, and the
> client waits on their fences forever.
> 
> Seen on ADL-P with a Vulkan compute job (llama.cpp) and a short
> request_timeout_ms: a GuC context reset, the ban, the replay moving
> 10 requests back to the queue, then a full GT reset after a stopped
> heartbeat. Afterwards the context was unregistered and the GuC submit
> queue was empty, but 9 of its requests never completed: 7.5 GB of
> objects stayed pinned after the process was killed.
> 
> Complete every request on the context's list, as
> guc_cancel_context_requests() does, but only when the context is off
> the HW (not enabled, no enable or disable in flight), so a request
> that
> is still executing is never marked complete. The submitted request is
> on that list too, so it is completed on its own only when the context
> is still on the HW, and nothing touches it after the lock is dropped:
> once complete it can be retired and freed.
> 
> Found and fixed with Claude Code; details in the cover letter.

This commit message will be part of git history, while the cover letter
won't. So this message doesn't belong here to start with.
The commit needs to be self sufficient and self explained.
Also, please ask your LLM to be concise in the messages and to
avoid unnecessary comments. A good code talks for itself.

With short texts your chances of getting a reviewer increases.

> 
> Fixes: ae8ac10dfd2a ("drm/i915/guc: Implement banned contexts for GuC
> submission")
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15301
> Signed-off-by: Javier MV <javiermv@duck.com>
> ---
> v2:
> - Complete rq on its own only when the loop doesn't run, under the
>   lock. v1 completed it again after the unlock, when the loop had
>   already completed it and it could have been retired and freed
>   (Sashiko).
> 
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 20
> ++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 788e59c..b2ffc6b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -709,7 +709,25 @@ static int __guc_add_request(struct intel_guc
> *guc, struct i915_request *rq)
>  	 * request resubmitted after the context was banned.
>  	 */
>  	if (unlikely(!intel_context_is_schedulable(ce))) {
> -		i915_request_put(i915_request_mark_eio(rq));
> +		struct i915_request *pos;
> +
> +		/*
> +		 * The dequeue only passes the last of the merged
> requests, and
> +		 * the context won't run again to write the others'
> breadcrumbs:
> +		 * complete them all (rq is on the list too), once
> the context
> +		 * is off the HW. Don't touch rq after the unlock:
> once complete
> +		 * it can be retired and freed.
> +		 */
> +		spin_lock(&ce->guc_state.lock);
> +		if (!context_enabled(ce) &&
> !context_pending_enable(ce) &&
> +		    !context_pending_disable(ce)) {
> +			list_for_each_entry(pos, &ce-
> >guc_state.requests, sched.link)
> +				i915_request_put(i915_request_mark_e
> io(pos));
> +		} else {
> +			i915_request_put(i915_request_mark_eio(rq));
> +		}
> +		spin_unlock(&ce->guc_state.lock);
> +
>  		intel_engine_signal_breadcrumbs(ce->engine);
>  		return 0;
>  	}