On 20/11/2025 14:03, Christian König wrote:
On 11/18/25 17:03, Tvrtko Ursulin wrote:
@@ -448,13 +465,19 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+ const struct dma_fence_ops *ops;
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
- if (fence->ops->signaled && fence->ops->signaled(fence)) {
+ rcu_read_lock();
+ ops = rcu_dereference(fence->ops);
+ if (ops->signaled && ops->signaled(fence)) {
+ rcu_read_unlock();
With the unlocked version two threads could race and one could make the
fence->lock go away just around here, before the dma_fence_signal below will
take it. It seems it is only safe to rcu_read_unlock before signaling if using the
embedded fence (later in the series). Can you think of a downside to holding the
rcu read lock to after signaling? that would make it safe I think.
Well it's good to talk about it but I think that it is not necessary to protect
the lock in this particular case.
See the RCU protection is only for the fence->ops pointer, but the lock can be
taken way after the fence is already signaled.
That's why I came up with the patch to move the lock into the fence in the
first place.
Right. And you think there is nothing to gain with the option of keeping the
rcu_read_unlock() to after signalling? Ie. why not plug a potential race if we
can for no negative effect.
I thought quite a bit over that, but at least of hand I can't come up with a
reason why we should do this. The signaling path doesn't need the RCU read side
lock as far as I can see.
Okay, and thanks, it indeed sounds right for the signalling path. Any
races hitting this would mean a more serious problem in the driver
implementation.
Regards,
Tvrtko
