fs_epoll: serveral epoll issues fix

1. fs_epoll: try again when epoll_teardown() return 0
when poll_notify() called larger than twice when epoll_wait() blocked
in the eph->sem, the semcount will be larger than 1 when epoll_wait()
unblocked and will return 0 directly at the next epoll_wait.
So retry to wait the eph->sem again when epoll_teardown return 0.

2. fs_epoll: poll_setup the fd again even this fd got non-expected event
Some poll implementations need call poll_setup again when their internal
states changed (e.g., local socket), so should add the fd to the epoll
teardown list and poll_setup again at the next epoll_wait even this fd
got the user non-expected event.

Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>

fs/vfs/fs_epoll.c

@@ -48,9 +48,11 @@
 
 struct epoll_node_s
 {
-  struct list_node      node;
+  struct list_node         node;
-  epoll_data_t          data;
+  epoll_data_t             data;
-  struct pollfd         pfd;
+  bool                     notified;
+  struct pollfd            pfd;
+  FAR struct epoll_head_s *eph;
 };
 
 typedef struct epoll_node_s epoll_node_t;
@@ -262,6 +264,20 @@ static int epoll_do_create(int size, int flags)
   return fd;
 }
 
+/****************************************************************************
+ * Name: epoll_setup
+ *
+ * Description:
+ *   Setup all the fd.
+ *
+ * Input Parameters:
+ *   eph       - The epoll head pointer
+ *
+ * Returned Value:
+ *   Positive on success, negative on fail
+ *
+ ****************************************************************************/
+
 static int epoll_setup(FAR epoll_head_t *eph)
 {
   FAR epoll_node_t *tepn;
@@ -280,6 +296,7 @@ static int epoll_setup(FAR epoll_head_t *eph)
        * cover the situation several poll event pending on one fd.
        */
 
+      epn->notified    = false;
       epn->pfd.revents = 0;
       ret = poll_fdsetup(epn->pfd.fd, &epn->pfd, true);
       if (ret < 0)
@@ -297,6 +314,24 @@ static int epoll_setup(FAR epoll_head_t *eph)
   return ret;
 }
 
+/****************************************************************************
+ * Name: epoll_teardown
+ *
+ * Description:
+ *   Teardown all the notifed fd and check the notified fd's event with user
+ *   expected event.
+ *
+ * Input Parameters:
+ *   eph       - The epoll head pointer
+ *   evs       - The epoll events array
+ *   maxevents - The epoll events array size
+ *
+ * Returned Value:
+ *   Return the number of fd that notifed and the events is also user
+ *   expected.
+ *
+ ****************************************************************************/
+
 static int epoll_teardown(FAR epoll_head_t *eph, FAR struct epoll_event *evs,
                           int maxevents)
 {
@@ -308,12 +343,22 @@ static int epoll_teardown(FAR epoll_head_t *eph, FAR struct epoll_event *evs,
 
   list_for_every_entry_safe(&eph->setup, epn, tepn, epoll_node_t, node)
     {
+      /* Only check the notifed fd */
+
+      if (!epn->notified)
+        {
+          continue;
+        }
+
+      /* Teradown all the notified fd */
+
+      poll_fdsetup(epn->pfd.fd, &epn->pfd, false);
+      list_delete(&epn->node);
+
       if (epn->pfd.revents != 0 && i < maxevents)
         {
           evs[i].data     = epn->data;
           evs[i++].events = epn->pfd.revents;
-          poll_fdsetup(epn->pfd.fd, &epn->pfd, false);
-          list_delete(&epn->node);
           if ((epn->pfd.events & EPOLLONESHOT) != 0)
             {
               list_add_tail(&eph->oneshot, &epn->node);
@@ -323,12 +368,47 @@ static int epoll_teardown(FAR epoll_head_t *eph, FAR struct epoll_event *evs,
               list_add_tail(&eph->teardown, &epn->node);
             }
         }
+      else
+        {
+          list_add_tail(&eph->teardown, &epn->node);
+        }
     }
 
   nxmutex_unlock(&eph->lock);
   return i;
 }
 
+/****************************************************************************
+ * Name: epoll_default_cb
+ *
+ * Description:
+ *   The default epoll callback function, this function do the final step of
+ *   poll notification.
+ *
+ * Input Parameters:
+ *   fds - The fds
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+static void epoll_default_cb(FAR struct pollfd *fds)
+{
+  FAR epoll_node_t *epn = fds->arg;
+  int semcount = 0;
+
+  epn->notified = true;
+  if (fds->revents != 0)
+    {
+      nxsem_get_value(&epn->eph->sem, &semcount);
+      if (semcount < 1)
+        {
+          nxsem_post(&epn->eph->sem);
+        }
+    }
+}
+
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -472,11 +552,13 @@ int epoll_ctl(int epfd, int op, int fd, FAR struct epoll_event *ev)
           }
 
         epn = container_of(list_remove_head(&eph->free), epoll_node_t, node);
+        epn->eph         = eph;
         epn->data        = ev->data;
-        epn->pfd.events  = ev->events;
+        epn->notified    = false;
+        epn->pfd.events  = ev->events | POLLALWAYS;
         epn->pfd.fd      = fd;
-        epn->pfd.arg     = &eph->sem;
+        epn->pfd.arg     = epn;
-        epn->pfd.cb      = poll_default_cb;
+        epn->pfd.cb      = epoll_default_cb;
         epn->pfd.revents = 0;
 
         ret = poll_fdsetup(fd, &epn->pfd, true);
@@ -530,12 +612,13 @@ int epoll_ctl(int epfd, int op, int fd, FAR struct epoll_event *ev)
           {
             if (epn->pfd.fd == fd)
               {
-                if (epn->pfd.events != ev->events)
+                if (epn->pfd.events != (ev->events | POLLALWAYS))
                   {
                     poll_fdsetup(fd, &epn->pfd, false);
 
+                    epn->notified    = false;
                     epn->data        = ev->data;
-                    epn->pfd.events  = ev->events;
+                    epn->pfd.events  = ev->events | POLLALWAYS;
                     epn->pfd.fd      = fd;
                     epn->pfd.revents = 0;
 
@@ -554,10 +637,11 @@ int epoll_ctl(int epfd, int op, int fd, FAR struct epoll_event *ev)
           {
             if (epn->pfd.fd == fd)
               {
-                if (epn->pfd.events != ev->events)
+                if (epn->pfd.events != (ev->events | POLLALWAYS))
                   {
+                    epn->notified    = false;
                     epn->data        = ev->data;
-                    epn->pfd.events  = ev->events;
+                    epn->pfd.events  = ev->events | POLLALWAYS;
                     epn->pfd.fd      = fd;
                     epn->pfd.revents = 0;
 
@@ -579,8 +663,9 @@ int epoll_ctl(int epfd, int op, int fd, FAR struct epoll_event *ev)
           {
             if (epn->pfd.fd == fd)
               {
+                epn->notified    = false;
                 epn->data        = ev->data;
-                epn->pfd.events  = ev->events;
+                epn->pfd.events  = ev->events | POLLALWAYS;
                 epn->pfd.fd      = fd;
                 epn->pfd.revents = 0;
 
@@ -630,6 +715,7 @@ int epoll_pwait(int epfd, FAR struct epoll_event *evs,
       return ERROR;
     }
 
+retry:
   ret = epoll_setup(eph);
   if (ret < 0)
     {
@@ -642,7 +728,7 @@ int epoll_pwait(int epfd, FAR struct epoll_event *evs,
 
   if (timeout == 0)
     {
-      ret = OK;
+      ret = -ETIMEDOUT;
     }
   else if (timeout > 0)
     {
@@ -658,10 +744,6 @@ int epoll_pwait(int epfd, FAR struct epoll_event *evs,
 #endif
 
       ret = nxsem_tickwait(&eph->sem, ticks);
-      if (ret == -ETIMEDOUT)
-        {
-          ret = OK;
-        }
     }
   else
     {
@@ -669,12 +751,22 @@ int epoll_pwait(int epfd, FAR struct epoll_event *evs,
     }
 
   nxsig_procmask(SIG_SETMASK, &oldsigmask, NULL);
-  if (ret < 0)
+  if (ret < 0 && ret != -ETIMEDOUT)
     {
       goto err;
     }
+  else /* ret >= 0 or ret == -ETIMEDOUT */
+    {
+      int num = epoll_teardown(eph, evs, maxevents);
+      if (num == 0 && ret >= 0)
+        {
+          goto retry;
+        }
 
-  return epoll_teardown(eph, evs, maxevents);
+      ret = num;
+    }
+
+  return ret;
 
 err:
   set_errno(-ret);
@@ -704,6 +796,7 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
       return ERROR;
     }
 
+retry:
   ret = epoll_setup(eph);
   if (ret < 0)
     {
@@ -714,7 +807,7 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
 
   if (timeout == 0)
     {
-      ret = OK;
+      ret = -ETIMEDOUT;
     }
   else if (timeout > 0)
     {
@@ -730,22 +823,28 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
 #endif
 
       ret = nxsem_tickwait(&eph->sem, ticks);
-      if (ret == -ETIMEDOUT)
-        {
-          ret = OK;
-        }
     }
   else
     {
       ret = nxsem_wait(&eph->sem);
     }
 
-  if (ret < 0)
+  if (ret < 0 && ret != -ETIMEDOUT)
     {
       goto err;
     }
+  else /* ret >= 0 or ret == -ETIMEDOUT */
+    {
+      int num = epoll_teardown(eph, evs, maxevents);
+      if (num == 0 && ret >= 0)
+        {
+          goto retry;
+        }
 
-  return epoll_teardown(eph, evs, maxevents);
+      ret = num;
+    }
+
+  return ret;
 
 err:
   set_errno(-ret);

fs/vfs/fs_poll.c

@@ -264,7 +264,8 @@ void poll_notify(FAR struct pollfd **afds, int nfds, pollevent_t eventset)
               fds->revents &= ~POLLOUT;
             }
 
-          if (fds->revents != 0 && fds->cb != NULL)
+          if ((fds->revents != 0 || (fds->events & POLLALWAYS) != 0) &&
+              fds->cb != NULL)
             {
               finfo("Report events: %08" PRIx32 "\n", fds->revents);
               fds->cb(fds);

include/sys/poll.h

@@ -61,6 +61,11 @@
  *     Device has been disconnected (revents only).
  *   POLLNVAL
  *     Invalid fd member (revents only).
+ *
+ *   POLLALWAYS
+ *     Indicate that should ALWAYS call the poll callback whether the
+ *     drvier notified the user expected event or not, and this value is
+ *     used inside kernal only (events only).
  */
 
 #define POLLIN       (0x01)  /* NuttX does not make priority distinctions */
@@ -78,6 +83,8 @@
 #define POLLRDHUP    (0x10)  /* NuttX does not support shutdown(fd, SHUT_RD) */
 #define POLLNVAL     (0x20)
 
+#define POLLALWAYS   (0x10000) /* For not conflict with Linux */
+
 /****************************************************************************
  * Public Type Definitions
  ****************************************************************************/