On Wed, Aug 29, 2012 at 04:26:47PM -0700, Chase Douglas wrote: > On 08/29/2012 03:25 PM, Peter Hutterer wrote: > >On Wed, Aug 29, 2012 at 01:09:32PM -0700, Chase Douglas wrote: > >>On 08/28/2012 11:14 PM, Peter Hutterer wrote: > >>>uinput can be too slow, leaving us with GuessDeviceNode() failing and an > >>>empty device string. Use inotify to tell us when a /dev/input/event device > >>>appeared and then continue. However, even when inotify tells us the device > >>>is there, the EVIOCGNAME may still fail on the device, so we need to keep > >>>the backup GuessDeviceNode() in place. > >>> > >>>This leaves us with a race condition - if a different device pops up while > >>>we're waiting, then we may still not get the device name. Chance of that > >>>happening is tiny. > >>> > >>>Signed-off-by: Peter Hutterer <[email protected]> > >>>--- > >>> src/device.cpp | 59 > >>> +++++++++++++++++++++++++++++++++++++- > >>> test/.gitignore | 1 + > >>> test/Makefile.am | 10 +++++-- > >>> test/PIXART-USB-OPTICAL-MOUSE.desc | 41 ++++++++++++++++++++++++++ > >>> test/device-test.cpp | 33 +++++++++++++++++++++ > >>> 5 files changed, 141 insertions(+), 3 deletions(-) > >>> create mode 100644 test/PIXART-USB-OPTICAL-MOUSE.desc > >>> create mode 100644 test/device-test.cpp > >>> > >>>diff --git a/src/device.cpp b/src/device.cpp > >>>index 232d3ae..ea98d17 100644 > >>>--- a/src/device.cpp > >>>+++ b/src/device.cpp > >>>@@ -31,6 +31,8 @@ > >>> #include <linux/input.h> > >>> #include <fcntl.h> > >>> #include <dirent.h> > >>>+#include <sys/inotify.h> > >>>+#include <poll.h> > >>> > >>> #include <stdexcept> > >>> > >>>@@ -110,6 +112,48 @@ void > >>>xorg::testing::evemu::Device::GuessDeviceNode(time_t ctime) { > >>> free(event_devices); > >>> } > >>> > >>>+static std::string wait_for_inotify(int fd) > >>>+{ > >>>+ std::string devnode; > >>>+ bool found = false; > >>>+ struct pollfd pfd; > >>>+ > >>>+ pfd.fd = fd; > >>>+ pfd.events = POLLIN; > >>>+ > >>>+ char buf[1024]; > >>>+ size_t bufidx = 0; > >>>+ > >>>+ while (!found && poll(&pfd, 1, 2000) > 0) { > >>>+ ssize_t r; > >>>+ > >>>+ r = read(fd, buf + bufidx, sizeof(buf) - bufidx); > >>>+ if (r == -1 && errno != EAGAIN) { > >>>+ std::cerr << "inotify read failed with: " << > >>>std::string(strerror(errno)) << std::endl; > >>>+ break; > >>>+ } > >>>+ > >>>+ bufidx += r; > >>>+ > >>>+ struct inotify_event *e = reinterpret_cast<struct > >>>inotify_event*>(buf); > >>>+ > >>>+ while (bufidx > sizeof(*e) && bufidx >= sizeof(*e) + e->len) { > >>>+ if (strncmp(e->name, "event", 5) == 0) { > >>>+ devnode = DEV_INPUT_DIR + std::string(e->name); > >>>+ found = true; > >>>+ break; > >>>+ } > >>>+ > >>>+ /* this packet didn't contain what we're looking for */ > >>>+ int len = sizeof(*e) + e->len; > >>>+ memmove(buf, buf + len, bufidx - len); > >>>+ bufidx -= len; > >>>+ } > >>>+ } > >>>+ > >>>+ return devnode; > >>>+} > >>>+ > >>> xorg::testing::evemu::Device::Device(const std::string& path) > >>> : d_(new Private) { > >>> static const char UINPUT_NODE[] = "/dev/uinput"; > >>>@@ -132,6 +176,14 @@ xorg::testing::evemu::Device::Device(const > >>>std::string& path) > >>> > >>> fclose(fp); > >>> > >>>+ int ifd = inotify_init1(IN_NONBLOCK); > >>>+ if (ifd == -1 || inotify_add_watch(ifd, DEV_INPUT_DIR, IN_CREATE) == > >>>-1) { > >>>+ std::cerr << "Failed to create inotify watch" << std::endl; > >>>+ if (ifd != -1) > >>>+ close(ifd); > >>>+ ifd = -1; > >>>+ } > >>>+ > >>> d_->fd = open(UINPUT_NODE, O_WRONLY); > >>> if (d_->fd < 0) { > >>> evemu_delete(d_->device); > >>>@@ -145,7 +197,12 @@ xorg::testing::evemu::Device::Device(const > >>>std::string& path) > >>> throw std::runtime_error("Failed to create evemu device"); > >>> } > >>> > >>>- GuessDeviceNode(d_->ctime); > >>>+ if (ifd != -1) { > >>>+ std::string devnode = wait_for_inotify(ifd); > >>>+ if (event_is_device(devnode, evemu_get_name(d_->device), d_->ctime)) > >>>+ d_->device_node = devnode; > >>>+ close(ifd); > >> > >>We could instead loop around here waiting until the correct device > >>is seen, potentially with a timeout? I'm guessing you've already > >>thought of that, so I'm interested to hear your thoughts. > > > >the problem here is the failed ioctl(EVIOCGNAME). inotify gives us the right > >device, but because the ioctl fails we cannot be sure it really is our > >device. > > > >the alternative here is to loop until the ioctl succeeds but I don't know if > >that can hang the test, and putting a loop limit on it just means we'll need > >GuessDeviceNode() again. > > > >you can try it, remove the GuessDeviceNode() call and run the new test in a > >loop > > > > while [ $? -eq 0 ]; do sudo ./device-test; done > > > >it'll run anywhere from a minute to several minutes before it fails. > > Hmm... strange. Oh well. This shouldn't be a huge concern when > running the tests.
actually, it is. that's why I started running the test as above. I hit the issue in real test runs (i.e. make check) after around 30-40 tests, but inconsistently. the while loop above was just a quicker way to reproduce, and it ruled out any issues with the tests themselves. > Reviewed-by: Chase Douglas <[email protected]> thanks! Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
