Re: [PATCH v6 02/16] drm/msm: Rework queuelock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Deadlock in `msm_submitqueue_remove()` due to holding `ctxlock` during queue destruction.
--

--- Patch [2]: [PATCH v6 02/16] drm/msm: Rework queuelock ---
commit 308ab05b33546bb1bb25510a8f2b68c4b798cefc
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Rework queuelock

Rename to ctxlock, and use cleanup guards to manage releasing the lock.
This will let us re-use it for other per-context read/write serial-
ization, such as VM creation.

> diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c
> index 1a5a77b280163..8b5980d5a6a79 100644
> --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> +++ b/drivers/gpu/drm/msm/msm_submitqueue.c

[ ... ]

> @@ -335,19 +330,16 @@ int msm_submitqueue_remove(struct msm_context *ctx, u32 id)
>  	if (!id)
>  		return -ENOENT;
>  
> -	write_lock(&ctx->queuelock);
> +	guard(rwsem_write)(&ctx->ctxlock);
>  
>  	list_for_each_entry(entry, &ctx->submitqueues, node) {
>  		if (entry->id == id) {
>  			list_del(&entry->node);
> -			write_unlock(&ctx->queuelock);
> -
>  			msm_submitqueue_put(entry);
>  			return 0;
>  		}
>  	}

[Severity: High]
Does using guard() here extend the lock lifetime and create a deadlock?

In the original code, the lock was explicitly dropped before calling
msm_submitqueue_put(entry). By using guard(), the write lock is now held
during the msm_submitqueue_put() call since the scope of the guard is the
entire function.

If a user destroys a queue while jobs are pending, msm_submitqueue_put() can
drop the last reference. This calls drm_sched_entity_destroy(), which blocks
waiting for those pending jobs to complete.

If another job needs to be submitted to signal a fence that the pending job is
waiting on, the submission path will call msm_submitqueue_get() and block
trying to acquire ctxlock for reading. This creates a circular deadlock where
queue destruction waits for the job to complete, but the job waits for a
submission that is blocked waiting for ctxlock.

> -	write_unlock(&ctx->queuelock);
>  	return -ENOENT;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717204841.26120-1-robin.clark@oss.qualcomm.com?part=2