From c16cf5b55ccf20d9b3d57f97181b2e492ae6117e Mon Sep 17 00:00:00 2001 From: Ilija Hadzic Date: Sat, 22 Oct 2011 00:04:29 -0400 Subject: [PATCH] drm: do not sleep on vblank while holding a mutex holding the drm_global_mutex in drm_wait_vblank and then going to sleep (via DRM_WAIT_ON) is a bad idea in general because it can block other processes that may issue ioctls that also grab drm_global_mutex. Blocking can occur until next vblank which is in the tens of microseconds order of magnitude. fix it, but making drm_wait_vblank DRM_UNLOCKED call and then grabbing the mutex inside the drm_wait_vblank function and only for the duration of the critical section (i.e., from first manipulation of vblank sequence number until putting the task in the wait queue). Signed-off-by: Ilija Hadzic --- drivers/gpu/drm/drm_drv.c | 2 +- drivers/gpu/drm/drm_irq.c | 16 +++++++++++----- include/drm/drm_os_linux.h | 25 +++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index dbabcb0..dc0eb0b 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), - DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0), + DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0), diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 3830e9e..d85d604 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1191,6 +1191,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data, DRM_DEBUG("failed to acquire vblank counter, %d\n", ret); return ret; } + mutex_lock(&drm_global_mutex); seq = drm_vblank_count(dev, crtc); switch (vblwait->request.type & _DRM_VBLANK_TYPES_MASK) { @@ -1200,12 +1201,15 @@ int drm_wait_vblank(struct drm_device *dev, void *data, case _DRM_VBLANK_ABSOLUTE: break; default: + mutex_unlock(&drm_global_mutex); ret = -EINVAL; goto done; } - if (flags & _DRM_VBLANK_EVENT) + if (flags & _DRM_VBLANK_EVENT) { + mutex_unlock(&drm_global_mutex); return drm_queue_vblank_event(dev, crtc, vblwait, file_priv); + } if ((flags & _DRM_VBLANK_NEXTONMISS) && (seq - vblwait->request.sequence) <= (1<<23)) { @@ -1215,10 +1219,12 @@ int drm_wait_vblank(struct drm_device *dev, void *data, DRM_DEBUG("waiting on vblank count %d, crtc %d\n", vblwait->request.sequence, crtc); dev->last_vblank_wait[crtc] = vblwait->request.sequence; - DRM_WAIT_ON(ret, dev->vbl_queue[crtc], 3 * DRM_HZ, - (((drm_vblank_count(dev, crtc) - - vblwait->request.sequence) <= (1 << 23)) || - !dev->irq_enabled)); + /* DRM_WAIT_ON_LOCKED will release drm_global_mutex */ + /* before going to sleep */ + DRM_WAIT_ON_LOCKED(ret, dev->vbl_queue[crtc], 3 * DRM_HZ, + (((drm_vblank_count(dev, crtc) - + vblwait->request.sequence) <= (1 << 23)) || + !dev->irq_enabled)); if (ret != -EINTR) { struct timeval now; diff --git a/include/drm/drm_os_linux.h b/include/drm/drm_os_linux.h index 3933691..fc6aaf4 100644 --- a/include/drm/drm_os_linux.h +++ b/include/drm/drm_os_linux.h @@ -123,5 +123,30 @@ do { \ remove_wait_queue(&(queue), &entry); \ } while (0) +#define DRM_WAIT_ON_LOCKED( ret, queue, timeout, condition ) \ +do { \ + DECLARE_WAITQUEUE(entry, current); \ + unsigned long end = jiffies + (timeout); \ + add_wait_queue(&(queue), &entry); \ + mutex_unlock(&drm_global_mutex); \ + \ + for (;;) { \ + __set_current_state(TASK_INTERRUPTIBLE); \ + if (condition) \ + break; \ + if (time_after_eq(jiffies, end)) { \ + ret = -EBUSY; \ + break; \ + } \ + schedule_timeout((HZ/100 > 1) ? HZ/100 : 1); \ + if (signal_pending(current)) { \ + ret = -EINTR; \ + break; \ + } \ + } \ + __set_current_state(TASK_RUNNING); \ + remove_wait_queue(&(queue), &entry); \ +} while (0) + #define DRM_WAKEUP( queue ) wake_up( queue ) #define DRM_INIT_WAITQUEUE( queue ) init_waitqueue_head( queue ) -- 1.7.4.1