Imagine two threads read()'ing on the drm file and both are asleep waiting for events in drm_read(). If a single event occurs, both threads are woken up and start fetching the event. One thread will get it and return, the other thread will notice that there is no further event and return 0 to user-space.
We can avoid this by waiting for events until we got at least one event or an error occurred.
Signed-off-by: David Herrmann dh.herrmann@googlemail.com --- The patch might look a bit scary but it adds only a single do { } while(!total); around the whole block.
drivers/gpu/drm/drm_fops.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 123de28..6e7d349 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -635,25 +635,26 @@ ssize_t drm_read(struct file *filp, char __user *buffer, { struct drm_file *file_priv = filp->private_data; struct drm_pending_event *e; - size_t total; + size_t total = 0; 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)) { - if (copy_to_user(buffer + total, - e->event, e->event->length)) { - total = -EFAULT; - break; - } + do { + ret = wait_event_interruptible(file_priv->event_wait, + !list_empty(&file_priv->event_list)); + if (ret < 0) + return ret;
- total += e->event->length; - e->destroy(e); - } + while (drm_dequeue_event(file_priv, total, count, &e)) { + if (copy_to_user(buffer + total, + e->event, e->event->length)) { + total = -EFAULT; + break; + } + + total += e->event->length; + e->destroy(e); + } + } while (!total);
return total; }
On Fri, Jun 15, 2012 at 6:21 AM, David Herrmann dh.herrmann@googlemail.com wrote:
Imagine two threads read()'ing on the drm file and both are asleep waiting for events in drm_read(). If a single event occurs, both threads are woken up and start fetching the event. One thread will get it and return, the other thread will notice that there is no further event and return 0 to user-space.
We can avoid this by waiting for events until we got at least one event or an error occurred.
Anyone understand this code enough to review/ack this? krh gets explicit prodding.
logic look right to me, Dave.
Signed-off-by: David Herrmann dh.herrmann@googlemail.com
The patch might look a bit scary but it adds only a single do { } while(!total); around the whole block.
drivers/gpu/drm/drm_fops.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 123de28..6e7d349 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -635,25 +635,26 @@ ssize_t drm_read(struct file *filp, char __user *buffer, { struct drm_file *file_priv = filp->private_data; struct drm_pending_event *e;
size_t total;
size_t total = 0; 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)) {if (copy_to_user(buffer + total,e->event, e->event->length)) {total = -EFAULT;break;}
do {ret = wait_event_interruptible(file_priv->event_wait,!list_empty(&file_priv->event_list));if (ret < 0)return ret;
total += e->event->length;e->destroy(e);}
while (drm_dequeue_event(file_priv, total, count, &e)) {if (copy_to_user(buffer + total,e->event, e->event->length)) {total = -EFAULT;break;}total += e->event->length;e->destroy(e);}} while (!total); return total;}
1.7.10.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Dave Airlie -
David Herrmann