[2/2] drm: Implement nonblocking mode in drm_read()

Commit Message

Takashi Iwai Dec. 4, 2014, 10:56 a.m. UTC

drm_read() always blocks the read until an event comes up even if the
file is opened in nonblocking mode.  Also, there is a potential race
between the wake_event_interruptible() call and the actual event
retrieval in drm_dequeue_event(), and it may result in zero event read
even in the blocking mode.

This patch fixes these issues.  The fixes are two folds:

- Do drm_dequeue_event() at first, then call
  wait_event_interruptible() in a loop only if no event is pending.
  It's safe to call drm_dequeue_event() at first since it returns
  immediately if no event is found, and the function is protected
  properly by event_lock.

- Add a f_flags check when drm_dequeue_event() fails and returns
  immediately in nonblocking mode.  This is evaluated only at the
  first read.

Note: originally this issue comes up while debugging the hangup of X
at switching VT quickly.  For example, running the below on KDE
results in the stall of X:
  for i in $(seq 1 100); do chvt 1; chvt 7; done

This was reproduced on various Intel machines.  I took a stack trace
of the hanging process, and it points to the blockage in drm_read().

This patch "fixes" such a bug.  A drawback is that now it becomes more
difficult to find out such a buggy code in the caller side.

Signed-off-by: Takashi Iwai <[e-mail]>
---
 drivers/gpu/drm/drm_fops.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Patch

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index a82dc28d54f3..04833eaf51b3 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -515,13 +515,20 @@  ssize_t drm_read(struct file *filp, char __user *buffer,
 	size_t total;
 	ssize_t ret;
 
-	ret = wait_event_interruptible(file_priv->event_wait,
-				       !list_empty(&file_priv->event_list));
-	if (ret < 0)
-		return ret;
-
 	total = 0;
-	while (drm_dequeue_event(file_priv, total, count, &e)) {
+	for (;;) {
+		if (!drm_dequeue_event(file_priv, total, count, &e)) {
+			if (total)
+				break;
+			if (filp->f_flags & O_NONBLOCK)
+				return -EAGAIN;
+			ret = wait_event_interruptible(file_priv->event_wait,
+						       !list_empty(&file_priv->event_list));
+			if (ret < 0)
+				return ret;
+			continue;
+		}
+
 		if (copy_to_user(buffer + total,
 				 e->event, e->event->length)) {
 			total = -EFAULT;