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

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Aug 09, 2023 at 09:16:18AM +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.
> v3 (Matt Brost):
>  - Keep the read_stopped() on the wait event, since there is no need to
>    wait for an already scheduled GT reset. If it is set we can then just
>    bail without signalling anything.
> 
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 52c61f78b083..90ed47cb509b 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
> @@ -903,7 +905,7 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>  		ret = wait_event_timeout(guc->ct.wq,
>  					 !exec_queue_pending_disable(q) ||
>  					 guc_read_stopped(guc), HZ * 5);
> -		if (!ret) {
> +		if (!ret || guc_read_stopped(guc)) {
>  			XE_WARN_ON("Schedule disable failed to respond");
>  			sched->timeout = MIN_SCHED_TIMEOUT;
>  			list_add(&drm_job->list, &sched->pending_list);
> -- 
> 2.41.0
>