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
---
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4-mcanal@igalia.com...
- 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. */ 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,
On Sun, 2026-04-19 at 10:48 -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
Reviewed-by: Philipp Stanner phasta@kernel.org
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4-mcanal@igalia.com...
- 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.
*/ 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,
Hi Christian,
When possible, could you take a look at this patch? If you are okay with it, I can apply it to drm-misc-next.
Best regards, - Maíra
On 20/04/26 06:18, Philipp Stanner wrote:
On Sun, 2026-04-19 at 10:48 -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
Reviewed-by: Philipp Stanner phasta@kernel.org
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4-mcanal@igalia.com...
- 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.
*/ 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,
+cc Christian's AMD e-mail
On 5/4/26 11:47, Maíra Canal wrote:
Hi Christian,
When possible, could you take a look at this patch? If you are okay with it, I can apply it to drm-misc-next.
Gentle ping.
Best regards, - Maíra
Best regards,
- Maíra
On 20/04/26 06:18, Philipp Stanner wrote:
On Sun, 2026-04-19 at 10:48 -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
Reviewed-by: Philipp Stanner phasta@kernel.org
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4- mcanal@igalia.com/
- 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.
*/ 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,
+cc Christian's AMD e-mail
On 5/4/26 11:47, Maíra Canal wrote:
Hi Christian,
When possible, could you take a look at this patch? If you are okay with it, I can apply it to drm-misc-next.
Gentle ping.
Best regards, - Maíra
Best regards,
- Maíra
On 20/04/26 06:18, Philipp Stanner wrote:
On Sun, 2026-04-19 at 10:48 -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
Reviewed-by: Philipp Stanner phasta@kernel.org
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4- mcanal@igalia.com/
- 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.
*/ 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,
On 5/15/26 00:39, Maíra Canal wrote:
+cc Christian's AMD e-mail
On 5/4/26 11:47, Maíra Canal wrote:
Hi Christian,
When possible, could you take a look at this patch? If you are okay with it, I can apply it to drm-misc-next.
Gentle ping.
Sorry for the delay, please always CC my AMD email address as well.
Regards, Christian.
Best regards,
- Maíra
Best regards,
- Maíra
On 20/04/26 06:18, Philipp Stanner wrote:
On Sun, 2026-04-19 at 10:48 -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
Reviewed-by: Philipp Stanner phasta@kernel.org
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4- mcanal@igalia.com/
- 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.
*/ 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,
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 mcanal@igalia.com
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4-mcanal@igalia.com...
- 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,
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 mcanal@igalia.com
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4-mcanal@igalia.com...
- 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,
On 5/18/26 15:00, Philipp Stanner wrote:
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 mcanal@igalia.com
v1 -> v2: https://lore.kernel.org/dri-devel/20260411185756.1887119-4-mcanal@igalia.com...
- 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.
Well yeah it's just historically.
The point is *if* you use one lock for all your fences *and not* have an is_signaled callback then all fences signal in order.
It's just that as far as I know all drivers sooner or later implemented the is_signaled callback because of the performance benefits and so abandoned the signaling order approach.
So we could in theory fix all drivers to not use the external lock, but that's tons of extra work which nobody wants to do for a radeon, nouveau or QXL driver.
Afair you agree with that in general.
Do you have a suggestion for a better formulation?
Yeah I really liked the "relic" and "MUST NOT" wording, that is pretty much exactly what we need here.
Maybe something like that here:
* External locks are a relic from legacy use cases that needed a shared lock * to serialize signaling when no out of order signaling through the is_signaled * callback was possible. We have long abandoned that concept, 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.
Regards, Christian.
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,
linaro-mm-sig@lists.linaro.org
-
Christian König -
Maíra Canal -
Philipp Stanner