[PATCH libinput] timer: drain data on the timerfd when it triggers

[Derek Foreman]

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.  So the following order of events:

Timer fires (fd becomes readable)
change timer for some reason
epoll
  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)
 int
 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;


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...).
>  
>  	now = libinput_now(libinput);
>  	if (now == 0)
>