Re: [PATCH v5 02/10] drm/msm: Rework queuelock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The conversion to `guard(rwsem_write)` in `msm_submitqueue_remove()` extends the lock's scope to cover `msm_submitqueue_put()`, leading to lock contention and potential deadlocks.
--

--- Patch [2]: [PATCH v5 02/10] drm/msm: Rework queuelock ---
commit 578ea2d7dbb1b26cbd22beab7c73963f3cc195e7
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Rework queuelock

This commit renames queuelock to ctxlock and changes it from a rwlock
to a rw_semaphore. It also introduces cleanup guards to manage releasing
the lock.

> 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);

[Severity: High]
Does extending the lock's scope over msm_submitqueue_put() create a deadlock
risk?

By using guard(rwsem_write), the ctxlock remains held until the function
returns. Previously, the lock was explicitly released before calling
msm_submitqueue_put().

For VM_BIND queues, dropping the last reference can call
drm_sched_entity_destroy(), which blocks in drm_sched_entity_flush() waiting
for pending jobs to finish. If userspace submits a job with an in-fence
dependency and then removes the queue, the kernel will wait inside
msm_submitqueue_put() while still holding the write lock. 

If another thread needs the ctxlock to signal the dependency or perform other
IOCTLs on the same context, could this result in a permanent deadlock?

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

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