Am 04.05.22 um 09:53 schrieb Tvrtko Ursulin:
On 02/05/2022 17:37, Christian König wrote:
[SNIP] @@ -398,6 +408,8 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence); static inline bool dma_fence_is_signaled_locked(struct dma_fence *fence) { + WARN_ON(test_bit(DMA_FENCE_FLAG_USER, &fence->flags));
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return true; @@ -428,6 +440,9 @@ dma_fence_is_signaled_locked(struct dma_fence *fence) static inline bool dma_fence_is_signaled(struct dma_fence *fence) { + if (test_bit(DMA_FENCE_FLAG_USER, &fence->flags)) + might_fault();
Why this one can fault and the dma_fence_signal_locked cannot? I mean obviosuly it must not in the locked section, but how do signalling callbacks know the context of the caller?
Well, dma_fence_is_signaled_locked() shouldn't exists in the first place.
The locking can only be done by either the framework itself or the driver exporting the fence. And if you take at the actual users than you indeed see that the only two cases where this is used the driver knows what fence it has and should probably call it's internal function itself.
This maybe ties back to the issue I don't think I ever found an explanation to - why "is signal" helpers actually need (or want?) to do the signalling itself, and are therefore hit also with the signalling annotations?
Yeah, exactly that's what I could never figure out as well. And I think that it isn't good design to do this, because it results in the dma_fence being signaled from different sources.
Without it it would just be consistently signaled from the interrupt (or whatever context) the exporter uses to signal it.
Thanks, Christian.
Regards,
Tvrtko
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return true; @@ -583,7 +598,7 @@ static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr) return ret < 0 ? ret : 0; } -struct dma_fence *dma_fence_get_stub(void); +struct dma_fence *dma_fence_get_stub(bool user); struct dma_fence *dma_fence_allocate_private_stub(void); u64 dma_fence_context_alloc(unsigned num);