| Message ID | 20200227085723.1961649-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/20] drm/i915: Skip barriers inside waits | expand |
Hi Cris, > Attaching to the i915_active barrier is a two stage process, and a flush > is only effective when the barrier is activation. Thus it is possible > for us to see a barrier, and attempt to flush, only for our flush to > have no effect. As such, before attempting to activate signaling on the > fence we need to double check it is a fence! > > Fixes: d13a31770077 ("drm/i915: Flush idle barriers when waiting") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Andi
On 27/02/2020 08:57, Chris Wilson wrote: > Attaching to the i915_active barrier is a two stage process, and a flush > is only effective when the barrier is activation. Thus it is possible > for us to see a barrier, and attempt to flush, only for our flush to > have no effect. As such, before attempting to activate signaling on the > fence we need to double check it is a fence! > > Fixes: d13a31770077 ("drm/i915: Flush idle barriers when waiting") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_active.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c > index 0b12d5023800..7b3d6c12ad61 100644 > --- a/drivers/gpu/drm/i915/i915_active.c > +++ b/drivers/gpu/drm/i915/i915_active.c > @@ -453,6 +453,9 @@ static void enable_signaling(struct i915_active_fence *active) > { > struct dma_fence *fence; > > + if (unlikely(is_barrier(active))) > + return; > + > fence = i915_active_fence_get(active); > if (!fence) > return; > So that smp_rmb() is not really effective, I mean the race is wider than that. I was worried about that.. now I need to figure out where it starts and where it ends (the race). Regards, Tvrtko
Quoting Tvrtko Ursulin (2020-02-28 11:53:19) > > On 27/02/2020 08:57, Chris Wilson wrote: > > Attaching to the i915_active barrier is a two stage process, and a flush > > is only effective when the barrier is activation. Thus it is possible > > for us to see a barrier, and attempt to flush, only for our flush to > > have no effect. As such, before attempting to activate signaling on the > > fence we need to double check it is a fence! > > > > Fixes: d13a31770077 ("drm/i915: Flush idle barriers when waiting") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > drivers/gpu/drm/i915/i915_active.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c > > index 0b12d5023800..7b3d6c12ad61 100644 > > --- a/drivers/gpu/drm/i915/i915_active.c > > +++ b/drivers/gpu/drm/i915/i915_active.c > > @@ -453,6 +453,9 @@ static void enable_signaling(struct i915_active_fence *active) > > { > > struct dma_fence *fence; > > > > + if (unlikely(is_barrier(active))) > > + return; > > + > > fence = i915_active_fence_get(active); > > if (!fence) > > return; > > > > So that smp_rmb() is not really effective, I mean the race is wider than > that. I was worried about that.. now I need to figure out where it > starts and where it ends (the race). That smp_rmb matches the update when we actually replace the barrier with the fence. But, yes, the reuse_idle_barrier() has interesting implications. There's no use after free and the iterator is technically safe, but the flush isn't quite the flush I had hoped for. -Chris
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index 0b12d5023800..7b3d6c12ad61 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -453,6 +453,9 @@ static void enable_signaling(struct i915_active_fence *active) { struct dma_fence *fence; + if (unlikely(is_barrier(active))) + return; + fence = i915_active_fence_get(active); if (!fence) return;
Attaching to the i915_active barrier is a two stage process, and a flush is only effective when the barrier is activation. Thus it is possible for us to see a barrier, and attempt to flush, only for our flush to have no effect. As such, before attempting to activate signaling on the fence we need to double check it is a fence! Fixes: d13a31770077 ("drm/i915: Flush idle barriers when waiting") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_active.c | 3 +++ 1 file changed, 3 insertions(+)