Otherwise drmHandleEvent will block if accidentally read too often...
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Phillip Haddad phillip.haddad@gmail.com --- drivers/gpu/drm/drm_fops.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 2ec7d48..279aa95 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -617,6 +617,9 @@ ssize_t drm_read(struct file *filp, char __user *buffer, size_t total; ssize_t ret;
+ if (filp->f_flags & O_NONBLOCK && list_empty(&file_priv->event_list)) + return -EAGAIN; + ret = wait_event_interruptible(file_priv->event_wait, !list_empty(&file_priv->event_list)); if (ret < 0)
On Wed, 8 Jun 2011 20:14:01 +0100 Chris Wilson chris@chris-wilson.co.uk wrote:
Otherwise drmHandleEvent will block if accidentally read too often...
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Phillip Haddad phillip.haddad@gmail.com
drivers/gpu/drm/drm_fops.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 2ec7d48..279aa95 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -617,6 +617,9 @@ ssize_t drm_read(struct file *filp, char __user *buffer, size_t total; ssize_t ret;
- if (filp->f_flags & O_NONBLOCK && list_empty(&file_priv->event_list))
return -EAGAIN;
- ret = wait_event_interruptible(file_priv->event_wait, !list_empty(&file_priv->event_list)); if (ret < 0)
What happens if someone else empties the list between the test and the wait_event_interruptible ?
Otherwise drmHandleEvent will block if accidentally read too often. In order to handle the case where the event is read off the queue by another thread before we dequeue the event, we delay the actual checking for EAGAIN to under the spinlock and so inline drm_dequeue_event().
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- drivers/gpu/drm/drm_fops.c | 76 +++++++++++++++++++++----------------------- 1 files changed, 36 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 2ec7d48..d248366 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -580,61 +580,57 @@ int drm_release(struct inode *inode, struct file *filp) } EXPORT_SYMBOL(drm_release);
-static bool -drm_dequeue_event(struct drm_file *file_priv, - size_t total, size_t max, struct drm_pending_event **out) +ssize_t drm_read(struct file *filp, char __user *buffer, + size_t count, loff_t *offset) { + struct drm_file *file_priv = filp->private_data; struct drm_device *dev = file_priv->minor->dev; struct drm_pending_event *e; unsigned long flags; - bool ret = false; - - spin_lock_irqsave(&dev->event_lock, flags); + ssize_t ret; + int err;
- *out = NULL; - if (list_empty(&file_priv->event_list)) - goto out; - e = list_first_entry(&file_priv->event_list, - struct drm_pending_event, link); - if (e->event->length + total > max) - goto out; + if ((filp->f_flags & O_NONBLOCK) == 0) { + ret = wait_event_interruptible(file_priv->event_wait, + !list_empty(&file_priv->event_list)); + if (ret < 0) + return ret; + }
- file_priv->event_space += e->event->length; - list_del(&e->link); - *out = e; - ret = true; + ret = err = 0; + spin_lock_irqsave(&dev->event_lock, flags); + do { + if (list_empty(&file_priv->event_list)) { + if (flip->f_flags & O_NONBLOCK) + err = -EAGAIN; + break; + }
-out: - spin_unlock_irqrestore(&dev->event_lock, flags); - return ret; -} + e = list_first_entry(&file_priv->event_list, + struct drm_pending_event, link); + if (e->event->length + ret > max) { + err = -EINVAL; + break; + }
-ssize_t drm_read(struct file *filp, char __user *buffer, - size_t count, loff_t *offset) -{ - struct drm_file *file_priv = filp->private_data; - struct drm_pending_event *e; - size_t total; - ssize_t ret; + file_priv->event_space += e->event->length; + list_del(&e->link);
- ret = wait_event_interruptible(file_priv->event_wait, - !list_empty(&file_priv->event_list)); - if (ret < 0) - return ret; + spin_unlock_irqrestore(&dev->event_lock, flags);
- total = 0; - while (drm_dequeue_event(file_priv, total, count, &e)) { - if (copy_to_user(buffer + total, - e->event, e->event->length)) { - total = -EFAULT; + if (copy_to_user(buffer + ret, e->event, e->event->length)) { + err = -EFAULT; break; }
- total += e->event->length; + ret += e->event->length; e->destroy(e); - }
- return total; + spin_lock_irqsave(&dev->event_lock, flags); + } while (1); + spin_unlock_irqrestore(&dev->event_lock, flags); + + return ret ? ret : err; } EXPORT_SYMBOL(drm_read);
Otherwise drmHandleEvent will block if accidentally read too often. In order to handle the case where the event is read off the queue by another thread before we dequeue the event, we delay the actual checking for EAGAIN to under the spinlock and so inline drm_dequeue_event().
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- v3: Compilation. Remember to at least test changes before hitting send. -Chris --- drivers/gpu/drm/drm_fops.c | 79 +++++++++++++++++++++----------------------- 1 files changed, 38 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 2ec7d48..8efc07f 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -580,61 +580,58 @@ int drm_release(struct inode *inode, struct file *filp) } EXPORT_SYMBOL(drm_release);
-static bool -drm_dequeue_event(struct drm_file *file_priv, - size_t total, size_t max, struct drm_pending_event **out) +ssize_t drm_read(struct file *filp, char __user *buffer, + size_t count, loff_t *offset) { + struct drm_file *file_priv = filp->private_data; struct drm_device *dev = file_priv->minor->dev; struct drm_pending_event *e; unsigned long flags; - bool ret = false; + ssize_t ret; + int err;
- spin_lock_irqsave(&dev->event_lock, flags); + if ((filp->f_flags & O_NONBLOCK) == 0) { + ret = wait_event_interruptible(file_priv->event_wait, + !list_empty(&file_priv->event_list)); + if (ret < 0) + return ret; + }
- *out = NULL; - if (list_empty(&file_priv->event_list)) - goto out; - e = list_first_entry(&file_priv->event_list, - struct drm_pending_event, link); - if (e->event->length + total > max) - goto out; + ret = err = 0; + spin_lock_irqsave(&dev->event_lock, flags); + do { + if (list_empty(&file_priv->event_list)) { + if (filp->f_flags & O_NONBLOCK) + err = -EAGAIN; + goto unlock; + }
- file_priv->event_space += e->event->length; - list_del(&e->link); - *out = e; - ret = true; + e = list_first_entry(&file_priv->event_list, + struct drm_pending_event, link); + if (e->event->length + ret > count) { + err = -EINVAL; + goto unlock; + }
-out: - spin_unlock_irqrestore(&dev->event_lock, flags); - return ret; -} + file_priv->event_space += e->event->length; + list_del(&e->link);
-ssize_t drm_read(struct file *filp, char __user *buffer, - size_t count, loff_t *offset) -{ - struct drm_file *file_priv = filp->private_data; - struct drm_pending_event *e; - size_t total; - ssize_t ret; + spin_unlock_irqrestore(&dev->event_lock, flags);
- 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)) { - if (copy_to_user(buffer + total, - e->event, e->event->length)) { - total = -EFAULT; - break; + if (copy_to_user(buffer + ret, e->event, e->event->length)) { + err = -EFAULT; + goto out; }
- total += e->event->length; + ret += e->event->length; e->destroy(e); - }
- return total; + spin_lock_irqsave(&dev->event_lock, flags); + } while (1); +unlock: + spin_unlock_irqrestore(&dev->event_lock, flags); +out: + return ret ? ret : err; } EXPORT_SYMBOL(drm_read);
dri-devel@lists.freedesktop.org