[PATCH] drm: fix drm_read() returning 0

[Dave Airlie]

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