| Message ID | 20190703171913.16585-4-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915/gem: Defer obj->base.resv fini until RCU callback | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Trying to drain a workqueue while we may still be adding to it from > background tasks is, according to kernel/workqueue.c, verboten. So, add > a flush_workqueue() at the start of our cleanup procedure. I don't get it. drain_workqueue does it's own flushing. -Mika > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 9d132c9d17b0..d2f9af3a16dc 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2472,6 +2472,7 @@ static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915) > */ > int pass = 3; > do { > + flush_workqueue(i915->wq); > rcu_barrier(); > i915_gem_drain_freed_objects(i915); > } while (--pass); > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2019-07-04 11:22:17) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Trying to drain a workqueue while we may still be adding to it from > > background tasks is, according to kernel/workqueue.c, verboten. So, add > > a flush_workqueue() at the start of our cleanup procedure. > > I don't get it. drain_workqueue does it's own flushing. Ordering is important here. The problem with drain_workqueue() is that is forbids us from adding more tasks into the workqueue as it drains, so before we drain we must plug. It's just adding more hammers. Eventually it'll break. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2019-07-04 11:22:17) >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > Trying to drain a workqueue while we may still be adding to it from >> > background tasks is, according to kernel/workqueue.c, verboten. So, add >> > a flush_workqueue() at the start of our cleanup procedure. >> >> I don't get it. drain_workqueue does it's own flushing. > > Ordering is important here. The problem with drain_workqueue() is that > is forbids us from adding more tasks into the workqueue as it drains, so > before we drain we must plug. > > It's just adding more hammers. Eventually it'll break. If so, then we just increase passes? :) Ok, I was going to say we don't add to free work from any of it's handlers. But then realized this is not about the free list handling, even tho the freed objects is drained along. And yes, drain only handles flushing for it's chain. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9d132c9d17b0..d2f9af3a16dc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2472,6 +2472,7 @@ static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915) */ int pass = 3; do { + flush_workqueue(i915->wq); rcu_barrier(); i915_gem_drain_freed_objects(i915); } while (--pass);
Trying to drain a workqueue while we may still be adding to it from background tasks is, according to kernel/workqueue.c, verboten. So, add a flush_workqueue() at the start of our cleanup procedure. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.h | 1 + 1 file changed, 1 insertion(+)