[PATCH] dma-fence: Use dma_fence_set_error() internally

[PATCH] dma-fence: Use dma_fence_set_error() internally

[Philipp Stanner]

The dma_fence implementation sets a fence error in dma_fence_release()
manually.

Make the code a bit more readable by using the appropriate API function
for that.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
Only compile-tested.
---
 drivers/dma-buf/dma-fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index c7ea1e75d38a..ae5550cda421 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -599,7 +599,7 @@ void dma_fence_release(struct kref *kref)
 		 * so that the callbacks know this signal is due to an error.
 		 */
 		dma_fence_lock_irqsave(fence, flags);
-		fence->error = -EDEADLK;
+		dma_fence_set_error(fence, -EDEADLK);
 		dma_fence_signal_locked(fence);
 		dma_fence_unlock_irqrestore(fence, flags);
 	}
-- 
2.54.0

Claude review: dma-fence: Use dma_fence_set_error() internally

[Claude Code Review Bot]

Overall Series Review

Subject: dma-fence: Use dma_fence_set_error() internally
Author: Philipp Stanner <phasta@kernel.org>
Patches: 1
Reviewed: 2026-06-04T11:41:50.918398

---

This is a single-patch series making a trivial cleanup in `dma_fence_release()`: replacing a direct assignment `fence->error = -EDEADLK` with the API helper `dma_fence_set_error(fence, -EDEADLK)`. The intent is to improve readability 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 >= 0 || error < -MAX_ERRNO);
```

The second check is trivially satisfied since `-EDEADLK` is always a valid negative errno. The first check — that the fence is not already signaled — 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()` check at line 581 is done without the fence lock, and the lock is only acquired at line 601. In principle, another thread could signal the fence between the check and the lock acquisition, causing the `WARN_ON` inside `dma_fence_set_error()` to fire. **However, this same race existed before this patch** — the original code would have set the error on an already-signaled 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 separate bug. This patch does not make that situation any worse; the new `WARN_ON` would actually *help* detect it.

Overall: a clean, low-risk readability improvement. No functional concerns.

---
Generated by Claude Code Patch Reviewer

Claude review: dma-fence: Use dma_fence_set_error() internally

[Claude Code Review Bot]

Patch Review

**Status: Looks good**

The change at `drivers/dma-buf/dma-fence.c:602`:
```c
-		fence->error = -EDEADLK;
+		dma_fence_set_error(fence, -EDEADLK);
```

**Positive aspects:**
- Uses the proper API helper instead of poking the struct field directly, which is exactly what `dma_fence_set_error()` was designed for.
- The additional `WARN_ON` checks in the helper add a safety net — 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 one-line cleanup, but the `WARN_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))` in the helper is worth mentioning in the commit message, since it adds a new runtime check in an error path. It's a beneficial check, but reviewers should be aware it's not a pure no-op substitution.
- The pre-existing race between the unsynchronized `!dma_fence_test_signaled_flag(fence)` check (line 581) and the locked region (line 601) is not introduced by this patch and is not a concern for this review — but if the `WARN_ON` ever fires spuriously in the field, this is the path to investigate.

**Verdict:** Reviewed-by worthy. The patch improves code consistency with negligible risk.

---
Generated by Claude Code Patch Reviewer