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: Sun, 12 Apr 2026 08:49:38 +1000 Message-ID: In-Reply-To: <20260411185756.1887119-4-mcanal@igalia.com> References: <20260411185756.1887119-4-mcanal@igalia.com> <20260411185756.1887119-4-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 **Assessment: Good patch, minor nit.** The commit message explains the rationale well: > a shared spinlock does not (and cannot) prevent the signaler from signali= ng out of order. Signaling order is the driver's responsibility regardless = of whether the lock is shared or per-fence. > > What a shared lock actually provides is serialization of signaling and ob= servation across fences in a given context, so that observers never see a l= ater fence signaled while an earlier one is not. This is correct. Looking at `dma_fence_signal()` (line 487), it acquires th= e fence lock, then calls `dma_fence_signal_timestamp_locked()`. If two fenc= es share a lock, signaling one will block until the other's signal+callback= processing completes, ensuring observers under the same lock see a consist= ent ordering. The code change itself: ```c - * lock and fence life time. This is only allowed for legacy use cases when - * multiple fences need to be prevented from signaling out of order. + * lock and fence lifetime. This is only allowed for legacy use cases that= need + * a shared lock to serialize signaling and observation of fences within a + * context, so that observers never see a later fence signaled while an ea= rlier + * one isn't. ``` This is applied identically to both `dma_fence_init()` and `dma_fence_init6= 4()`, which is correct since they share the same doc structure. **Minor nit:** The fix of "life time" =E2=86=92 "lifetime" is a nice bonus = typo fix. It might be worth calling that out in the commit message so revie= wers know it's intentional, though it's trivially obvious. **One small wording suggestion:** The phrase "one isn't" at the end is slig= htly informal for kernel documentation. Consider "one is not" for consisten= cy with the formal tone of the rest of the doc comment. This is purely styl= istic and not a blocking concern. **Reviewed-by worthy: Yes.** The patch is correct, well-explained, and impr= oves clarity. The cover letter also shows good engagement with the communit= y by inviting feedback on the interpretation. --- Generated by Claude Code Patch Reviewer