[PATCH libinput 2/2] evdev: ratelimit SYN_DROPPED logging

[Peter Hutterer]

On Tue, Nov 04, 2014 at 03:17:36PM -0800, Bill Spitzak wrote:
> I got the impression the ratelimit object was intended to be a single static
> instance, not per-device. The idea is that if there is a burst of these at
> once they are all from the same device, or due to some interaction between
> them, so you want to limit it all. Also it prevents an implementation detail
> from being in the exposed data structure.

it's fine to be per device. most use-cases I've seen so far only had a
single device triggering this anyway (the touchpad/touchscreen). and the
information that multiple devices are stalling can be quite useful in
debugging.

Cheers,
   Peter

> 
> On 11/04/2014 12:35 AM, David Herrmann wrote:
> >Use the ratelimit helpers for SYN_DROPPED logging. This guarantees that we
> >will still receive SYN_DROPPED log-messages after multiple days of
> >runtime, even though there might have been a SYN_DROPPED flood at one
> >point in time.
> >
> >Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
> >---
> >  src/evdev.c | 14 +++++++-------
> >  src/evdev.h |  4 +---
> >  2 files changed, 8 insertions(+), 10 deletions(-)
> >
> >diff --git a/src/evdev.c b/src/evdev.c
> >index 3aa87a7..836ce56 100644
> >--- a/src/evdev.c
> >+++ b/src/evdev.c
> >@@ -926,16 +926,14 @@ evdev_device_dispatch(void *data)
> >  		rc = libevdev_next_event(device->evdev,
> >  					 LIBEVDEV_READ_FLAG_NORMAL, &ev);
> >  		if (rc == LIBEVDEV_READ_STATUS_SYNC) {
> >-			if (device->syn_drops_received < 10) {
> >-				device->syn_drops_received++;
> >+			if (ratelimit_test(&device->syn_drop_limit))
> >  				log_info(libinput, "SYN_DROPPED event from "
> >  					 "\"%s\" - some input events have "
> >  					 "been lost.\n", device->devname);
> >-				if (device->syn_drops_received == 10)
> >-					log_info(libinput, "No longer logging "
> >-						 "SYN_DROPPED events for "
> >-						 "\"%s\"\n", device->devname);
> >-			}
> >+			else if (ratelimit_cutoff(&device->syn_drop_limit))
> >+					log_info(libinput, "SYN_DROPPED flood "
> >+						 "for \"%s\"\n",
> >+						 device->devname);
> >
> >  			/* send one more sync event so we handle all
> >  			   currently pending events before we sync up
> >@@ -1296,6 +1294,8 @@ evdev_device_create(struct libinput_seat *seat,
> >  	device->scroll.threshold = 5.0; /* Default may be overridden */
> >  	device->scroll.direction = 0;
> >  	device->dpi = DEFAULT_MOUSE_DPI;
> >+	/* at most 5 SYN_DROPPED log-messages per 30s */
> >+	device->syn_drop_limit = RATELIMIT_INIT(30ULL * 1000 * 1000, 5);
> >
> >  	matrix_init_identity(&device->abs.calibration);
> >  	matrix_init_identity(&device->abs.usermatrix);
> >diff --git a/src/evdev.h b/src/evdev.h
> >index 666c8dc..eefbb79 100644
> >--- a/src/evdev.h
> >+++ b/src/evdev.h
> >@@ -137,9 +137,7 @@ struct evdev_device {
> >  	} buttons;
> >
> >  	int dpi; /* HW resolution */
> >-	/* The number of times libevdev processes a SYN_DROPPED, so we can
> >-	 * stop logging them to avoid flooding the logs. */
> >-	int syn_drops_received;
> >+	struct ratelimit syn_drop_limit; /* ratelimit for SYN_DROPPED logging */
> >  };
> >
> >  #define EVDEV_UNHANDLED_DEVICE ((struct evdev_device *) 1)