Re: [PATCH] drm/xe: Assign queue name in time for drm_sched_init

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

On Fri, May 22, 2026 at 03:40:03PM +0100, Tvrtko Ursulin wrote:
> Currently the queue name is only assigned after the drm scheduler instance
> has been created. This loses information with all logging or debug
> workqueue facilities so lets re-order things a bit so the name gets
> assigned in time.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Thomas Hellstrom <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index afd8cc7bd231..28f3b23527e6 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1961,6 +1961,13 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
>  	timeout = (q->vm && xe_vm_in_lr_mode(q->vm)) ? MAX_SCHEDULE_TIMEOUT :
>  		  msecs_to_jiffies(q->sched_props.job_timeout_ms);
>  
> +	mutex_lock(&guc->submission_state.lock);
> +	err = alloc_guc_id(guc, q);
> +	mutex_unlock(&guc->submission_state.lock);
> +	if (err)
> +		goto err_free;
> +	xe_exec_queue_assign_name(q, q->guc->id);

I'm afraid this might open some race conditions here.

First I wondering about simply moving the mutex to inside the alloc_guc_id
itself where the submission_state is used so it gets clear, then
I saw the FIXME message in there and wondered why could had made it
harder when it was first developed.

Then I read the Sashiko
https://sashiko.dev/#/patchset/20260522144003.12836-1-tvrtko.ursulin%40igalia.com

Perhaps the safest approach here is to use a unlocked xa_reserve to get
the id and create the name. With this you already get the GFP_NOWAIT FIXME note
fixed.

Then you keep the xa_store down later, not opening for any possible races.
2-in-1 fix...

> +
>  	/*
>  	 * Use primary queue's submit_wq for all secondary queues of a
>  	 * multi queue group. This serialization avoids any locking around
> @@ -1977,28 +1984,20 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
>  			    timeout, guc_to_gt(guc)->ordered_wq, NULL,
>  			    q->name, gt_to_xe(q->gt)->drm.dev);
>  	if (err)
> -		goto err_free;
> +		goto err_release_id;
>  
>  	sched = &ge->sched;
>  	err = xe_sched_entity_init(&ge->entity, sched);
>  	if (err)
>  		goto err_sched;
>  
> -	mutex_lock(&guc->submission_state.lock);
> -
> -	err = alloc_guc_id(guc, q);
> -	if (err)
> -		goto err_entity;
> -
>  	q->entity = &ge->entity;
>  
> +	mutex_lock(&guc->submission_state.lock);
>  	if (xe_guc_read_stopped(guc) || vf_recovery(guc))
>  		xe_sched_stop(sched);
> -
>  	mutex_unlock(&guc->submission_state.lock);
>  
> -	xe_exec_queue_assign_name(q, q->guc->id);
> -
>  	/*
>  	 * Maintain secondary queues of the multi queue group in a list
>  	 * for handling dependencies across the queues in the group.
> @@ -2021,11 +2020,10 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
>  
>  	return 0;
>  
> -err_entity:
> -	mutex_unlock(&guc->submission_state.lock);
> -	xe_sched_entity_fini(&ge->entity);
>  err_sched:
>  	xe_sched_fini(&ge->sched);
> +err_release_id:
> +	release_guc_id(guc, q);
>  err_free:
>  	kfree(ge);
>  
> -- 
> 2.54.0
>