fdo-mirrors/libevdev
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/libevdev/libevdev.git synced 2025-12-24 19:20:06 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Don't read events unless required

Browse source 
With the previous approach, every libevdev_next_event() invocation triggered a
read() on the device fd. This is not efficient, the kernel provides whole
event frames at a time so we're guaranteed to have more events waiting unless
the current event is a SYN_REPORT.

Assuming a fast-enough client and e.g. a touchpad device with multiple axes
per frame, we'd thus trigger several unnecessary read() calls per event frame.

Drop this behavior, instead only trigger the read when our internal queue is
empty and we need more events.

Fallout:
- we don't have any warning about a too-slow sync, i.e. if a SYN_DROPPED
  arrives while we're syncing, we don't get a warning in the log anymore.
  the test for this was removed.
- the tests that required the specific behavior were rewritten accordingly
- a revoke on a kernel device doesn't return ENODEV until already-received
  events have been processed

The above shouldn't be an issue for existing real-world clients.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This commit is contained in:
Peter Hutterer 2019-01-16 13:55:25 +10:00
parent 951cd6a189
commit 76662deeff
3 changed files with 25 additions and 134 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
libevdev/libevdev.c
View file

@ -1068,8 +1068,7 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
read in any more.
*/
do {
if (!(flags & LIBEVDEV_READ_FLAG_BLOCKING) ||
queue_num_elements(dev) == 0) {
if (queue_num_elements(dev) == 0) {
rc = read_more_events(dev);
if (rc < 0 && rc != -EAGAIN)
goto out;
@ -1101,15 +1100,8 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
if (flags & LIBEVDEV_READ_FLAG_SYNC && dev->queue_nsync > 0) {
dev->queue_nsync--;
rc = LIBEVDEV_READ_STATUS_SYNC;
if (dev->queue_nsync == 0) {
struct input_event next;
if (dev->queue_nsync == 0)
dev->sync_state = SYNC_NONE;
if (queue_peek(dev, 0, &next) == 0 &&
next.type == EV_SYN && next.code == SYN_DROPPED)
log_info(dev, "SYN_DROPPED received after finished "
"sync - you're not keeping up\n");
}
}
out:

19
test/test-kernel.c
View file

@ -61,15 +61,17 @@ START_TEST(test_revoke)
uinput_device_event(uidev, EV_REL, REL_X, 1);
uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev1);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
for (int i = 0; i < 2; i++) {
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev1);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
rc = libevdev_next_event(dev2, LIBEVDEV_READ_FLAG_NORMAL, &ev2);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
rc = libevdev_next_event(dev2, LIBEVDEV_READ_FLAG_NORMAL, &ev2);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
ck_assert_int_eq(ev1.type, ev2.type);
ck_assert_int_eq(ev1.code, ev2.code);
ck_assert_int_eq(ev1.value, ev2.value);
ck_assert_int_eq(ev1.type, ev2.type);
ck_assert_int_eq(ev1.code, ev2.code);
ck_assert_int_eq(ev1.value, ev2.value);
}
/* revoke first device, expect it closed, second device still open */
dev_fd = libevdev_get_fd(dev);
@ -81,6 +83,9 @@ START_TEST(test_revoke)
}
ck_assert_msg(rc == 0, "Failed to revoke device: %s", strerror(errno));
uinput_device_event(uidev, EV_REL, REL_X, 1);
uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev1);
ck_assert_int_eq(rc, -ENODEV);

128
test/test-libevdev-events.c
View file

@ -147,11 +147,10 @@ START_TEST(test_syn_dropped_event)
/* This is a bit complicated:
we can't get SYN_DROPPED through uinput, so we push two events down
uinput, and fetch one off libevdev (reading in the other one on the
way). Then write a SYN_DROPPED on a pipe, switch the fd and read
one event off the wire (but returning the second event from
before). Switch back, so that when we do read off the SYN_DROPPED
we have the fd back on the device and the ioctls work.
uinput, and process those. Then write a SYN_DROPPED on a pipe,
switch the fd and read one event off the wire. Switch back, so
that when we do read off the SYN_DROPPED we have the fd back on
the device and the ioctls work.
*/
uinput_device_event(uidev, EV_KEY, BTN_LEFT, 1);
uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
@ -159,6 +158,12 @@ START_TEST(test_syn_dropped_event)
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
ck_assert_int_eq(ev.type, EV_KEY);
ck_assert_int_eq(ev.code, BTN_LEFT);
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
ck_assert_int_eq(ev.type, EV_SYN);
ck_assert_int_eq(ev.code, SYN_REPORT);
rc = pipe2(pipefd, O_NONBLOCK);
ck_assert_int_eq(rc, 0);
@ -168,18 +173,13 @@ START_TEST(test_syn_dropped_event)
ev.value = 0;
rc = write(pipefd[1], &ev, sizeof(ev));
ck_assert_int_eq(rc, sizeof(ev));
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
libevdev_change_fd(dev, uinput_device_get_fd(uidev));
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
ck_assert_int_eq(ev.type, EV_SYN);
ck_assert_int_eq(ev.code, SYN_REPORT);
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC);
ck_assert_int_eq(ev.type, EV_SYN);
ck_assert_int_eq(ev.code, SYN_DROPPED);
libevdev_change_fd(dev, uinput_device_get_fd(uidev));
/* only check for the rc, nothing actually changed on the device */
libevdev_free(dev);
@ -191,111 +191,6 @@ START_TEST(test_syn_dropped_event)
}
END_TEST
void double_syn_dropped_logfunc(enum libevdev_log_priority priority,
void *data,
const char *file, int line,
const char *func,
const char *format, va_list args)
{
unsigned int *hit = data;
*hit = 1;
}
START_TEST(test_double_syn_dropped_event)
{
struct uinput_device* uidev;
struct libevdev *dev;
int rc;
struct input_event ev;
int pipefd[2];
unsigned int logfunc_hit = 0;
test_create_device(&uidev, &dev,
EV_SYN, SYN_REPORT,
EV_SYN, SYN_DROPPED,
EV_REL, REL_X,
EV_REL, REL_Y,
EV_KEY, BTN_LEFT,
-1);
libevdev_set_log_function(double_syn_dropped_logfunc, &logfunc_hit);
/* This is a bit complicated:
we can't get SYN_DROPPED through uinput, so we push two events down
uinput, and fetch one off libevdev (reading in the other one on the
way). Then write a SYN_DROPPED on a pipe, switch the fd and read
one event off the wire (but returning the second event from
before). Switch back, so that when we do read off the SYN_DROPPED
we have the fd back on the device and the ioctls work.
*/
uinput_device_event(uidev, EV_KEY, BTN_LEFT, 1);
uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
ck_assert_int_eq(ev.type, EV_KEY);
ck_assert_int_eq(ev.code, BTN_LEFT);
rc = pipe2(pipefd, O_NONBLOCK);
ck_assert_int_eq(rc, 0);
libevdev_change_fd(dev, pipefd[0]);
ev.type = EV_SYN;
ev.code = SYN_DROPPED;
ev.value = 0;
rc = write(pipefd[1], &ev, sizeof(ev));
ck_assert_int_eq(rc, sizeof(ev));
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
/* sneak in a button change event while we're not looking, this way
* the sync queue contains 2 events: BTN_LEFT and SYN_REPORT. */
uinput_device_event(uidev, EV_KEY, BTN_LEFT, 0);
ck_assert_int_eq(read(pipefd[0], &ev, sizeof(ev)), -1);
libevdev_change_fd(dev, uinput_device_get_fd(uidev));
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
ck_assert_int_eq(ev.type, EV_SYN);
ck_assert_int_eq(ev.code, SYN_REPORT);
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC);
ck_assert_int_eq(ev.type, EV_SYN);
ck_assert_int_eq(ev.code, SYN_DROPPED);
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC);
ck_assert_int_eq(ev.type, EV_KEY);
ck_assert_int_eq(ev.code, BTN_LEFT);
ck_assert_int_eq(ev.value, 0);
/* now write the second SYN_DROPPED on the pipe so we pick it up
* before we finish syncing. */
libevdev_change_fd(dev, pipefd[0]);
ev.type = EV_SYN;
ev.code = SYN_DROPPED;
ev.value = 0;
rc = write(pipefd[1], &ev, sizeof(ev));
ck_assert_int_eq(rc, sizeof(ev));
rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev);
ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC);
ck_assert_int_eq(ev.type, EV_SYN);
ck_assert_int_eq(ev.code, SYN_REPORT);
ck_assert_int_eq(ev.value, 0);
/* back to enable the ioctls again */
libevdev_change_fd(dev, uinput_device_get_fd(uidev));
ck_assert_int_eq(logfunc_hit, 1);
libevdev_free(dev);
uinput_device_free(uidev);
close(pipefd[0]);
close(pipefd[1]);
libevdev_set_log_function(test_logfunc_abort_on_error, NULL);
}
END_TEST
START_TEST(test_event_type_filtered)
{
struct uinput_device* uidev;
@ -2149,7 +2044,6 @@ TEST_SUITE_ROOT_PRIVILEGES(libevdev_events)
tcase_add_test(tc, test_next_event_invalid_fd);
tcase_add_test(tc, test_next_event_blocking);
tcase_add_test(tc, test_syn_dropped_event);
tcase_add_test(tc, test_double_syn_dropped_event);
tcase_add_test(tc, test_event_type_filtered);
tcase_add_test(tc, test_event_code_filtered);
tcase_add_test(tc, test_has_event_pending);