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 <ihadzic at research.bell-labs.com>
---
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.7
