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: Clarify external lock use case in dma_fence_init() docs Date: Thu, 04 Jun 2026 14:48:33 +1000 Message-ID: In-Reply-To: <20260531125115.1136036-1-mcanal@igalia.com> References: <20260531125115.1136036-1-mcanal@igalia.com> <20260531125115.1136036-1-mcanal@igalia.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 of the new wording:** The replacement text for `dma_fence_init()`: ``` + * External locks are a relic of legacy use cases that needed a shared lock + * to serialize signaling when no out-of-order signaling was possible thro= ugh + * &dma_fence_ops.signaled. Drivers have abandoned this concept since the + * introduction of the callback, but the external lock is still around. New + * users MUST NOT use external locks, as they force the issuer to outlive = all + * fences that reference the lock. ``` This is accurate. The old comment implied the shared lock *prevented* out-o= f-order signaling, but in reality signaling order is the driver's responsib= ility. The new text correctly identifies the actual purpose (serializing si= gnaling) and the actual problem with external locks (lifetime coupling =E2= =80=94 the issuer must outlive all fences). The reference to `&dma_fence_op= s.signaled` as the mechanism that made external locks unnecessary is techni= cally sound, as the `.signaled` callback allows the fence framework to peek= at fence state without holding the shared lock. **De-duplication in `dma_fence_init64()`:** ``` + * New users MUST NOT use external locks. Check the documentation in + * dma_fence_init() to understand the motives behind the legacy use cases. ``` This is a reasonable approach. One very minor nit: kerneldoc cross-referenc= es typically use `&function()` or are more explicit. The plain text "dma_fe= nce_init()" will render fine in the generated docs, but using `dma_fence_in= it()` as-is is conventional enough and readable. Not worth a respin. **Minor observation:** The phrase "when no out-of-order signaling was possi= ble through `&dma_fence_ops.signaled`" could be read two ways =E2=80=94 eit= her "when the `.signaled` callback couldn't handle out-of-order signaling" = or "when out-of-order signaling was not possible [and they relied on] `.sig= naled`". In context, the intended meaning (the former) is clear enough, esp= ecially for the audience of dma-fence developers. **No issues found.** This is a clean documentation improvement with correct= technical content.=20 Reviewed-by recommendation: **Accept as-is.** --- Generated by Claude Code Patch Reviewer