From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups Date: Thu, 04 Jun 2026 13:44:21 +1000 Message-ID: In-Reply-To: <20260601-v3d-fix-indirect-csd-v2-1-aaebf035b936@igalia.com> References: <20260601-v3d-fix-indirect-csd-v2-0-aaebf035b936@igalia.com> <20260601-v3d-fix-indirect-csd-v2-1-aaebf035b936@igalia.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 **Status: Good** This patch fixes a straightforward resource leak. In `v3d_rewrite_csd_job_w= g_counts_from_indirect()`, two BOs are mapped at the top of the function: ```c v3d_get_bo_vaddr(bo); v3d_get_bo_vaddr(indirect); ``` When any workgroup count is zero, the early `return` at line 406 skips the = cleanup: ```c if (wg_counts[0] =3D=3D 0 || wg_counts[1] =3D=3D 0 || wg_counts[2] =3D=3D 0) return; // leaked both vaddr mappings ``` The fix correctly changes `return` to `goto unmap_bo`, jumping to the exist= ing cleanup that calls `v3d_put_bo_vaddr()` on both BOs. The label placemen= t is correct =E2=80=94 it's right before both `v3d_put_bo_vaddr` calls at t= he end of the function. One minor observation: the `goto` label name `unmap_bo` references only one= BO, but both `indirect` and `bo` are unmapped there. A name like `unmap` w= ould be slightly more precise, but this is a very minor style nit and not w= orth a respin given it follows common kernel convention where the label nam= es the first cleanup action. No issues with this patch. --- Generated by Claude Code Patch Reviewer