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/msm: fbdev: Use a DRM client buffer Date: Thu, 23 Apr 2026 08:30:53 +1000 Message-ID: In-Reply-To: <20260421125733.209568-6-tzimmermann@suse.de> References: <20260421125733.209568-1-tzimmermann@suse.de> <20260421125733.209568-6-tzimmermann@suse.de> 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: Bug in error handling.** The core conversion to use `drm_client_buffer_create()` and `drm_client_buf= fer_delete()` is correct in concept. The success path reference counting is= sound: 1. `msm_gem_new()` =E2=86=92 refcount =3D 1 2. `drm_gem_handle_create(file, bo, &handle)` =E2=86=92 refcount =3D 2 3. `drm_client_buffer_create()` (internally looks up handle, fb takes ref) = =E2=86=92 refcount =3D 3 4. `drm_gem_handle_delete(file, handle)` =E2=86=92 refcount =3D 2 5. `drm_gem_object_put(bo)` =E2=86=92 refcount =3D 1 (only the framebuffer = holds a reference) The destroy path in `msm_fbdev_fb_destroy()` is also correct: `msm_gem_put_= vaddr()` releases the mapping, then `drm_client_buffer_delete()` removes th= e framebuffer (dropping the last GEM reference). **Bug:** The error goto when `msm_gem_get_vaddr()` fails is wrong: ```c fbi->screen_buffer =3D msm_gem_get_vaddr(bo); if (IS_ERR(fbi->screen_buffer)) { ret =3D PTR_ERR(fbi->screen_buffer); goto err_msm_gem_put_vaddr; // BUG: vaddr was never acquired } ``` Looking at `get_vaddr()` in `msm_gem.c:708`, when it fails it cleans up int= ernally (decrements `vmap_count`, unpins). Calling `msm_gem_put_vaddr()` af= ter a failed `msm_gem_get_vaddr()` would decrement `vmap_count` below its c= orrect value, triggering: ```c GEM_WARN_ON(msm_obj->vmap_count < 1); // msm_gem.c:782 ``` **The fix:** This goto should be `goto err_drm_client_buffer_delete;` inste= ad. --- Generated by Claude Code Patch Reviewer