From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dma-fence: Use dma_fence_set_error() internally Date: Thu, 04 Jun 2026 11:41:51 +1000 Message-ID: In-Reply-To: <20260603150531.2217691-2-phasta@kernel.org> References: <20260603150531.2217691-2-phasta@kernel.org> <20260603150531.2217691-2-phasta@kernel.org> 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: Looks good** The change at `drivers/dma-buf/dma-fence.c:602`: ```c - fence->error =3D -EDEADLK; + dma_fence_set_error(fence, -EDEADLK); ``` **Positive aspects:** - Uses the proper API helper instead of poking the struct field directly, w= hich is exactly what `dma_fence_set_error()` was designed for. - The additional `WARN_ON` checks in the helper add a safety net =E2=80=94 = if someone ever manages to hit this path with an already-signaled fence, it= will now produce a warning rather than silently corrupting state. - Commit message is clear and accurately describes the change. **Minor observations:** - The commit message says "Only compile-tested" which is reasonable for a o= ne-line cleanup, but the `WARN_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fe= nce->flags))` in the helper is worth mentioning in the commit message, sinc= e it adds a new runtime check in an error path. It's a beneficial check, bu= t reviewers should be aware it's not a pure no-op substitution. - The pre-existing race between the unsynchronized `!dma_fence_test_signale= d_flag(fence)` check (line 581) and the locked region (line 601) is not int= roduced by this patch and is not a concern for this review =E2=80=94 but if= the `WARN_ON` ever fires spuriously in the field, this is the path to inve= stigate. **Verdict:** Reviewed-by worthy. The patch improves code consistency with n= egligible risk. --- Generated by Claude Code Patch Reviewer