[Intel-xe] [PATCH v2] drm/xe/guc_submit: fixup deregister in job timeout

[Matthew Brost]

On Tue, Aug 08, 2023 at 10:19:04AM +0100, Matthew Auld wrote:
> Rather check if the engine is still registered before proceeding with
> deregister steps. Also the engine being marked as disabled doesn't mean
> the engine has been disabled or deregistered from GuC pov, and here we
> are signalling fences so we need to be sure GuC is not still using this
> context.
> 
> v2:
>  - Drop the read_stopped() for this path. Since we are signalling
>    fences on error here, best play it safe and wait for the GT reset to
>    mark the engine as disabled, rather than it just being queued.
> 
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Matthew Brost <matthew.brost at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 52c61f78b083..6126ddf2fdd5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -881,15 +881,17 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>  	}
>  
>  	/* Engine state now stable, disable scheduling if needed */
> -	if (exec_queue_enabled(q)) {
> +	if (exec_queue_registered(q)) {
>  		struct xe_guc *guc = exec_queue_to_guc(q);
>  		int ret;
>  
>  		if (exec_queue_reset(q))
>  			err = -EIO;
>  		set_exec_queue_banned(q);
> -		xe_exec_queue_get(q);
> -		disable_scheduling_deregister(guc, q);
> +		if (!exec_queue_destroyed(q)) {
> +			xe_exec_queue_get(q);
> +			disable_scheduling_deregister(guc, q);
> +		}
>  
>  		/*
>  		 * Must wait for scheduling to be disabled before signalling
> @@ -901,8 +903,8 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>  		 */
>  		smp_rmb();
>  		ret = wait_event_timeout(guc->ct.wq,
> -					 !exec_queue_pending_disable(q) ||
> -					 guc_read_stopped(guc), HZ * 5);

I think we want the guc_read_stopped here as we want to pop out of the
wait if a GT reset is scheduled.

> +					 !exec_queue_pending_disable(q),
> +					 HZ * 5);
>  		if (!ret) {

Then here we want the check to be:

!ret || guc_read_stopped

As we want delay the signaling of fences behind the GT reset.

Matt

>  			XE_WARN_ON("Schedule disable failed to respond");
>  			sched->timeout = MIN_SCHED_TIMEOUT;
> -- 
> 2.41.0
>