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/vmwgfx: Change ttm refs for dumb buffers. Date: Sat, 16 May 2026 14:26:31 +1000 Message-ID: In-Reply-To: <20260512002752.2826685-5-maaz.mombasawala@broadcom.com> References: <20260512002752.2826685-1-maaz.mombasawala@broadcom.com> <20260512002752.2826685-5-maaz.mombasawala@broadcom.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 **The dma_buf leak fix in ttm_prime_fd_to_handle is clearly correct.** The = original code returns `-ENOSYS` without calling `dma_buf_put(dma_buf)`, lea= king the reference obtained from `dma_buf_get(fd)`: ```c - if (dma_buf->ops !=3D &tdev->ops) - return -ENOSYS; + if (dma_buf->ops !=3D &tdev->ops) { + ret =3D -ENOSYS; + goto out; + } ``` This is a straightforward and correct fix. **The vmw_dumb_create change needs careful consideration.** The patch chang= es the error path to conditionally unref: ```c - ttm_ref_object_base_unref(tfile, arg.rep.handle); + if (ret) + ttm_ref_object_base_unref(tfile, arg.rep.handle); ``` Combined with `usurf->prime.base.refcount_release =3D NULL`, this preserves= the TTM reference for dumb buffers on the success path. The idea is that t= he GEM object holds the surface alive, and the TTM refcount_release callbac= k is nulled out so that when the TTM ref is eventually dropped, it doesn't = try to destroy the surface (since the GEM path handles that). This is a som= ewhat subtle lifetime management change =E2=80=94 the commit message should= explain the ownership model more clearly. **Setting `refcount_release =3D NULL` is outside the error check.** The lin= e `usurf->prime.base.refcount_release =3D NULL;` executes before the `err:`= label, meaning it only runs on the success path. This is correct, but the = code flow would be clearer if there were a comment explaining why. --- Generated by Claude Code Patch Reviewer