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 mcanal@igalia.com ---
Hi,
While reading the documentation, I found this particular paragraph quite hard to understand. As I understand it, locks don't enforce order, only serialization, but the paragraph seems to communicate the other way around. Due to that, I had the impression that the current wording can be misleading for driver developers.
I'm proposing a new wording to better describe the use case of the external lock based on my understanding, but it would be great to hear the feedback and suggestions from more experienced developers who might have more insight about these legacy use cases.
Best regards, - Maíra
drivers/dma-buf/dma-fence.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1826ba73094c..bdc29d1c1b5c 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1102,8 +1102,10 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, * 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. + * 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 earlier + * one isn't. */ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, @@ -1129,8 +1131,10 @@ EXPORT_SYMBOL(dma_fence_init); * 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. + * 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 earlier + * one isn't. */ void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
On Sat, 2026-04-11 at 15:47 -0300, 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 mcanal@igalia.com
Hi,
While reading the documentation, I found this particular paragraph quite hard to understand. As I understand it, locks don't enforce order, only serialization, but the paragraph seems to communicate the other way around.
Yes, 100%. That's one of the reasons why Christian recently moved to using inline- locks.
Due to that, I had the impression that the current wording can be misleading for driver developers.
I'm proposing a new wording to better describe the use case of the external lock based on my understanding, but it would be great to hear the feedback and suggestions from more experienced developers who might have more insight about these legacy use cases.
Best regards,
- Maíra
drivers/dma-buf/dma-fence.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1826ba73094c..bdc29d1c1b5c 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1102,8 +1102,10 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, * 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.
- 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 earlier
- one isn't.
I would probably delete the explanation altogether and just state "allowed for legacy reasons". The legacy people know why it's allowed, and new users don't need to care. Same below of course.
Thx P.
*/ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, @@ -1129,8 +1131,10 @@ EXPORT_SYMBOL(dma_fence_init); * 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.
- 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 earlier
- one isn't.
*/ void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
Hi Philipp,
On 4/13/26 03:47, Philipp Stanner wrote:
On Sat, 2026-04-11 at 15:47 -0300, 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 mcanal@igalia.com
Hi,
While reading the documentation, I found this particular paragraph quite hard to understand. As I understand it, locks don't enforce order, only serialization, but the paragraph seems to communicate the other way around.
Yes, 100%. That's one of the reasons why Christian recently moved to using inline- locks.
Due to that, I had the impression that the current wording can be misleading for driver developers.
I'm proposing a new wording to better describe the use case of the external lock based on my understanding, but it would be great to hear the feedback and suggestions from more experienced developers who might have more insight about these legacy use cases.
Best regards,
- Maíra
drivers/dma-buf/dma-fence.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1826ba73094c..bdc29d1c1b5c 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1102,8 +1102,10 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, * 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.
- 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 earlier
- one isn't.
I would probably delete the explanation altogether and just state "allowed for legacy reasons". The legacy people know why it's allowed, and new users don't need to care. Same below of course.
Actually, I ended up in this part of the documentation as I'm thinking about dropping the external lock in v3d driver. I was reading the docs exactly to know if v3d is a legacy use case (spoiler: it isn't) and I got confused by the current wording. So, I think there is value in documenting the legacy use cases for the external lock.
Maybe, instead of "strongly discouraged", we could add a disclaimer like: "New drivers MUST NOT use an external lock because...". What do you think?
Best regards, - Maíra
Thx P.
*/ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, @@ -1129,8 +1131,10 @@ EXPORT_SYMBOL(dma_fence_init); * 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.
- 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 earlier
- one isn't.
*/ void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
On Mon, 2026-04-13 at 09:00 -0300, Maíra Canal wrote:
Hi Philipp,
On 4/13/26 03:47, Philipp Stanner wrote:
On Sat, 2026-04-11 at 15:47 -0300, 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 mcanal@igalia.com
Hi,
While reading the documentation, I found this particular paragraph quite hard to understand. As I understand it, locks don't enforce order, only serialization, but the paragraph seems to communicate the other way around.
Yes, 100%. That's one of the reasons why Christian recently moved to using inline- locks.
Due to that, I had the impression that the current wording can be misleading for driver developers.
I'm proposing a new wording to better describe the use case of the external lock based on my understanding, but it would be great to hear the feedback and suggestions from more experienced developers who might have more insight about these legacy use cases.
Best regards,
- Maíra
drivers/dma-buf/dma-fence.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1826ba73094c..bdc29d1c1b5c 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1102,8 +1102,10 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, * 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.
- 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 earlier
- one isn't.
I would probably delete the explanation altogether and just state "allowed for legacy reasons". The legacy people know why it's allowed, and new users don't need to care. Same below of course.
Actually, I ended up in this part of the documentation as I'm thinking about dropping the external lock in v3d driver. I was reading the docs exactly to know if v3d is a legacy use case (spoiler: it isn't) and I got confused by the current wording. So, I think there is value in documenting the legacy use cases for the external lock.
Maybe, instead of "strongly discouraged", we could add a disclaimer like: "New drivers MUST NOT use an external lock because...". What do you think?
Hmm. Yeah, I mean you are right that what the lock was supposed to be good for was never really applicable anyways.
Thinking about it, I would probably state:
"External locks are a relic from a past design mistake. New users must not use external locks since they do not provide value and prevent fences' life time to be completely decoupled from their issuer's."
Sth like that sounds to the point to me.
P.
Best regards,
- Maíra
Thx P.
*/ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, @@ -1129,8 +1131,10 @@ EXPORT_SYMBOL(dma_fence_init); * 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.
- 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 earlier
- one isn't.
*/ void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
linaro-mm-sig@lists.linaro.org
-
Maíra Canal -
Philipp Stanner