| Message ID | 20180904131207.17563-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Pull intel_uncore_arm_unclaimed_mmio_detection() under the spinlock | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Elsewhere we manipulate uncore.unclaimed_mmio_check and > i915_param.mmio_debug under the irq lock (e.g. preserving the current > value across a user forcewake grab), but do not protect the manipulation > inside intel_uncore_arm_unclaimed_mmio_detection() from concurrent > access, even from itself. This is an issue as we do call > arm_unclaimed_mmio_detection from multiple threads without coordination. > > Suggested-by: Mika Kuoppala <mika.kuoppala@intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/intel_uncore.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 05f0cda18501..3ad302c66254 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -2283,8 +2283,12 @@ bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv) > bool > intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv) > { > + bool ret = false; > + > + spin_lock_irq(&dev_priv->uncore.lock); > + > if (unlikely(dev_priv->uncore.unclaimed_mmio_check <= 0)) > - return false; > + goto out; > > if (unlikely(intel_uncore_unclaimed_mmio(dev_priv))) { > if (!i915_modparams.mmio_debug) { > @@ -2294,10 +2298,13 @@ intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv) > i915_modparams.mmio_debug++; > } > dev_priv->uncore.unclaimed_mmio_check--; > - return true; > + ret = true; > } > > - return false; > +out: > + spin_unlock_irq(&dev_priv->uncore.lock); > + > + return ret; Patchbot didn't see how the future will unfold. I did. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intelcom> > } > > static enum forcewake_domains > -- > 2.19.0.rc1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 05f0cda18501..3ad302c66254 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -2283,8 +2283,12 @@ bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv) bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv) { + bool ret = false; + + spin_lock_irq(&dev_priv->uncore.lock); + if (unlikely(dev_priv->uncore.unclaimed_mmio_check <= 0)) - return false; + goto out; if (unlikely(intel_uncore_unclaimed_mmio(dev_priv))) { if (!i915_modparams.mmio_debug) { @@ -2294,10 +2298,13 @@ intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv) i915_modparams.mmio_debug++; } dev_priv->uncore.unclaimed_mmio_check--; - return true; + ret = true; } - return false; +out: + spin_unlock_irq(&dev_priv->uncore.lock); + + return ret; } static enum forcewake_domains
Elsewhere we manipulate uncore.unclaimed_mmio_check and i915_param.mmio_debug under the irq lock (e.g. preserving the current value across a user forcewake grab), but do not protect the manipulation inside intel_uncore_arm_unclaimed_mmio_detection() from concurrent access, even from itself. This is an issue as we do call arm_unclaimed_mmio_detection from multiple threads without coordination. Suggested-by: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)