Re: [PR] fs_epoll: several epoll problems fix [nuttx]
CV-Bowen commented on PR #10706: URL: https://github.com/apache/nuttx/pull/10706#issuecomment-1789946388 @thebolt Could you try this PR, the endless loop issue should have been fixed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
xiaoxiang781216 merged PR #10706: URL: https://github.com/apache/nuttx/pull/10706 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
xiaoxiang781216 commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1378747374
##
fs/vfs/fs_epoll.c:
##
@@ -323,12 +368,47 @@ static int epoll_teardown(FAR epoll_head_t *eph, FAR
struct epoll_event *evs,
list_add_tail(>teardown, >node);
}
}
+ else
+{
+ list_add_tail(>teardown, >node);
+}
}
nxmutex_unlock(>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(>eph->sem, );
Review Comment:
how about multiple callers epoll_wait?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
CV-Bowen commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1378706704
##
fs/vfs/fs_epoll.c:
##
@@ -730,22 +823,28 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
#endif
ret = nxsem_tickwait(>sem, ticks);
- if (ret == -ETIMEDOUT)
-{
- ret = OK;
-}
}
else
{
ret = nxsem_wait(>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 && timeout != 0 && ret != -ETIMEDOUT)
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
CV-Bowen commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1378706278
##
fs/vfs/fs_epoll.c:
##
@@ -658,23 +744,29 @@ int epoll_pwait(int epfd, FAR struct epoll_event *evs,
#endif
ret = nxsem_tickwait(>sem, ticks);
- if (ret == -ETIMEDOUT)
-{
- ret = OK;
-}
}
else
{
ret = nxsem_wait(>sem);
}
nxsig_procmask(SIG_SETMASK, , 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 && timeout != 0 && ret != -ETIMEDOUT)
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
CV-Bowen commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1378700858
##
fs/vfs/fs_epoll.c:
##
@@ -323,12 +368,47 @@ static int epoll_teardown(FAR epoll_head_t *eph, FAR
struct epoll_event *evs,
list_add_tail(>teardown, >node);
}
}
+ else
+{
+ list_add_tail(>teardown, >node);
+}
}
nxmutex_unlock(>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(>eph->sem, );
Review Comment:
No need, only one place call `nxsem_wait()`, the smallest value of semcount
is -1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
xiaoxiang781216 commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1376318537
##
fs/vfs/fs_epoll.c:
##
@@ -323,12 +368,47 @@ static int epoll_teardown(FAR epoll_head_t *eph, FAR
struct epoll_event *evs,
list_add_tail(>teardown, >node);
}
}
+ else
+{
+ list_add_tail(>teardown, >node);
+}
}
nxmutex_unlock(>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(>eph->sem, );
Review Comment:
should we post until semcount == 1?
##
fs/vfs/fs_epoll.c:
##
@@ -658,23 +744,29 @@ int epoll_pwait(int epfd, FAR struct epoll_event *evs,
#endif
ret = nxsem_tickwait(>sem, ticks);
- if (ret == -ETIMEDOUT)
-{
- ret = OK;
-}
}
else
{
ret = nxsem_wait(>sem);
}
nxsig_procmask(SIG_SETMASK, , 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 && timeout != 0 && ret != -ETIMEDOUT)
Review Comment:
make the similar change
##
fs/vfs/fs_epoll.c:
##
@@ -730,22 +823,28 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
#endif
ret = nxsem_tickwait(>sem, ticks);
- if (ret == -ETIMEDOUT)
-{
- ret = OK;
-}
}
else
{
ret = nxsem_wait(>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 && timeout != 0 && ret != -ETIMEDOUT)
Review Comment:
but let's change line 810 to `ret = -ETIMEOUT;`:
```suggestion
if (num == 0 && ret >= 0)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
CV-Bowen commented on PR #10706: URL: https://github.com/apache/nuttx/pull/10706#issuecomment-1785098790 > can we add a flag(e.g. POLLALWAYS)? @xiaoxiang781216 Done. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
xiaoxiang781216 commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1376134981
##
fs/vfs/fs_epoll.c:
##
@@ -730,22 +823,28 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
#endif
ret = nxsem_tickwait(>sem, ticks);
- if (ret == -ETIMEDOUT)
-{
- ret = OK;
-}
}
else
{
ret = nxsem_wait(>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 && timeout != 0 && ret != -ETIMEDOUT)
Review Comment:
Ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
CV-Bowen commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1376055075
##
fs/vfs/fs_epoll.c:
##
@@ -730,22 +823,28 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
#endif
ret = nxsem_tickwait(>sem, ticks);
- if (ret == -ETIMEDOUT)
-{
- ret = OK;
-}
}
else
{
ret = nxsem_wait(>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 && timeout != 0 && ret != -ETIMEDOUT)
Review Comment:
When `timeout == 0`, there is an endless loop from `retry:` to `goto retry;`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
xiaoxiang781216 commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1375699183
##
fs/vfs/fs_epoll.c:
##
@@ -730,22 +823,28 @@ int epoll_wait(int epfd, FAR struct epoll_event *evs,
#endif
ret = nxsem_tickwait(>sem, ticks);
- if (ret == -ETIMEDOUT)
-{
- ret = OK;
-}
}
else
{
ret = nxsem_wait(>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 && timeout != 0 && ret != -ETIMEDOUT)
Review Comment:
```suggestion
if (num == 0 && ret != -ETIMEDOUT)
```
##
net/local/local_netpoll.c:
##
@@ -119,7 +119,10 @@ static void local_inout_poll_cb(FAR struct pollfd *fds)
{
FAR struct pollfd *originfds = fds->arg;
- poll_notify(, 1, fds->revents);
+ if (fds->revents != 0)
Review Comment:
can we add a flag(e.g. POLLALWAYS)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
CV-Bowen commented on code in PR #10706:
URL: https://github.com/apache/nuttx/pull/10706#discussion_r1375635957
##
fs/vfs/fs_epoll.c:
##
@@ -308,12 +311,22 @@ static int epoll_teardown(FAR epoll_head_t *eph, FAR
struct epoll_event *evs,
list_for_every_entry_safe(>setup, epn, tepn, epoll_node_t, node)
{
+ /* Only check the notifed fd */
+
+ if (epn->notified == false)
Review Comment:
Done
##
fs/vfs/fs_epoll.c:
##
@@ -323,12 +336,52 @@ static int epoll_teardown(FAR epoll_head_t *eph, FAR
struct epoll_event *evs,
list_add_tail(>teardown, >node);
}
}
+ else
+{
+ list_add_tail(>teardown, >node);
+}
}
nxmutex_unlock(>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;
+
+ if (epn == NULL)
Review Comment:
Done
##
fs/vfs/fs_epoll.c:
##
@@ -535,6 +590,7 @@ int epoll_ctl(int epfd, int op, int fd, FAR struct
epoll_event *ev)
poll_fdsetup(fd, >pfd, false);
epn->data= ev->data;
+epn->notified= false;
Review Comment:
Done
##
fs/vfs/fs_epoll.c:
##
@@ -473,10 +526,12 @@ int epoll_ctl(int epfd, int op, int fd, FAR struct
epoll_event *ev)
epn = container_of(list_remove_head(>free), epoll_node_t, node);
epn->data= ev->data;
+epn->eph = eph;
Review Comment:
Done
##
fs/vfs/fs_epoll.c:
##
@@ -674,7 +675,13 @@ int epoll_pwait(int epfd, FAR struct epoll_event *evs,
goto err;
}
- return epoll_teardown(eph, evs, maxevents);
+ ret = epoll_teardown(eph, evs, maxevents);
+ if (ret == 0 && timeout != 0)
Review Comment:
Done
##
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;
+ struct pollfdpfd;
+ FAR struct epoll_head_s *eph;
Review Comment:
Done
##
net/local/local_netpoll.c:
##
@@ -119,7 +119,10 @@ static void local_inout_poll_cb(FAR struct pollfd *fds)
{
FAR struct pollfd *originfds = fds->arg;
- poll_notify(, 1, fds->revents);
+ if (fds->revents != 0)
Review Comment:
Move this check out of poll notify to make epoll_default_cb can know which
fd is notifed. As mentioned in the commit, some drivers' poll implementations
need we call the poll_setup() again when their internal states changed (e.g.,
local socket), so I move this check to poll callback and epoll can know which
fd is notified and setup it again in the next epoll_wait call.
##
fs/vfs/fs_epoll.c:
##
@@ -281,6 +283,7 @@ static int epoll_setup(FAR epoll_head_t *eph)
*/
epn->pfd.revents = 0;
+ epn->notified= false;
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fs_epoll: several epoll problems fix [nuttx]
acassis commented on PR #10706: URL: https://github.com/apache/nuttx/pull/10706#issuecomment-1779502401 @CV-Bowen do you have news about this issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
