Allow select() calls on FDs that are already ready
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 1 Apr 2016 19:46:31 +0000 (15:46 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 5 Apr 2016 19:42:19 +0000 (15:42 -0400)
The super-spurious select() required programs to drain an FD completely
(i.e. read until you get an EAGAIN), though it is legal to just keep
asking.  Unfortunately, we have an app that does that (dropbear), so we
need to check if an FD is readable manually.

The bug was:

- packet arrives
- tap fires
- user wakes up
- user reads some of it, but not all of it
- user selects

Then we never way up, since a tap will not fire again.

If the user checks the FD (as we do here) and it has no data, then we can
sleep safely, knowing that an edge-triggered event will occur (since we
know the underlying queue was empty at some point).

Hopefully we don't need to do this for write().

Note that this doesn't work for "normal" disk files.  We don't have a way
to tell if a file is readable.  That's basically kernel support for select.

Also note that this 'fix' could mask errors related to changing the FD set
between successive calls to select().

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
user/iplib/select.c

index 76a6e7f..18176e6 100644 (file)
  *   select use it as a timer only.  if that comes up, we can expand this.
  * - if you epoll or FD tap an FD, then try to use select on it, you'll get an
  *   error (only one tap per FD).  select() only knows about the FDs in its set.
+ * - if you select() on a readfd that is a disk file, it'll always say it is
+ *   available for I/O.
  */
 
 #include <sys/select.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <sys/stat.h>
 
 #include <ros/common.h>
 #include <parlib/uthread.h>
@@ -128,6 +131,21 @@ static int select_tv_to_ep_timeout(struct timeval *tv)
        return tv->tv_sec * 1000 + DIV_ROUND_UP(tv->tv_usec, 1000);
 }
 
+/* Check with the kernel if FD is readable or not.  Some apps will call select()
+ * on something even if it is already readable.
+ *
+ * TODO: this *won't* work for disk based files.  It only works on qids that are
+ * backed with qio queues or something similar, where size == readable. */
+static bool fd_is_readable(int fd)
+{
+       struct stat stat_buf;
+       int ret;
+
+       ret = fstat(fd, &stat_buf);
+       assert(!ret);
+       return stat_buf.st_size > 0;
+}
+
 int select(int nfds, fd_set *readfds, fd_set *writefds,
            fd_set *exceptfds, struct timeval *timeout)
 {
@@ -143,6 +161,13 @@ int select(int nfds, fd_set *readfds, fd_set *writefds,
                errno = EINVAL;
                return -1;
        }
+       /* It is legal to select even if you didn't consume all of the data in an
+        * FD.  Similar for writables, though it's harder for us to check those.
+        * Hopefully we don't need to. */
+       for (int i = 0; i < nfds; i++) {
+               if (fd_is_set(i, readfds) && fd_is_readable(i))
+                       return nfds;
+       }
        uth_mutex_lock(fdset_mtx);
        for (int i = 0; i < nfds; i++) {
                if ((fd_is_set(i, readfds) || fd_is_set(i, writefds) ||