On Wed, Aug 06, 2014 at 03:23:01PM +0200, Daniel Vetter wrote:
On Wed, Aug 06, 2014 at 02:49:52PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Currently it's possible that the following will happen:
- drm_wait_vblank() calls drm_vblank_get()
- drm_vblank_off() gets called
- drm_wait_vblank() calls drm_queue_vblank_event() which adds the event to the queue event though vblank interrupts are currently disabled (and may not be re-enabled ever again).
To fix the problem, add another vblank->enabled check into drm_queue_vblank_event().
drm_vblank_off() holds event_lock around the vblank disable, so no further locking needs to be added to drm_queue_vblank_event(). vblank disable from another source is not possible since drm_wait_vblank() already holds a vblank reference.
Reviewed-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
I guess the window is too small here to make this reproducible in a test?
I must admit that I didn't even try. I supposed I could try it now.
Especially since each attempt will take a few hundred ms ... -Daniel
drivers/gpu/drm/drm_irq.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 9353609..b2428cb 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1270,6 +1270,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe, union drm_wait_vblank *vblwait, struct drm_file *file_priv) {
- struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; struct drm_pending_vblank_event *e; struct timeval now; unsigned long flags;
@@ -1293,6 +1294,18 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
spin_lock_irqsave(&dev->event_lock, flags);
- /*
* drm_vblank_off() might have been called after we called* drm_vblank_get(). drm_vblank_off() holds event_lock* around the vblank disable, so no need for further locking.* The reference from drm_vblank_get() protects against* vblank disable from another source.*/- if (!vblank->enabled) {
ret = -EINVAL;goto err_unlock;- }
- if (file_priv->event_space < sizeof e->event) { ret = -EBUSY; goto err_unlock;
-- 1.8.5.5
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch