* [PATCH v2] drm: make drm_file use keyed wakeups @ 2020-04-24 16:26 Kenny Levinsen 2020-04-28 15:14 ` Daniel Vetter 2020-04-29 11:19 ` kl 0 siblings, 2 replies; 4+ messages in thread From: Kenny Levinsen @ 2020-04-24 16:26 UTC (permalink / raw) To: intel-gfx, dri-devel Cc: daniel, airlied, tzimmermann, mripard, maarten.lankhorst, linux-kernel, Kenny Levinsen Some processes, such as systemd, are only polling for EPOLLERR|EPOLLHUP. As drm_file uses unkeyed wakeups, such a poll can receive many spurious wakeups from uninteresting events if, for example, the file description is subscribed to vblank events. This is the case with systemd, as it polls a file description from logind that is shared with the users' compositor. Use keyed wakeups to allow the wakeup target to more efficiently discard these uninteresting events. Signed-off-by: Kenny Levinsen <kl@kl.wtf> --- drivers/gpu/drm/drm_file.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index c4c704e01961..ec25b3d979d9 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -608,7 +608,8 @@ 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); + wake_up_interruptible_poll(&file_priv->event_wait, + EPOLLIN | EPOLLRDNORM); break; } @@ -804,7 +805,8 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) list_del(&e->pending_link); list_add_tail(&e->link, &e->file_priv->event_list); - wake_up_interruptible(&e->file_priv->event_wait); + wake_up_interruptible_poll(&e->file_priv->event_wait, + EPOLLIN | EPOLLRDNORM); } EXPORT_SYMBOL(drm_send_event_locked); -- 2.26.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm: make drm_file use keyed wakeups 2020-04-24 16:26 [PATCH v2] drm: make drm_file use keyed wakeups Kenny Levinsen @ 2020-04-28 15:14 ` Daniel Vetter 2020-04-29 11:19 ` kl 1 sibling, 0 replies; 4+ messages in thread From: Daniel Vetter @ 2020-04-28 15:14 UTC (permalink / raw) To: Kenny Levinsen Cc: intel-gfx, dri-devel, daniel, airlied, tzimmermann, mripard, maarten.lankhorst, linux-kernel On Fri, Apr 24, 2020 at 06:26:15PM +0200, Kenny Levinsen wrote: > Some processes, such as systemd, are only polling for EPOLLERR|EPOLLHUP. > As drm_file uses unkeyed wakeups, such a poll can receive many spurious > wakeups from uninteresting events if, for example, the file description > is subscribed to vblank events. This is the case with systemd, as it > polls a file description from logind that is shared with the users' > compositor. > > Use keyed wakeups to allow the wakeup target to more efficiently discard > these uninteresting events. > > Signed-off-by: Kenny Levinsen <kl@kl.wtf> Hm I applied v1 and I'm not spotting what's different here, and there's no changelog explaining what changed ... Please send a fixup if there's anything important missing. -Daniel > --- > drivers/gpu/drm/drm_file.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index c4c704e01961..ec25b3d979d9 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -608,7 +608,8 @@ 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); > + wake_up_interruptible_poll(&file_priv->event_wait, > + EPOLLIN | EPOLLRDNORM); > break; > } > > @@ -804,7 +805,8 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) > list_del(&e->pending_link); > list_add_tail(&e->link, > &e->file_priv->event_list); > - wake_up_interruptible(&e->file_priv->event_wait); > + wake_up_interruptible_poll(&e->file_priv->event_wait, > + EPOLLIN | EPOLLRDNORM); > } > EXPORT_SYMBOL(drm_send_event_locked); > > -- > 2.26.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm: make drm_file use keyed wakeups 2020-04-24 16:26 [PATCH v2] drm: make drm_file use keyed wakeups Kenny Levinsen 2020-04-28 15:14 ` Daniel Vetter @ 2020-04-29 11:19 ` kl 2020-04-30 14:04 ` Daniel Vetter 1 sibling, 1 reply; 4+ messages in thread From: kl @ 2020-04-29 11:19 UTC (permalink / raw) To: Daniel Vetter Cc: intel-gfx, dri-devel, airlied, tzimmermann, mripard, maarten.lankhorst, linux-kernel April 28, 2020 5:14 PM, "Daniel Vetter" <daniel@ffwll.ch> wrote: > On Fri, Apr 24, 2020 at 06:26:15PM +0200, Kenny Levinsen wrote: > >> Some processes, such as systemd, are only polling for EPOLLERR|EPOLLHUP. >> As drm_file uses unkeyed wakeups, such a poll can receive many spurious >> wakeups from uninteresting events if, for example, the file description >> is subscribed to vblank events. This is the case with systemd, as it >> polls a file description from logind that is shared with the users' >> compositor. >> >> Use keyed wakeups to allow the wakeup target to more efficiently discard >> these uninteresting events. >> >> Signed-off-by: Kenny Levinsen <kl@kl.wtf> > > Hm I applied v1 and I'm not spotting what's different here, and there's no > changelog explaining what changed ... > > Please send a fixup if there's anything important missing. > -Daniel > It's only the summary that differed as you and sravn requested in #dri-devel, so it's probably fine. I should have explained the change. I'm still trying to get the hang of the email-based workflow. :) Best regards, Kenny Levinsen >> --- >> drivers/gpu/drm/drm_file.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c >> index c4c704e01961..ec25b3d979d9 100644 >> --- a/drivers/gpu/drm/drm_file.c >> +++ b/drivers/gpu/drm/drm_file.c >> @@ -608,7 +608,8 @@ 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); >> + wake_up_interruptible_poll(&file_priv->event_wait, >> + EPOLLIN | EPOLLRDNORM); >> break; >> } >> >> @@ -804,7 +805,8 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) >> list_del(&e->pending_link); >> list_add_tail(&e->link, >> &e->file_priv->event_list); >> - wake_up_interruptible(&e->file_priv->event_wait); >> + wake_up_interruptible_poll(&e->file_priv->event_wait, >> + EPOLLIN | EPOLLRDNORM); >> } >> EXPORT_SYMBOL(drm_send_event_locked); >> >> -- >> 2.26.1 > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm: make drm_file use keyed wakeups 2020-04-29 11:19 ` kl @ 2020-04-30 14:04 ` Daniel Vetter 0 siblings, 0 replies; 4+ messages in thread From: Daniel Vetter @ 2020-04-30 14:04 UTC (permalink / raw) To: kl Cc: Daniel Vetter, intel-gfx, dri-devel, airlied, tzimmermann, mripard, maarten.lankhorst, linux-kernel On Wed, Apr 29, 2020 at 11:19:07AM +0000, kl@kl.wtf wrote: > April 28, 2020 5:14 PM, "Daniel Vetter" <daniel@ffwll.ch> wrote: > > > On Fri, Apr 24, 2020 at 06:26:15PM +0200, Kenny Levinsen wrote: > > > >> Some processes, such as systemd, are only polling for EPOLLERR|EPOLLHUP. > >> As drm_file uses unkeyed wakeups, such a poll can receive many spurious > >> wakeups from uninteresting events if, for example, the file description > >> is subscribed to vblank events. This is the case with systemd, as it > >> polls a file description from logind that is shared with the users' > >> compositor. > >> > >> Use keyed wakeups to allow the wakeup target to more efficiently discard > >> these uninteresting events. > >> > >> Signed-off-by: Kenny Levinsen <kl@kl.wtf> > > > > Hm I applied v1 and I'm not spotting what's different here, and there's no > > changelog explaining what changed ... > > > > Please send a fixup if there's anything important missing. > > -Daniel > > > > It's only the summary that differed as you and sravn requested in #dri-devel, so it's probably fine. > > I should have explained the change. I'm still trying to get the hang of the email-based workflow. :) Oops sorry, I generally run as a stateless maintainer so forgot :-/ And yes email based workflow is full of warts, it's a pain. -Daniel > > Best regards, > Kenny Levinsen > > >> --- > >> drivers/gpu/drm/drm_file.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > >> index c4c704e01961..ec25b3d979d9 100644 > >> --- a/drivers/gpu/drm/drm_file.c > >> +++ b/drivers/gpu/drm/drm_file.c > >> @@ -608,7 +608,8 @@ 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); > >> + wake_up_interruptible_poll(&file_priv->event_wait, > >> + EPOLLIN | EPOLLRDNORM); > >> break; > >> } > >> > >> @@ -804,7 +805,8 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) > >> list_del(&e->pending_link); > >> list_add_tail(&e->link, > >> &e->file_priv->event_list); > >> - wake_up_interruptible(&e->file_priv->event_wait); > >> + wake_up_interruptible_poll(&e->file_priv->event_wait, > >> + EPOLLIN | EPOLLRDNORM); > >> } > >> EXPORT_SYMBOL(drm_send_event_locked); > >> > >> -- > >> 2.26.1 > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-30 14:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-24 16:26 [PATCH v2] drm: make drm_file use keyed wakeups Kenny Levinsen 2020-04-28 15:14 ` Daniel Vetter 2020-04-29 11:19 ` kl 2020-04-30 14:04 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).