[Intel-gfx] [PATCH 4/9] drm/i915: rework dev->first_error locking

[Daniel Vetter]

- reduce the irq disabled section, even for a debugfs file this was
  way too long.
- protect readers of the captured error state from concurrent freeing
  of the same by holding dev->struct_mutex.
- always disable irqs when taking the lock.

Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |    9 ++++++---
 drivers/gpu/drm/i915/i915_dma.c     |    2 ++
 drivers/gpu/drm/i915/i915_drv.h     |    3 +++
 drivers/gpu/drm/i915/i915_irq.c     |    7 +++++--
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e1c5aa1..63e1c36 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -768,13 +768,16 @@ static int i915_error_state(struct seq_file *m, void *unused)
 	unsigned long flags;
 	int i, page, offset, elt;
 
+	mutex_lock(&dev->struct_mutex);
 	spin_lock_irqsave(&dev_priv->error_lock, flags);
-	if (!dev_priv->first_error) {
+	error = dev_priv->first_error;
+	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+
+	if (!error) {
 		seq_printf(m, "no error state collected\n");
 		goto out;
 	}
 
-	error = dev_priv->first_error;
 
 	seq_printf(m, "Time: %ld s %ld us\n", error->time.tv_sec,
 		   error->time.tv_usec);
@@ -846,7 +849,7 @@ static int i915_error_state(struct seq_file *m, void *unused)
 		intel_display_print_error_state(m, dev, error->display);
 
 out:
-	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+	mutex_unlock(&dev->struct_mutex);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ab3a3fd..0034c6a 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2153,7 +2153,9 @@ int i915_driver_unload(struct drm_device *dev)
 	/* Free error state after interrupts are fully disabled. */
 	del_timer_sync(&dev_priv->hangcheck_timer);
 	cancel_work_sync(&dev_priv->error_work);
+	mutex_lock(&dev->struct_mutex);
 	i915_destroy_error_state(dev);
+	mutex_unlock(&dev->struct_mutex);
 
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8bb83c0..660b62c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -386,6 +386,9 @@ typedef struct drm_i915_private {
 	unsigned int fsb_freq, mem_freq, is_ddr3;
 
 	spinlock_t error_lock;
+	/* Protected by dev->error_lock. To ensure that the error_state obtained
+	 * through this pointer doesn't disappear, you also need to hold
+	 * dev->struct_mutex */
 	struct drm_i915_error_state *first_error;
 	struct work_struct error_work;
 	struct completion error_completion;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a04d606..c9b0766 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1024,11 +1024,14 @@ void i915_destroy_error_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_error_state *error;
+	unsigned long flags;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
 
-	spin_lock(&dev_priv->error_lock);
+	spin_lock_irqsave(&dev_priv->error_lock, flags);
 	error = dev_priv->first_error;
 	dev_priv->first_error = NULL;
-	spin_unlock(&dev_priv->error_lock);
+	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
 
 	if (error)
 		i915_error_state_free(dev, error);
-- 
1.7.6.4