From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Re: [PATCH 1/2] drm/amdgpu: protect waitq access with userq_mutex in wait IOCTL Date: Tue, 10 Mar 2026 12:44:43 +1000 Message-ID: In-Reply-To: <91221541-6d21-44a2-a9b2-f1a30c27327a@amd.com> References: <20260309022229.63071-1-chenyuan_mi@163.com> <91221541-6d21-44a2-a9b2-f1a30c27327a@amd.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Already fixed upstream via refcounting.** The current tree uses: ```c waitq =3D amdgpu_userq_get(userq_mgr, wait_info->waitq_id); ``` (line 866) instead of the raw `xa_load()` this patch targets. The `amdgpu_u= serq_get()` function takes a kref under xa_lock, and `amdgpu_userq_put()` i= s called at the end (line 951), providing proper lifetime protection withou= t holding the mutex across the entire loop. **Even evaluated against the older base, there is a concern:** - **Sleeping under mutex**: The patch holds `userq_mutex` across `dma_fence= _wait(fences[i], true)` (line 909 in the diff context). `dma_fence_wait` wi= th `intr=3Dtrue` can sleep for an extended/unbounded period waiting for GPU= work to complete. Holding `userq_mutex` during this time blocks all other = userqueue operations (create, destroy, signal) for this file descriptor. Th= is is a significant liveness concern =E2=80=94 a slow or stalled fence woul= d effectively deadlock the entire userqueue subsystem for the process. The = refcounting approach in drm-next avoids this problem entirely. --- Generated by Claude Code Patch Reviewer