On 4/19/26 15:48, Maíra Canal wrote: > The kerneldoc comment on dma_fence_init() and dma_fence_init64() describe > the legacy reason to pass an external lock as a need to prevent multiple > fences "from signaling out of order". However, this wording is a bit > misleading: a shared spinlock does not (and cannot) prevent the signaler > from signaling 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 > observation across fences in a given context, so that observers never > see a later fence signaled while an earlier one is not. > > Reword both comments to describe this more accurately. > > Signed-off-by: Maíra Canal <[email protected]> > > --- > > v1 -> v2: > https://lore.kernel.org/dri-devel/[email protected]/ > > - Be more explicit about not allowing new users to use an external lock. > - De-duplicate the explanation in dma_fence_init64() by pointing to the > dma_fence_init() documentation. > --- > drivers/dma-buf/dma-fence.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 1c1eaecaf1b0..63b349ba9a34 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -1102,9 +1102,11 @@ __dma_fence_init(struct dma_fence *fence, const struct > dma_fence_ops *ops, > * context and seqno are used for easy comparison between fences, allowing > * to check which fence is later by simply using dma_fence_later(). > * > - * It is strongly discouraged to provide an external lock because this > couples > - * 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. > + * External locks are a relic from legacy use cases that needed a shared lock > + * to serialize signaling and observation of fences within a context, so that > + * observers never see a later fence signaled while an earlier one isn't. New > + * users MUST NOT use external locks, as they force the issuer to outlive all > + * fences that reference the lock.
No that's not correct. The use of the external lock was indeed to prevent signaling out of order. It is perfectly possible to observe the fence as signaled even without taking the lock because of the unlocked test_bit() operation we use, so the new wording is clearly misleading and not explaining properly what is going on here. Regards, Christian. > */ > void > dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, > @@ -1129,9 +1131,8 @@ EXPORT_SYMBOL(dma_fence_init); > * Context and seqno are used for easy comparison between fences, allowing > * to check which fence is later by simply using dma_fence_later(). > * > - * It is strongly discouraged to provide an external lock because this > couples > - * 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. > + * New users MUST NOT use external locks. Check the documentation in > + * dma_fence_init() to understand the motives behind the legacy use cases. > */ > void > dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
