At Thu, 4 Dec 2014 11:51:14 +0000,
Chris Wilson wrote:
>
> On Thu, Dec 04, 2014 at 11:56:42AM +0100, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > drivers/gpu/drm/drm_fops.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > index ed7bc68f7e87..a82dc28d54f3 100644
> > --- a/drivers/gpu/drm/drm_fops.c
> > +++ b/drivers/gpu/drm/drm_fops.c
> > @@ -525,6 +525,7 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
> > if (copy_to_user(buffer + total,
> > e->event, e->event->length)) {
> > total = -EFAULT;
> > + e->destroy(e);
>
> We shouldn't just be throwing away the event here, but put the event
> back at the front of the queue. Poses an interesting race issue. Seems
> like we want to hold the spinlock until the copy is complete so that we
> can fix up the failure correctly.
Yeah, I thought of it while writing this, but as a starter, I tried
the simpler one. (And I didn't realize your comment referring to the
already existing fix in kernel, sorry!)
The problem to hold the spinlock for the whole is that you can't it
with copy_to_user(). So it'd be a bit tricky.
And, why not using event_wait.lock instead of the extra event_lock?
Then we can use wait_event_lock_*() variant that covers more race
between dequeuing and wait_event.
Takashi
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
