On Thu, May 22, 2025 at 03:09:49PM +0200, Christian König wrote: > On 5/22/25 14:59, Danilo Krummrich wrote: > > On Thu, May 22, 2025 at 02:34:33PM +0200, Christian König wrote: > >> See all the functions inside include/linux/dma-fence.h can be used by > >> everybody. It's basically the public interface of the dma_fence object. > > > > As you write below, in certain cases it is valid to call this from drivers, > > so > > it's not unreasonable to have it as part of the public API. > > The question is from which drivers?
Well, any driver that uses it to check its own fences, as you say below. > >> So testing if a fence is signaled without calling the callback is only > >> allowed by whoever implemented the fence. > >> > >> In other words nouveau can test nouveau fences, i915 can test i915 fences, > >> amdgpu can test amdgpu fences etc... But if you have the wrapper that > >> makes it officially allowed that nouveau starts testing i915 fences and > >> that would be problematic. > > > > In general, I like the __dma_fence_is_signaled() helper, because this way > > we > > can document in which cases it is allowed to be used, i.e. the ones you > > descibe > > above. > > > > test_bit() can be called by anyone and there is no documentation comment > > explaining that it is only allowed under certain conditions. > > That's a rather good argument. > > > Having the __dma_fence_is_signaled() helper properly documented could get > > you > > rid of having to explain in which case the test_bit() dance is allowed to do > > over and over again. :-) > > That's an even better argument. > > > I also think the name is good, since the '__' prefix already implies that > > there > > are some restrictions on the use of this helper. > > I'm still hesitating. Adding something to the API always made it usable by > everybody. You can't prevent that, the test_bit() dance can be done by anyone, but you can document it properly with this helper. :-)