| Message ID | 20170804082328.17173-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 04, 2017 at 09:23:28AM +0100, Chris Wilson wrote: > After an event is sent, we try to copy it into the user buffer of the > first waiter in drm_read() and if the user buffer doesn't have enough > room we put it back onto the list. However, we didn't wake up any > subsequent waiter, so that event may sit on the list until either a new > vblank event is sent or a new waiter appears. Rare, but in the worst > case may lead to a stuck process. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> New subtestcase in igt@drm_read? -Daniel > --- > drivers/gpu/drm/drm_file.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 59b75a974357..7e6db9f7a058 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -524,6 +524,7 @@ ssize_t drm_read(struct file *filp, char __user *buffer, > file_priv->event_space -= length; > list_add(&e->link, &file_priv->event_list); > spin_unlock_irq(&dev->event_lock); > + wake_up_interruptible(&file_priv->event_wait); > break; > } > > -- > 2.13.3 >
Quoting Daniel Vetter (2017-08-07 10:28:58) > On Fri, Aug 04, 2017 at 09:23:28AM +0100, Chris Wilson wrote: > > After an event is sent, we try to copy it into the user buffer of the > > first waiter in drm_read() and if the user buffer doesn't have enough > > room we put it back onto the list. However, we didn't wake up any > > subsequent waiter, so that event may sit on the list until either a new > > vblank event is sent or a new waiter appears. Rare, but in the worst > > case may lead to a stuck process. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > New subtestcase in igt@drm_read? Caught it! Testcase: igt/drm_read/short-buffer-wakeup -Chris
Quoting Chris Wilson (2019-02-27 10:17:15) > Quoting Daniel Vetter (2017-08-07 10:28:58) > > On Fri, Aug 04, 2017 at 09:23:28AM +0100, Chris Wilson wrote: > > > After an event is sent, we try to copy it into the user buffer of the > > > first waiter in drm_read() and if the user buffer doesn't have enough > > > room we put it back onto the list. However, we didn't wake up any > > > subsequent waiter, so that event may sit on the list until either a new > > > vblank event is sent or a new waiter appears. Rare, but in the worst > > > case may lead to a stuck process. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > New subtestcase in igt@drm_read? > > Caught it! > Testcase: igt/drm_read/short-buffer-wakeup And pushed with Ville's irc r-b. -Chris
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 59b75a974357..7e6db9f7a058 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -524,6 +524,7 @@ ssize_t drm_read(struct file *filp, char __user *buffer, file_priv->event_space -= length; list_add(&e->link, &file_priv->event_list); spin_unlock_irq(&dev->event_lock); + wake_up_interruptible(&file_priv->event_wait); break; }
After an event is sent, we try to copy it into the user buffer of the first waiter in drm_read() and if the user buffer doesn't have enough room we put it back onto the list. However, we didn't wake up any subsequent waiter, so that event may sit on the list until either a new vblank event is sent or a new waiter appears. Rare, but in the worst case may lead to a stuck process. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_file.c | 1 + 1 file changed, 1 insertion(+)