drm: Fix deadlock between event_lock and vbl_lock/vblank_time_lock
Currently both drm_irq.c and several drivers call drm_vblank_put()
while holding event_lock. Now that drm_vblank_put() can disable the
vblank interrupt directly it may need to grab vbl_lock and
vblank_time_lock. That causes deadlocks since we take the locks
in the opposite order in two places in drm_irq.c. So let's make
sure the locking order is always event_lock->vbl_lock->vblank_time_lock.
In drm_vblank_off() pull up event_lock from underneath vbl_lock. Hold
the event_lock across the whole operation to make sure we only send
out the events that were on the queue when we disabled the interrupt,
and not ones that got added just after (assuming drm_vblank_on() already
managed to get called somewhere between).
To sort the other deadlock pull the event_lock out from
drm_handle_vblank_events() into drm_handle_vblank() to be taken outside
vblank_time_lock. Add the appropriate assert_spin_locked() to
drm_handle_vblank_events().
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b4460bf..9353609 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1037,14 +1037,25 @@
unsigned long irqflags;
unsigned int seq;
- spin_lock_irqsave(&dev->vbl_lock, irqflags);
+ spin_lock_irqsave(&dev->event_lock, irqflags);
+
+ spin_lock(&dev->vbl_lock);
vblank_disable_and_save(dev, crtc);
wake_up(&vblank->queue);
+ /*
+ * Prevent subsequent drm_vblank_get() from re-enabling
+ * the vblank interrupt by bumping the refcount.
+ */
+ if (!vblank->inmodeset) {
+ atomic_inc(&vblank->refcount);
+ vblank->inmodeset = 1;
+ }
+ spin_unlock(&dev->vbl_lock);
+
/* Send any queued vblank events, lest the natives grow disquiet */
seq = drm_vblank_count_and_time(dev, crtc, &now);
- spin_lock(&dev->event_lock);
list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
if (e->pipe != crtc)
continue;
@@ -1055,18 +1066,7 @@
drm_vblank_put(dev, e->pipe);
send_vblank_event(dev, e, seq, &now);
}
- spin_unlock(&dev->event_lock);
-
- /*
- * Prevent subsequent drm_vblank_get() from re-enabling
- * the vblank interrupt by bumping the refcount.
- */
- if (!vblank->inmodeset) {
- atomic_inc(&vblank->refcount);
- vblank->inmodeset = 1;
- }
-
- spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+ spin_unlock_irqrestore(&dev->event_lock, irqflags);
}
EXPORT_SYMBOL(drm_vblank_off);
@@ -1446,12 +1446,11 @@
{
struct drm_pending_vblank_event *e, *t;
struct timeval now;
- unsigned long flags;
unsigned int seq;
- seq = drm_vblank_count_and_time(dev, crtc, &now);
+ assert_spin_locked(&dev->event_lock);
- spin_lock_irqsave(&dev->event_lock, flags);
+ seq = drm_vblank_count_and_time(dev, crtc, &now);
list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
if (e->pipe != crtc)
@@ -1467,8 +1466,6 @@
send_vblank_event(dev, e, seq, &now);
}
- spin_unlock_irqrestore(&dev->event_lock, flags);
-
trace_drm_vblank_event(crtc, seq);
}
@@ -1491,15 +1488,18 @@
if (!dev->num_crtcs)
return false;
+ spin_lock_irqsave(&dev->event_lock, irqflags);
+
/* Need timestamp lock to prevent concurrent execution with
* vblank enable/disable, as this would cause inconsistent
* or corrupted timestamps and vblank counts.
*/
- spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
+ spin_lock(&dev->vblank_time_lock);
/* Vblank irq handling disabled. Nothing to do. */
if (!vblank->enabled) {
- spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
+ spin_unlock(&dev->vblank_time_lock);
+ spin_unlock_irqrestore(&dev->event_lock, irqflags);
return false;
}
@@ -1539,10 +1539,13 @@
crtc, (int) diff_ns);
}
+ spin_unlock(&dev->vblank_time_lock);
+
wake_up(&vblank->queue);
drm_handle_vblank_events(dev, crtc);
- spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
+ spin_unlock_irqrestore(&dev->event_lock, irqflags);
+
return true;
}
EXPORT_SYMBOL(drm_handle_vblank);