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:50 +1000 Message-ID: In-Reply-To: <20260603150531.2217691-2-phasta@kernel.org> References: <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 Overall Series Review Subject: dma-fence: Use dma_fence_set_error() internally Author: Philipp Stanner Patches: 1 Reviewed: 2026-06-04T11:41:50.918398 --- This is a single-patch series making a trivial cleanup in `dma_fence_releas= e()`: replacing a direct assignment `fence->error =3D -EDEADLK` with the AP= I helper `dma_fence_set_error(fence, -EDEADLK)`. The intent is to improve r= eadability and consistency by using the designated API. The change is **correct but has a minor behavioral subtlety worth noting**.= The `dma_fence_set_error()` helper (defined at `include/linux/dma-fence.h:= 686`) includes two `WARN_ON` checks: ```c WARN_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)); WARN_ON(error >=3D 0 || error < -MAX_ERRNO); ``` The second check is trivially satisfied since `-EDEADLK` is always a valid = negative errno. The first check =E2=80=94 that the fence is not already sig= naled =E2=80=94 is also safe in this context because the enclosing `if` at = line 581 explicitly verified `!dma_fence_test_signaled_flag(fence)` before = entering the block. There is a theoretical race window: the `!dma_fence_test_signaled_flag()` c= heck at line 581 is done without the fence lock, and the lock is only acqui= red at line 601. In principle, another thread could signal the fence betwee= n the check and the lock acquisition, causing the `WARN_ON` inside `dma_fen= ce_set_error()` to fire. **However, this same race existed before this patc= h** =E2=80=94 the original code would have set the error on an already-sign= aled fence too, and then called `dma_fence_signal_locked()` which also has = its own signaled-state handling. Furthermore, we are in `dma_fence_release(= )` meaning the refcount has hit zero, so concurrent signaling would be a se= parate bug. This patch does not make that situation any worse; the new `WAR= N_ON` would actually *help* detect it. Overall: a clean, low-risk readability improvement. No functional concerns. --- Generated by Claude Code Patch Reviewer