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/i915/gem: Don't use VMA from wrong VM in EXECBUF Date: Sun, 12 Apr 2026 12:35:35 +1000 Message-ID: In-Reply-To: <20260408110551.84120-1-joonas.lahtinen@linux.intel.com> References: <20260408110551.84120-1-joonas.lahtinen@linux.intel.com> <20260408110551.84120-1-joonas.lahtinen@linux.intel.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 **Correctness: Good.** The logic change is sound: ```c - if (likely(vma && vma->vm =3D=3D vm)) + if (likely(vma && vma->vm =3D=3D vm)) { vma =3D i915_vma_tryget(vma); + } else { + WARN_ON_ONCE(vma && vma->vm !=3D vm); + vma =3D NULL; + } ``` The `else` branch is entered when `!(vma && vma->vm =3D=3D vm)`, which cove= rs two cases: 1. `vma =3D=3D NULL` =E2=80=94 no entry in radix tree (benign, `WARN_ON_ONC= E` condition is false) 2. `vma !=3D NULL && vma->vm !=3D vm` =E2=80=94 wrong VM (the bug, `WARN_ON= _ONCE` fires) Both cases correctly set `vma =3D NULL`, preventing the un-refcounted point= er from being returned at line 904's `if (likely(vma)) return vma;`. **RCU safety:** The `vma->vm` dereference in the `WARN_ON_ONCE` is safe bec= ause we're still inside `rcu_read_lock()`, so the VMA can't be freed undern= eath us. **Minor observations:** 1. **Conflict with current tree:** As noted, the drm-next tree already has = `else vma =3D NULL;` at this location. The patch will need a trivial rebase= to apply on top of that =E2=80=94 essentially just adding the `WARN_ON_ONC= E` inside the existing `else` branch. It would be worth confirming who alre= ady applied the partial fix and whether that commit should be referenced. 2. **WARN_ON_ONCE appropriateness:** Given Sima's analysis that this condit= ion is impossible to trigger on any kernel with proto-contexts (>=3D d4433c= 7600f7), `WARN_ON_ONCE` is the right severity =E2=80=94 it shouldn't fire i= n practice, but if it ever does, it signals a serious regression in the VM = invariant. A `drm_WARN_ON_ONCE` passing the `i915` device might be slightly= preferred for consistency with the rest of the i915 driver's warning style= , but this is a very minor nit. 3. **Commit message quality:** Excellent. The historical analysis, the Fixe= s tag, the References link to the original report, and the explanation of w= hy it's safe to backport are all thorough and helpful for anyone reading th= is in `git log` later. **Verdict:** The patch is correct and ready to merge, pending a trivial reb= ase against the current tree. Consider using `drm_WARN_ON_ONCE()` instead o= f `WARN_ON_ONCE()` for i915 driver convention consistency. --- Generated by Claude Code Patch Reviewer