[PATCH libinput] timer: drain data on the timerfd when it triggers
peter.hutterer at who-t.net
Thu Apr 30 16:07:12 PDT 2015
On Thu, Apr 30, 2015 at 09:24:41AM -0500, Derek Foreman wrote:
> On 29/04/15 08:33 PM, Peter Hutterer wrote:
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > I admit, I'm not sure of the effect it has leaving the data sitting there.
> > timerfd_create states that only the value read changes, not the actual size
> > of the data (which I couldn't reproduce in a quick test, it's always 1)
> > epoll sounds like it should keep triggering as long as data is sitting there
> > but it doesn't. Didn't have time to investigate further, so here's the patch
> > for what seems to be the correct thing to do.
> > src/timer.c | 3 +++
> > 1 file changed, 3 insertions(+)
> > diff --git a/src/timer.c b/src/timer.c
> > index f6c8e42..285f75b 100644
> > --- a/src/timer.c
> > +++ b/src/timer.c
> > @@ -101,6 +101,9 @@ libinput_timer_handler(void *data)
> > struct libinput *libinput = data;
> > struct libinput_timer *timer, *tmp;
> > uint64_t now;
> > + uint64_t discard;
> > +
> > + read(libinput->timer.fd, &discard, sizeof(discard));
> In some cases I think this can lead to a deadlock. When you reset a
> timerfd it empties out the data.
oh, right, this explains why I kept getting only one value even when
multiple timers had expired.
> So the following order of events:
> Timer fires (fd becomes readable)
> change timer for some reason
> fd did change, call timer handler
> At this point the read blocks until next timer fire (if you're lucky) or
> forever if no timer is set.
> I *think* you need this to make it safe...
> diff --git a/src/timer.c b/src/timer.c
> index 285f75b..114a649 100644
> --- a/src/timer.c
> +++ b/src/timer.c
> @@ -122,7 +122,8 @@ libinput_timer_handler(void *data)
> libinput_timer_subsys_init(struct libinput *libinput)
> - libinput->timer.fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
> + libinput->timer.fd = timerfd_create(CLOCK_MONOTONIC,
> + TFD_CLOEXEC | TFD_NONBLOCK);
> if (libinput->timer.fd < 0)
> return -1;
yep, I'll merge that in thanks.
> I don't know if you want to do anything in the handler when you catch an
> EAGAIN (which indicates the timer was reset between trigger and epoll...).
That can actually happen but shouldn't be an error, I think. We pull the
events off the devices in a loop, so there's the chance of the timer
triggering while we're in that loop and one subsequent event may cause e.g.
a tap timer reset. not 100% of the effect that has on the timerfd behaviour
though but it's an unrelated problem that we need to fix anyway to avoid
starving the timerfd.
> > now = libinput_now(libinput);
> > if (now == 0)
More information about the wayland-devel