| Message ID | 20200402080805.20292-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Add one more rcu_barrier() after draining the ppGTT freelist | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Since gen7 full-ppgtt uses nested vm references (the ppGTT is a slot > within the global GTT and so has a buried i915_ggtt) we may need to go > through a couple of RCU barriers before we have freed all the structs. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gt/intel_ggtt.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c > index 894d08e5a21f..cb31ed712db3 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > @@ -684,8 +684,9 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt) > > atomic_set(&ggtt->vm.open, 0); > > - rcu_barrier(); /* flush the RCU'ed__i915_vm_release */ > - flush_workqueue(ggtt->vm.i915->wq); > + do { /* flush the RCU'ed__i915_vm_release */ > + rcu_barrier(); > + } while (flush_workqueue(ggtt->vm.i915->wq)); flush_workqueue returns void. you want flush_work? -Mika > > mutex_lock(&ggtt->vm.mutex); > > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2020-04-02 09:44:33) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Since gen7 full-ppgtt uses nested vm references (the ppGTT is a slot > > within the global GTT and so has a buried i915_ggtt) we may need to go > > through a couple of RCU barriers before we have freed all the structs. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c > > index 894d08e5a21f..cb31ed712db3 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > > @@ -684,8 +684,9 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt) > > > > atomic_set(&ggtt->vm.open, 0); > > > > - rcu_barrier(); /* flush the RCU'ed__i915_vm_release */ > > - flush_workqueue(ggtt->vm.i915->wq); > > + do { /* flush the RCU'ed__i915_vm_release */ > > + rcu_barrier(); > > + } while (flush_workqueue(ggtt->vm.i915->wq)); > > flush_workqueue returns void. you want flush_work? Darnation. I see in i915_gem_drain_workqueue() we just did N passes :( -Chris
Quoting Chris Wilson (2020-04-02 09:48:18) > Quoting Mika Kuoppala (2020-04-02 09:44:33) > > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > > > Since gen7 full-ppgtt uses nested vm references (the ppGTT is a slot > > > within the global GTT and so has a buried i915_ggtt) we may need to go > > > through a couple of RCU barriers before we have freed all the structs. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > --- > > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c > > > index 894d08e5a21f..cb31ed712db3 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > > > @@ -684,8 +684,9 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt) > > > > > > atomic_set(&ggtt->vm.open, 0); > > > > > > - rcu_barrier(); /* flush the RCU'ed__i915_vm_release */ > > > - flush_workqueue(ggtt->vm.i915->wq); > > > + do { /* flush the RCU'ed__i915_vm_release */ > > > + rcu_barrier(); > > > + } while (flush_workqueue(ggtt->vm.i915->wq)); > > > > flush_workqueue returns void. you want flush_work? > > Darnation. I see in i915_gem_drain_workqueue() we just did N passes :( Waitasec this is i915->wq, we can just use i915_gem_drain_workqueue() (although that's now a misnomer). -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 894d08e5a21f..cb31ed712db3 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -684,8 +684,9 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt) atomic_set(&ggtt->vm.open, 0); - rcu_barrier(); /* flush the RCU'ed__i915_vm_release */ - flush_workqueue(ggtt->vm.i915->wq); + do { /* flush the RCU'ed__i915_vm_release */ + rcu_barrier(); + } while (flush_workqueue(ggtt->vm.i915->wq)); mutex_lock(&ggtt->vm.mutex);
Since gen7 full-ppgtt uses nested vm references (the ppGTT is a slot within the global GTT and so has a buried i915_ggtt) we may need to go through a couple of RCU barriers before we have freed all the structs. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)