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. :-)

Reply via email to