[PATCH 01/15] drm/vblank: Use drm_event_reserve_init

* [PATCH 01/15] drm/vblank: Use drm_event_reserve_init
@ 2016-01-25 21:16 Daniel Vetter
  2016-01-25 21:16 ` [PATCH 02/15] drm: Clean up pending events in the core Daniel Vetter
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: Daniel Vetter @ 2016-01-25 21:16 UTC (permalink / raw)
  To: DRI Development, Intel Graphics Development
  Cc: Alex Deucher, Daniel Vetter, Daniel Stone, Daniel Vetter,
	Laurent Pinchart

Well we can't use that directly since that code must hold
dev->event_lock already. Extract an _unlocked version.

Embarrassingly I've totally forgotten about this patch and any kind of
event-based vblank wait totally blew up, killing the kernel.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fops.c | 64 +++++++++++++++++++++++++++++++++++-----------
 drivers/gpu/drm/drm_irq.c  | 10 +++-----
 include/drm/drmP.h         |  4 +++
 3 files changed, 56 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index e13501e3606e..eb6a02f78697 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -678,7 +678,7 @@ unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait)
 EXPORT_SYMBOL(drm_poll);
 
 /**
- * drm_event_reserve_init - init a DRM event and reserve space for it
+ * drm_event_reserve_init_locked - init a DRM event and reserve space for it
  * @dev: DRM device
  * @file_priv: DRM file private data
  * @p: tracking structure for the pending event
@@ -694,24 +694,20 @@ EXPORT_SYMBOL(drm_poll);
  * If callers embedded @p into a larger structure it must be allocated with
  * kmalloc and @p must be the first member element.
  *
+ * This is the locked version of drm_event_reserve_init() for callers which
+ * already hold dev->event_lock.
+ *
  * RETURNS:
  *
  * 0 on success or a negative error code on failure.
  */
-int drm_event_reserve_init(struct drm_device *dev,
-			   struct drm_file *file_priv,
-			   struct drm_pending_event *p,
-			   struct drm_event *e)
+int drm_event_reserve_init_locked(struct drm_device *dev,
+				  struct drm_file *file_priv,
+				  struct drm_pending_event *p,
+				  struct drm_event *e)
 {
-	unsigned long flags;
-	int ret = 0;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-
-	if (file_priv->event_space < e->length) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (file_priv->event_space < e->length)
+		return -ENOMEM;
 
 	file_priv->event_space -= e->length;
 
@@ -721,8 +717,46 @@ int drm_event_reserve_init(struct drm_device *dev,
 	/* we *could* pass this in as arg, but everyone uses kfree: */
 	p->destroy = (void (*) (struct drm_pending_event *)) kfree;
 
-out:
+	return 0;
+}
+EXPORT_SYMBOL(drm_event_reserve_init_locked);
+
+/**
+ * drm_event_reserve_init - init a DRM event and reserve space for it
+ * @dev: DRM device
+ * @file_priv: DRM file private data
+ * @p: tracking structure for the pending event
+ * @e: actual event data to deliver to userspace
+ *
+ * This function prepares the passed in event for eventual delivery. If the event
+ * doesn't get delivered (because the IOCTL fails later on, before queuing up
+ * anything) then the even must be cancelled and freed using
+ * drm_event_cancel_free(). Successfully initialized events should be sent out
+ * using drm_send_event() or drm_send_event_locked() to signal completion of the
+ * asynchronous event to userspace.
+ *
+ * If callers embedded @p into a larger structure it must be allocated with
+ * kmalloc and @p must be the first member element.
+ *
+ * Callers which already hold dev->event_lock should use
+ * drm_event_reserve_init() instead.
+ *
+ * RETURNS:
+ *
+ * 0 on success or a negative error code on failure.
+ */
+int drm_event_reserve_init(struct drm_device *dev,
+			   struct drm_file *file_priv,
+			   struct drm_pending_event *p,
+			   struct drm_event *e)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+	ret = drm_event_reserve_init_locked(dev, file_priv, p, e);
 	spin_unlock_irqrestore(&dev->event_lock, flags);
+
 	return ret;
 }
 EXPORT_SYMBOL(drm_event_reserve_init);
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 4ec8bca643ac..aa602199cb5d 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1598,9 +1598,6 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 	e->event.base.type = DRM_EVENT_VBLANK;
 	e->event.base.length = sizeof(e->event);
 	e->event.user_data = vblwait->request.signal;
-	e->base.event = &e->event.base;
-	e->base.file_priv = file_priv;
-	e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
@@ -1616,12 +1613,11 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 		goto err_unlock;
 	}
 
-	if (file_priv->event_space < sizeof(e->event)) {
-		ret = -EBUSY;
+	ret = drm_event_reserve_init_locked(dev, file_priv, &e->base, &e->event);
+
+	if (ret)
 		goto err_unlock;
-	}
 
-	file_priv->event_space -= sizeof(e->event);
 	seq = drm_vblank_count_and_time(dev, pipe, &now);
 
 	if ((vblwait->request.type & _DRM_VBLANK_NEXTONMISS) &&
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 306ef32ec086..1b71852d0a55 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -926,6 +926,10 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
 int drm_release(struct inode *inode, struct file *filp);
 int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv);
 unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
+int drm_event_reserve_init_locked(struct drm_device *dev,
+				  struct drm_file *file_priv,
+				  struct drm_pending_event *p,
+				  struct drm_event *e);
 int drm_event_reserve_init(struct drm_device *dev,
 			   struct drm_file *file_priv,
 			   struct drm_pending_event *p,
-- 
2.7.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread