Re: [Intel-xe] [PATCH] drm/xe: Call __guc_engine_fini_async direct for KERNEL engines

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

On Thu, Jul 27, 2023 at 10:41:07AM -0700, Matthew Brost wrote:
> Usually we call __guc_engine_fini_async via a worker as the engine fini
> can be done from within the GPU scheduler which creates a circular
> dependency without a worker. Kernel engines are fini'd at driver unload
> (not from within the GPU scheduler) so it is safe to directly call
> __guc_engine_fini_async.
> 
> Reported-by: Oded Gabbay <ogabbay@kernel.org>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>

For some reason the BAT result didn't come out :(
it should be better to get some CI on it.
But the code looks correct to me and indeed better,

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 911d4965c27c..6cb64a097297 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -943,27 +943,19 @@ static void __guc_engine_fini_async(struct work_struct *w)
>  	drm_sched_entity_fini(&ge->entity);
>  	drm_sched_fini(&ge->sched);
>  
> -	if (!(e->flags & ENGINE_FLAG_KERNEL)) {
> -		kfree(ge);
> -		xe_engine_fini(e);
> -	}
> +	kfree(ge);
> +	xe_engine_fini(e);
>  }
>  
>  static void guc_engine_fini_async(struct xe_engine *e)
>  {
> -	bool kernel = e->flags & ENGINE_FLAG_KERNEL;
> -
>  	INIT_WORK(&e->guc->fini_async, __guc_engine_fini_async);
> -	queue_work(system_wq, &e->guc->fini_async);
>  
>  	/* We must block on kernel engines so slabs are empty on driver unload */
> -	if (kernel) {
> -		struct xe_guc_engine *ge = e->guc;
> -
> -		flush_work(&ge->fini_async);
> -		kfree(ge);
> -		xe_engine_fini(e);
> -	}
> +	if (e->flags & ENGINE_FLAG_KERNEL)
> +		__guc_engine_fini_async(&e->guc->fini_async);
> +	else
> +		queue_work(system_wq, &e->guc->fini_async);
>  }
>  
>  static void __guc_engine_fini(struct xe_guc *guc, struct xe_engine *e)
> -- 
> 2.34.1
>