Re: [PATCH v2] drm: make drm_file use keyed wakeups
On Wed, Apr 29, 2020 at 11:19:07AM +, k...@kl.wtf wrote: > April 28, 2020 5:14 PM, "Daniel Vetter" 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 > > > > 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(>link, _priv->event_list); > >> spin_unlock_irq(>event_lock); > >> - wake_up_interruptible(_priv->event_wait); > >> + wake_up_interruptible_poll(_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(>pending_link); > >> list_add_tail(>link, > >> >file_priv->event_list); > >> - wake_up_interruptible(>file_priv->event_wait); > >> + wake_up_interruptible_poll(>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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: make drm_file use keyed wakeups
April 28, 2020 5:14 PM, "Daniel Vetter" 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 > > 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(>link, _priv->event_list); >> spin_unlock_irq(>event_lock); >> - wake_up_interruptible(_priv->event_wait); >> + wake_up_interruptible_poll(_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(>pending_link); >> list_add_tail(>link, >> >file_priv->event_list); >> - wake_up_interruptible(>file_priv->event_wait); >> + wake_up_interruptible_poll(>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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: make drm_file use keyed wakeups
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 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(>link, _priv->event_list); > spin_unlock_irq(>event_lock); > - wake_up_interruptible(_priv->event_wait); > + > wake_up_interruptible_poll(_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(>pending_link); > list_add_tail(>link, > >file_priv->event_list); > - wake_up_interruptible(>file_priv->event_wait); > + wake_up_interruptible_poll(>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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: make drm_file use keyed wakeups
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 --- 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(>link, _priv->event_list); spin_unlock_irq(>event_lock); - wake_up_interruptible(_priv->event_wait); + wake_up_interruptible_poll(_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(>pending_link); list_add_tail(>link, >file_priv->event_list); - wake_up_interruptible(>file_priv->event_wait); + wake_up_interruptible_poll(>file_priv->event_wait, + EPOLLIN | EPOLLRDNORM); } EXPORT_SYMBOL(drm_send_event_locked); -- 2.26.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
