[PATCH 1/2] dma-fence: Rename dma_fence_is_signaled()

[Christian König]

Am 09.04.25 um 17:04 schrieb Philipp Stanner:
> On Wed, 2025-04-09 at 16:10 +0200, Christian König wrote:
>>> I only see improvement by making things more obvious.
>>>
>>> In any case, how would you call a wrapper that just does
>>> test_bit(IS_SIGNALED, …) ?
>> Broken, that was very intentionally removed quite shortly after we
>> created the framework.
>>
>> We have a few cases were implementations do check that for their
>> fences, but consumers should never be allowed to touch such
>> internals.
> There is theory and there is practice. In practice, those internals are
> being used by Nouveau, i915, Xe, vmgfx and radeon.

What do you mean? I only skimmed over the use cases, but as far as I can see those are all valid.

You can test the flag if you know what the fence means to you, that is not a problem at all.

> So it seems that we failed quite a bit at communicating clearly how the
> interface should be used.
>
> And, to repeat myself, with both name and docu of that function, I
> think it is very easy to misunderstand what it's doing. You say that it
> shouldn't matter – and maybe that's true, in theory. In practice, it
> does matter. In practice, APIs get misused and have side-effects. And
> making that harder is desirable.

That sounds like I didn't used the right wording.

It *must* not matter to the consumer. See the purpose of the DMA-fence framework is to make it irrelevant for the consumer how the provider has implemented it's fences.

This means that things like if polling or interrupt driven signaling is used, 32bit vs 64bit seq numbers, etc... should all be hidden by the framework from the consumer of the fences.


BTW I'm actually not sure if nouveau has a bug here. As far as I can see nouveau_fence_signal() will be called later eventually and do the necessary cleanup.

But on the other hand it wouldn't surprise me if nouveau has a bug with that. The driver has been basically only barely maintained for quite a while.

> In any case, I might have to add another such call to Nouveau, because
> the solution preferred by you over the callback causes another race.
> Certainly one could solve this in a clean way, but someone has to do
> the work, and we're talking about more than a few hours here.

Well this is not my preferred solution, it's just the technical correct solution as far as I can see.

> In any case, be so kind and look at patch 2 and tell me there if you're
> at least OK with making the documentation more detailed.

As far as I can see that is clearly the wrong place to document that stuff.

Regards,
Christian.

>
> P.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250410/9e74a761/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-nouveau-fix-and-cleanup-fence-handling.patch
Type: text/x-patch
Size: 2670 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250410/9e74a761/attachment.bin>