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 <[email protected]>

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/[email protected]?part=2

Reply via email to