On Mon, 2026-05-18 at 13:30 +0200, Christian König wrote: > 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. >
I think the point Maíra / we tried to make with that statement is that an external lock is effectively useless for ensuring correct signaling order. Afair you agree with that in general. Do you have a suggestion for a better formulation? Regards P. > 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, >
