Re: [Intel-gfx] [PATCH 3/3] drm/i915: Flush the workqueue before draining
Chris Wilson writes: > Quoting Mika Kuoppala (2019-07-04 11:22:17) >> Chris Wilson 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 > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Flush the workqueue before draining
Quoting Mika Kuoppala (2019-07-04 11:22:17) > Chris Wilson 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Flush the workqueue before draining
Chris Wilson 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
> ---
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Flush the workqueue before draining
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
---
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
