This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 2f9c082f8f fs_epoll: serveral epoll issues fix
2f9c082f8f is described below
commit 2f9c082f8f26a1abcc5484085f329040c7224fda
Author: Bowen Wang <wangbow...@xiaomi.com>
AuthorDate: Fri Sep 22 15:17:15 2023 +0800
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 <wangbow...@xiaomi.com>
---
fs/vfs/fs_epoll.c | 153 +++++++++++++++++++++++++++++++++++++++++++----------
fs/vfs/fs_poll.c | 3 +-
include/sys/poll.h | 7 +++
3 files changed, 135 insertions(+), 28 deletions(-)
diff --git a/fs/vfs/fs_epoll.c b/fs/vfs/fs_epoll.c
index e599200f46..40ed949e32 100644
--- a/fs/vfs/fs_epoll.c
+++ b/fs/vfs/fs_epoll.c
@@ -48,9 +48,11 @@
struct epoll_node_s
{
- struct list_node node;
- epoll_data_t data;
- struct pollfd pfd;
+ struct list_node node;
+ epoll_data_t data;
+ 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.cb = poll_default_cb;
+ epn->pfd.arg = epn;
+ 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;
+ }
+
+ ret = num;
+ }
- return epoll_teardown(eph, evs, maxevents);
+ 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;
+ }
+
+ ret = num;
+ }
- return epoll_teardown(eph, evs, maxevents);
+ return ret;
err:
set_errno(-ret);
diff --git a/fs/vfs/fs_poll.c b/fs/vfs/fs_poll.c
index 9660e951ec..b3f23fecc3 100644
--- a/fs/vfs/fs_poll.c
+++ b/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);
diff --git a/include/sys/poll.h b/include/sys/poll.h
index 94904ffd50..d9665551df 100644
--- a/include/sys/poll.h
+++ b/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
****************************************************************************/
