On Fri, Jun 15, 2012 at 6:21 AM, David Herrmann
<dh.herrmann@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
