You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/09/19 03:32:04 UTC
[GitHub] [incubator-nuttx] CV-Bowen opened a new pull request, #7131: poll: add poll_notify() api and call it in all drivers. [WIP]
CV-Bowen opened a new pull request, #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131
## Summary
1. add poll_notify() api for poll notification;
2. call poll_notify() in all drivers poll notification process;
## Impact
Only code refactor
## Testing
Pass CI
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#issuecomment-1257648669
> @CV-Bowen @xiaoxiang781216 I noticed that `sim:usrsocktest` failed with this PR.
>
> ```
> NuttX 10.4.0 40ef5bc6db Sep 26 2022 16:59:16 sim sim
> nsh> usrsocktest
> Starting unit-tests...
> Testing group "char_dev" =>
> Group "char_dev": [OK]
> Testing group "no_daemon" =>
> Group "no_daemon": [OK]
> Testing group "basic_daemon" =>
> Group "basic_daemon": [OK]
> Testing group "basic_connect" =>
> Group "basic_connect": [OK]
> Testing group "basic_connect_delay" =>
> Group "basic_connect_delay": [OK]
> Testing group "no_block_connect" =>
> Group "no_block_connect": [OK]
> Testing group "basic_send" =>
> Group "basic_send": [OK]
> Testing group "no_block_send" =>
> Group "no_block_send": [OK]
> Testing group "block_send" =>
> Group "block_send": [OK]
> Testing group "no_block_recv" =>
> Group "no_block_recv": [OK]
> Testing group "block_recv" =>
> Group "block_recv": [OK]
> Testing group "remote_disconnect" =>
> [TEST ASSERT FAILED!]
> In function "remote_disconnect_poll":
> line 688: Assertion `(ssize_t)((pfd.revents & (0x01))) == (ssize_t)(((0x01)))' failed.
> got value: 0
> should be: 1
> Group "remote_disconnect": [FAILED]
> Testing group "basic_setsockopt" =>
> Group "basic_setsockopt": [OK]
> Testing group "basic_getsockopt" =>
> Group "basic_getsockopt": [OK]
> Testing group "basic_getsockname" =>
> Group "basic_getsockname": [OK]
> Testing group "wake_with_signal" =>
> Group "wake_with_signal": [OK]
> Testing group "multithread" =>
> Group "multithread": [OK]
> Unit-test groups done... OK:16, FAILED:1, TOTAL:17
> ```
I will take a look, thanks.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r977249262
##########
drivers/can/can.c:
##########
@@ -175,22 +175,7 @@ static int can_takesem(FAR sem_t *sem)
static void can_pollnotify(FAR struct can_dev_s *dev, pollevent_t eventset)
Review Comment:
Done
##########
drivers/input/keyboard_upper.c:
##########
@@ -105,22 +105,7 @@ static const struct file_operations g_keyboard_fops =
static void keyboard_notify(FAR struct keyboard_opriv_s *opriv)
Review Comment:
Done
##########
drivers/serial/serial.c:
##########
@@ -171,32 +171,7 @@ static int uart_takesem(FAR sem_t *sem, bool errout)
static void uart_pollnotify(FAR uart_dev_t *dev, pollevent_t eventset)
{
- int i;
-
- for (i = 0; i < CONFIG_SERIAL_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = dev->fds[i];
- if (fds)
- {
-#ifdef CONFIG_SERIAL_REMOVABLE
- fds->revents |= ((fds->events | (POLLERR | POLLHUP)) & eventset);
-#else
- fds->revents |= (fds->events & eventset);
-#endif
- if (fds->revents != 0)
- {
- int semcount;
-
- finfo("Report events: %08" PRIx32 "\n", fds->revents);
-
- nxsem_get_value(fds->sem, &semcount);
- if (semcount < 1)
- {
- nxsem_post(fds->sem);
- }
- }
- }
- }
+ poll_notify(dev->fds, CONFIG_SERIAL_NPOLLWAITERS, eventset);
Review Comment:
Done
##########
drivers/ipcc/ipcc_poll.c:
##########
@@ -175,32 +175,5 @@ int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
void ipcc_pollnotify(FAR struct ipcc_driver_s *priv, pollevent_t eventset)
Review Comment:
Done
##########
drivers/usbhost/usbhost_hidmouse.c:
##########
@@ -473,21 +473,7 @@ static void usbhost_forcetake(FAR sem_t *sem)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_HIDMOUSE_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_HIDMOUSE_NPOLLWAITERS, POLLIN);
Review Comment:
Done
##########
drivers/usbhost/usbhost_xboxcontroller.c:
##########
@@ -561,8 +561,6 @@ static void usbhost_destroy(FAR void *arg)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
Review Comment:
Done
##########
net/usrsock/usrsock_poll.c:
##########
@@ -117,8 +117,7 @@ static uint16_t poll_event(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
Done
##########
fs/mqueue/mq_open.c:
##########
@@ -373,28 +373,7 @@ static mqd_t nxmq_vopen(FAR const char *mq_name, int oflags, va_list ap)
#if CONFIG_FS_MQUEUE_NPOLLWAITERS > 0
void nxmq_pollnotify(FAR struct mqueue_inode_s *msgq, pollevent_t eventset)
Review Comment:
Done
##########
drivers/input/spq10kbd.c:
##########
@@ -446,21 +446,7 @@ static int spq10kbd_interrupt(int irq, FAR void *context, FAR void *arg)
static void spq10kbd_pollnotify(FAR struct spq10kbd_dev_s *priv)
Review Comment:
Done
##########
net/rpmsg/rpmsg_sockif.c:
##########
@@ -208,22 +208,7 @@ static inline void rpmsg_socket_post(FAR sem_t *sem)
static void rpmsg_socket_pollnotify(FAR struct rpmsg_socket_conn_s *conn,
Review Comment:
Done
##########
net/tcp/tcp_netpoll.c:
##########
@@ -352,12 +351,7 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
- {
- /* Yes.. then signal the poll logic */
-
- nxsem_post(fds->sem);
- }
+ poll_notify(&fds, 1, 0);
Review Comment:
Done
##########
drivers/pipes/pipe_common.c:
##########
@@ -80,35 +80,12 @@ static int pipecommon_semtake(FAR sem_t *sem)
static void pipecommon_pollnotify(FAR struct pipe_dev_s *dev,
Review Comment:
Done
##########
drivers/sensors/hc_sr04.c:
##########
@@ -275,24 +275,13 @@ static void hcsr04_notify(FAR struct hcsr04_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for data to become available,
* then wake them up now. NOTE: we wake up all waiting threads because we
* do not know that they are going to do. If they all try to read the
* data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_HCSR04_NPOLLWAITERS; i++)
- {
- FAR struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- hcsr04_dbg("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_HCSR04_NPOLLWAITERS, POLLIN);
Review Comment:
Done
##########
drivers/sensors/lis2dh.c:
##########
@@ -740,24 +740,13 @@ static void lis2dh_notify(FAR struct lis2dh_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for LIS2DH data to become
* available, then wake them up now. NOTE: we wake up all waiting threads
* because we do not know that they are going to do. If they all try to
* read the data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_LIS2DH_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- lis2dh_dbg("lis2dh: Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_LIS2DH_NPOLLWAITERS, POLLIN);
Review Comment:
Done
##########
fs/vfs/fs_poll.c:
##########
@@ -276,6 +277,60 @@ static inline int poll_teardown(FAR struct pollfd *fds, nfds_t nfds,
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: poll_notify
+ *
+ * Description:
+ * Notify the poll, this function should be called by drivers to notify
+ * the caller the poll is ready.
+ *
+ * Input Parameters:
+ * afds - The fds array
+ * nfds - Number of fds array
+ * eventset - List of events to check for activity
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void poll_notify(FAR struct pollfd **afds, int nfds, pollevent_t eventset)
+{
+ int i;
+ int semcount;
+ FAR struct pollfd *fds;
+
+ DEBUGASSERT(afds != NULL && nfds >= 1);
+
+ for (i = 0; i < nfds; i++)
+ {
+ fds = afds[i];
+ if (fds != NULL)
+ {
+ /* The error event must be set in fds->revents */
+
+ fds->revents |= eventset & (fds->events | POLLERR | POLLHUP);
Review Comment:
Done
##########
drivers/usbdev/adb.c:
##########
@@ -1553,22 +1553,7 @@ static void adb_char_notify_readers(FAR struct usbdev_adb_s *priv)
static void adb_char_pollnotify(FAR struct usbdev_adb_s *dev,
pollevent_t eventset)
{
- FAR struct pollfd *fds;
- int i;
-
- for (i = 0; i < CONFIG_USBADB_NPOLLWAITERS; i++)
- {
- fds = dev->fds[i];
- if (fds)
- {
- fds->revents |= eventset & (fds->events | POLLERR | POLLHUP);
-
- if (fds->revents != 0)
- {
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(dev->fds, CONFIG_USBADB_NPOLLWAITERS, eventset);
Review Comment:
Done
##########
net/local/local_netpoll.c:
##########
@@ -125,21 +126,7 @@ void local_event_pollnotify(FAR struct local_conn_s *conn,
pollevent_t eventset)
{
#ifdef CONFIG_NET_LOCAL_STREAM
- int i;
-
- for (i = 0; i < LOCAL_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = conn->lc_event_fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & eventset);
- if (fds->revents != 0)
- {
- ninfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(conn->lc_event_fds, LOCAL_NPOLLWAITERS, eventset);
Review Comment:
Done
##########
drivers/usbhost/usbhost_cdcmbim.c:
##########
@@ -405,21 +405,7 @@ static int usbhost_ctrl_cmd(FAR struct usbhost_cdcmbim_s *priv,
static void usbhost_pollnotify(FAR struct usbhost_cdcmbim_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_USBHOST_CDCMBIM_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_USBHOST_CDCMBIM_NPOLLWAITERS, POLLIN);
Review Comment:
Done
##########
net/tcp/tcp_netpoll.c:
##########
@@ -167,8 +167,7 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev,
info->cb->priv = NULL;
info->cb->event = NULL;
- info->fds->revents |= eventset;
- nxsem_post(info->fds->sem);
+ poll_notify(&info->fds, 1, eventset);
Review Comment:
Done
##########
drivers/usbhost/usbhost_hidkbd.c:
##########
@@ -686,21 +686,7 @@ static void usbhost_forcetake(FAR sem_t *sem)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_HIDKBD_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_HIDKBD_NPOLLWAITERS, POLLIN);
Review Comment:
Done
##########
drivers/sensors/hts221.c:
##########
@@ -1058,24 +1058,13 @@ static void hts221_notify(FAR struct hts221_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for data to become available,
* then wake them up now. NOTE: we wake up all waiting threads because we
* do not know that they are going to do. If they all try to read the
* data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_HTS221_NPOLLWAITERS; i++)
- {
- FAR struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- hts221_dbg("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_HTS221_NPOLLWAITERS, POLLIN);
Review Comment:
Done
##########
fs/vfs/fs_timerfd.c:
##########
@@ -178,22 +178,7 @@ static timerfd_t timerfd_get_counter(FAR struct timerfd_priv_s *dev)
static void timerfd_pollnotify(FAR struct timerfd_priv_s *dev,
Review Comment:
Done
##########
drivers/usbmisc/fusb303.c:
##########
@@ -927,8 +927,6 @@ static int fusb303_poll(FAR struct file *filep, FAR struct pollfd *fds,
static void fusb303_notify(FAR struct fusb303_dev_s *priv)
Review Comment:
Done
##########
drivers/usrsock/usrsock_dev.c:
##########
@@ -159,20 +159,7 @@ static bool usrsockdev_is_opened(FAR struct usrsockdev_s *dev)
static void usrsockdev_pollnotify(FAR struct usrsockdev_s *dev,
Review Comment:
Done
##########
drivers/usbmisc/fusb301.c:
##########
@@ -741,24 +741,13 @@ static void fusb301_notify(FAR struct fusb301_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
Review Comment:
Done
##########
net/udp/udp_netpoll.c:
##########
@@ -101,8 +101,7 @@ static uint16_t udp_poll_eventhandler(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
Done
##########
fs/vfs/fs_eventfd.c:
##########
@@ -148,22 +148,7 @@ static void eventfd_destroy(FAR struct eventfd_priv_s *dev)
static void eventfd_pollnotify(FAR struct eventfd_priv_s *dev,
Review Comment:
Done
##########
net/usrsock/usrsock_poll.c:
##########
@@ -265,12 +264,7 @@ static int usrsock_pollsetup(FAR struct socket *psock,
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
Review Comment:
Done
##########
net/udp/udp_netpoll.c:
##########
@@ -227,12 +226,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
Review Comment:
Done
##########
net/netlink/netlink_sockif.c:
##########
@@ -594,8 +592,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
revents &= fds->events;
if (revents != 0)
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r975210780
##########
include/sys/poll.h:
##########
@@ -82,6 +82,16 @@
#define POLLSOCK (0x80)
#define POLLMASK (0xC0)
+/* poll_notify option definitions */
+
+#define POLL_NOTIFY_NOCHECK (0x00000001)
+
+#define poll_notify(sfds, nfds, eventset) \
Review Comment:
```suggestion
#define poll_notify(sfds, nfds, eventset) \
```
##########
fs/vfs/fs_poll.c:
##########
@@ -276,6 +277,62 @@ static inline int poll_teardown(FAR struct pollfd *fds, nfds_t nfds,
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: poll_notify_common
+ *
+ * Description:
+ * Notify the poll, this function should be called by drivers to notify
+ * the caller the poll is ready.
+ *
+ * Input Parameters:
+ * afds - The fds array
+ * nfds - Number of fds array
+ * eventset - List of events to check for activity
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void poll_notify_common(FAR struct pollfd **afds, int nfds,
+ pollevent_t eventset, uint32_t option)
+{
+ int i;
+ int semcount;
+ FAR struct pollfd *fds;
+
+ DEBUGASSERT(nfds >= 1);
+
+ for (i = 0; i < nfds; i++)
+ {
+ fds = afds[i];
+ if (fds == NULL)
+ {
+ continue;
+ }
Review Comment:
minor / optional:
I would better use `if (fds != NULL)` and avoid `continue`.
##########
include/sys/poll.h:
##########
@@ -82,6 +82,16 @@
#define POLLSOCK (0x80)
#define POLLMASK (0xC0)
+/* poll_notify option definitions */
+
+#define POLL_NOTIFY_NOCHECK (0x00000001)
+
+#define poll_notify(sfds, nfds, eventset) \
+ poll_notify_common(sfds, nfds, eventset, 0)
+
+#define poll_notify_nocheck(sfds, nfds, eventset) \
Review Comment:
```suggestion
#define poll_notify_nocheck(sfds, nfds, eventset) \
```
##########
net/netlink/netlink_sockif.c:
##########
@@ -595,7 +593,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
if (revents != 0)
{
fds->revents = revents;
- nxsem_post(fds->sem);
+ poll_notify_nocheck(&fds, 1, 0);
Review Comment:
Should it be
```suggestion
poll_notify_nocheck(&fds, 1, revents);
```
?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r978407480
##########
net/rpmsg/rpmsg_sockif.c:
##########
@@ -910,7 +890,7 @@ static int rpmsg_socket_poll(FAR struct socket *psock,
if (eventset)
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r976127867
##########
net/netlink/netlink_sockif.c:
##########
@@ -595,7 +593,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
if (revents != 0)
{
fds->revents = revents;
- nxsem_post(fds->sem);
+ poll_notify_nocheck(&fds, 1, 0);
Review Comment:
Yes, and i will delete the api poll_notify_nocheck() (only left poll_notify()), because we must check fds->events before set the fds->revents, otherwise the app may poll success but the fds->revent is not the app wants.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r976117835
##########
net/netlink/netlink_sockif.c:
##########
@@ -595,7 +593,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
if (revents != 0)
{
fds->revents = revents;
- nxsem_post(fds->sem);
+ poll_notify_nocheck(&fds, 1, 0);
Review Comment:
What about this place. It seems to be different from other similar places
##########
net/netlink/netlink_sockif.c:
##########
@@ -595,7 +593,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
if (revents != 0)
{
fds->revents = revents;
- nxsem_post(fds->sem);
+ poll_notify_nocheck(&fds, 1, 0);
Review Comment:
What about this place? It seems to be different from other similar places
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r976633836
##########
drivers/analog/comp.c:
##########
@@ -91,35 +91,12 @@ static const struct comp_callback_s g_comp_callback =
static void comp_pollnotify(FAR struct comp_dev_s *dev,
pollevent_t eventset)
{
- int i;
-
if (eventset & POLLERR)
{
eventset &= ~(POLLOUT | POLLIN);
Review Comment:
don't need, handled by poll_notify now. let's remove comp_pollnotify and call poll_notify directly
##########
drivers/can/can.c:
##########
@@ -175,22 +175,7 @@ static int can_takesem(FAR sem_t *sem)
static void can_pollnotify(FAR struct can_dev_s *dev, pollevent_t eventset)
Review Comment:
remove?
##########
drivers/pipes/pipe_common.c:
##########
@@ -80,35 +80,12 @@ static int pipecommon_semtake(FAR sem_t *sem)
static void pipecommon_pollnotify(FAR struct pipe_dev_s *dev,
pollevent_t eventset)
{
- int i;
-
if (eventset & POLLERR)
{
eventset &= ~(POLLOUT | POLLIN);
Review Comment:
remove
##########
drivers/ipcc/ipcc_poll.c:
##########
@@ -175,32 +175,5 @@ int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
void ipcc_pollnotify(FAR struct ipcc_driver_s *priv, pollevent_t eventset)
Review Comment:
remove
##########
drivers/usbhost/usbhost_cdcmbim.c:
##########
@@ -405,21 +405,7 @@ static int usbhost_ctrl_cmd(FAR struct usbhost_cdcmbim_s *priv,
static void usbhost_pollnotify(FAR struct usbhost_cdcmbim_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_USBHOST_CDCMBIM_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_USBHOST_CDCMBIM_NPOLLWAITERS, POLLIN);
Review Comment:
remove usbhost_pollnotify
##########
drivers/usbhost/usbhost_hidkbd.c:
##########
@@ -686,21 +686,7 @@ static void usbhost_forcetake(FAR sem_t *sem)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_HIDKBD_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_HIDKBD_NPOLLWAITERS, POLLIN);
Review Comment:
remove usbhost_pollnotify
##########
drivers/usbhost/usbhost_xboxcontroller.c:
##########
@@ -561,8 +561,6 @@ static void usbhost_destroy(FAR void *arg)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
Review Comment:
remove usbhost_pollnotify
##########
drivers/usrsock/usrsock_dev.c:
##########
@@ -159,20 +159,7 @@ static bool usrsockdev_is_opened(FAR struct usrsockdev_s *dev)
static void usrsockdev_pollnotify(FAR struct usrsockdev_s *dev,
Review Comment:
remove
##########
fs/vfs/fs_timerfd.c:
##########
@@ -178,22 +178,7 @@ static timerfd_t timerfd_get_counter(FAR struct timerfd_priv_s *dev)
static void timerfd_pollnotify(FAR struct timerfd_priv_s *dev,
Review Comment:
remove
##########
net/tcp/tcp_netpoll.c:
##########
@@ -352,12 +351,7 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
- {
- /* Yes.. then signal the poll logic */
-
- nxsem_post(fds->sem);
- }
+ poll_notify(&fds, 1, 0);
Review Comment:
shouldn't change revents
##########
net/tcp/tcp_netpoll.c:
##########
@@ -167,8 +167,7 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev,
info->cb->priv = NULL;
info->cb->event = NULL;
- info->fds->revents |= eventset;
- nxsem_post(info->fds->sem);
+ poll_notify(&info->fds, 1, eventset);
Review Comment:
remove the check at line 162 and all `& info->fds->events`
##########
drivers/pipes/pipe_common.c:
##########
@@ -80,35 +80,12 @@ static int pipecommon_semtake(FAR sem_t *sem)
static void pipecommon_pollnotify(FAR struct pipe_dev_s *dev,
Review Comment:
remove
##########
drivers/input/keyboard_upper.c:
##########
@@ -105,22 +105,7 @@ static const struct file_operations g_keyboard_fops =
static void keyboard_notify(FAR struct keyboard_opriv_s *opriv)
Review Comment:
remove
##########
drivers/input/spq10kbd.c:
##########
@@ -446,21 +446,7 @@ static int spq10kbd_interrupt(int irq, FAR void *context, FAR void *arg)
static void spq10kbd_pollnotify(FAR struct spq10kbd_dev_s *priv)
Review Comment:
remove
##########
drivers/sensors/hc_sr04.c:
##########
@@ -275,24 +275,13 @@ static void hcsr04_notify(FAR struct hcsr04_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for data to become available,
* then wake them up now. NOTE: we wake up all waiting threads because we
* do not know that they are going to do. If they all try to read the
* data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_HCSR04_NPOLLWAITERS; i++)
- {
- FAR struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- hcsr04_dbg("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_HCSR04_NPOLLWAITERS, POLLIN);
Review Comment:
remove hcsr04_notify, call poll_notify directly
##########
net/local/local_netpoll.c:
##########
@@ -125,21 +126,7 @@ void local_event_pollnotify(FAR struct local_conn_s *conn,
pollevent_t eventset)
{
#ifdef CONFIG_NET_LOCAL_STREAM
- int i;
-
- for (i = 0; i < LOCAL_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = conn->lc_event_fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & eventset);
- if (fds->revents != 0)
- {
- ninfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(conn->lc_event_fds, LOCAL_NPOLLWAITERS, eventset);
Review Comment:
remove local_event_pollnotify
##########
drivers/sensors/hts221.c:
##########
@@ -1058,24 +1058,13 @@ static void hts221_notify(FAR struct hts221_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for data to become available,
* then wake them up now. NOTE: we wake up all waiting threads because we
* do not know that they are going to do. If they all try to read the
* data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_HTS221_NPOLLWAITERS; i++)
- {
- FAR struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- hts221_dbg("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_HTS221_NPOLLWAITERS, POLLIN);
Review Comment:
remove hts221_notify ditto
##########
drivers/sensors/lis2dh.c:
##########
@@ -740,24 +740,13 @@ static void lis2dh_notify(FAR struct lis2dh_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for LIS2DH data to become
* available, then wake them up now. NOTE: we wake up all waiting threads
* because we do not know that they are going to do. If they all try to
* read the data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_LIS2DH_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- lis2dh_dbg("lis2dh: Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_LIS2DH_NPOLLWAITERS, POLLIN);
Review Comment:
remove lis2dh_notify
##########
drivers/serial/serial.c:
##########
@@ -171,32 +171,7 @@ static int uart_takesem(FAR sem_t *sem, bool errout)
static void uart_pollnotify(FAR uart_dev_t *dev, pollevent_t eventset)
{
- int i;
-
- for (i = 0; i < CONFIG_SERIAL_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = dev->fds[i];
- if (fds)
- {
-#ifdef CONFIG_SERIAL_REMOVABLE
- fds->revents |= ((fds->events | (POLLERR | POLLHUP)) & eventset);
-#else
- fds->revents |= (fds->events & eventset);
-#endif
- if (fds->revents != 0)
- {
- int semcount;
-
- finfo("Report events: %08" PRIx32 "\n", fds->revents);
-
- nxsem_get_value(fds->sem, &semcount);
- if (semcount < 1)
- {
- nxsem_post(fds->sem);
- }
- }
- }
- }
+ poll_notify(dev->fds, CONFIG_SERIAL_NPOLLWAITERS, eventset);
Review Comment:
remove uart_pollnotify
##########
drivers/usbhost/usbhost_hidmouse.c:
##########
@@ -473,21 +473,7 @@ static void usbhost_forcetake(FAR sem_t *sem)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_HIDMOUSE_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_HIDMOUSE_NPOLLWAITERS, POLLIN);
Review Comment:
remove usbhost_pollnotify
##########
drivers/usbdev/adb.c:
##########
@@ -1553,22 +1553,7 @@ static void adb_char_notify_readers(FAR struct usbdev_adb_s *priv)
static void adb_char_pollnotify(FAR struct usbdev_adb_s *dev,
pollevent_t eventset)
{
- FAR struct pollfd *fds;
- int i;
-
- for (i = 0; i < CONFIG_USBADB_NPOLLWAITERS; i++)
- {
- fds = dev->fds[i];
- if (fds)
- {
- fds->revents |= eventset & (fds->events | POLLERR | POLLHUP);
-
- if (fds->revents != 0)
- {
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(dev->fds, CONFIG_USBADB_NPOLLWAITERS, eventset);
Review Comment:
remove adb_char_pollnotify
##########
drivers/usbmisc/fusb303.c:
##########
@@ -927,8 +927,6 @@ static int fusb303_poll(FAR struct file *filep, FAR struct pollfd *fds,
static void fusb303_notify(FAR struct fusb303_dev_s *priv)
Review Comment:
remove
##########
fs/mqueue/mq_open.c:
##########
@@ -373,28 +373,7 @@ static mqd_t nxmq_vopen(FAR const char *mq_name, int oflags, va_list ap)
#if CONFIG_FS_MQUEUE_NPOLLWAITERS > 0
void nxmq_pollnotify(FAR struct mqueue_inode_s *msgq, pollevent_t eventset)
Review Comment:
remove
##########
drivers/usbmisc/fusb301.c:
##########
@@ -741,24 +741,13 @@ static void fusb301_notify(FAR struct fusb301_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
Review Comment:
remove fusb301_notify
##########
fs/vfs/fs_eventfd.c:
##########
@@ -148,22 +148,7 @@ static void eventfd_destroy(FAR struct eventfd_priv_s *dev)
static void eventfd_pollnotify(FAR struct eventfd_priv_s *dev,
Review Comment:
remove
##########
net/udp/udp_netpoll.c:
##########
@@ -101,8 +101,7 @@ static uint16_t udp_poll_eventhandler(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
remove the check and ` & info->fds->events`
##########
net/usrsock/usrsock_poll.c:
##########
@@ -117,8 +117,7 @@ static uint16_t poll_event(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
remove the check, remove line 107-114
##########
net/udp/udp_netpoll.c:
##########
@@ -227,12 +226,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
Review Comment:
add events and don't change revents
##########
net/usrsock/usrsock_poll.c:
##########
@@ -265,12 +264,7 @@ static int usrsock_pollsetup(FAR struct socket *psock,
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
Review Comment:
let's create new variable(e.g. events), we shouldn't modify revents
##########
net/rpmsg/rpmsg_sockif.c:
##########
@@ -208,22 +208,7 @@ static inline void rpmsg_socket_post(FAR sem_t *sem)
static void rpmsg_socket_pollnotify(FAR struct rpmsg_socket_conn_s *conn,
Review Comment:
remove rpmsg_socket_pollnotify
##########
net/netlink/netlink_sockif.c:
##########
@@ -594,8 +592,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
revents &= fds->events;
if (revents != 0)
Review Comment:
remove line 592-593
##########
fs/vfs/fs_poll.c:
##########
@@ -276,6 +277,60 @@ static inline int poll_teardown(FAR struct pollfd *fds, nfds_t nfds,
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: poll_notify
+ *
+ * Description:
+ * Notify the poll, this function should be called by drivers to notify
+ * the caller the poll is ready.
+ *
+ * Input Parameters:
+ * afds - The fds array
+ * nfds - Number of fds array
+ * eventset - List of events to check for activity
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void poll_notify(FAR struct pollfd **afds, int nfds, pollevent_t eventset)
+{
+ int i;
+ int semcount;
+ FAR struct pollfd *fds;
+
+ DEBUGASSERT(afds != NULL && nfds >= 1);
+
+ for (i = 0; i < nfds; i++)
+ {
+ fds = afds[i];
+ if (fds != NULL)
+ {
+ /* The error event must be set in fds->revents */
+
+ fds->revents |= eventset & (fds->events | POLLERR | POLLHUP);
Review Comment:
remove the similar logic from all caller.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r977252653
##########
drivers/usbhost/usbhost_xboxcontroller.c:
##########
@@ -561,8 +561,6 @@ static void usbhost_destroy(FAR void *arg)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
Review Comment:
Seems, can not remove easily.
##########
drivers/usbhost/usbhost_xboxcontroller.c:
##########
@@ -561,8 +561,6 @@ static void usbhost_destroy(FAR void *arg)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
Review Comment:
Seems can not remove easily.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r978407691
##########
drivers/usrsock/usrsock_dev.c:
##########
@@ -514,7 +491,7 @@ static int usrsockdev_poll(FAR struct file *filep, FAR struct pollfd *fds,
if (eventset)
Review Comment:
Done
##########
fs/vfs/fs_eventfd.c:
##########
@@ -540,7 +514,7 @@ static int eventfd_do_poll(FAR struct file *filep, FAR struct pollfd *fds,
if (eventset)
Review Comment:
Done
##########
net/can/can_sockif.c:
##########
@@ -143,15 +143,14 @@ static uint16_t can_poll_eventhandler(FAR struct net_driver_s *dev,
else if ((flags & CAN_POLL) != 0 &&
psock_can_cansend(info->psock) >= 0)
{
- eventset |= (POLLOUT & info->fds->events);
+ eventset |= POLLOUT;
}
/* Awaken the caller of poll() is requested event occurred. */
if (eventset)
Review Comment:
Done
##########
net/icmp/icmp_netpoll.c:
##########
@@ -111,8 +111,7 @@ static uint16_t icmp_poll_eventhandler(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
Done
##########
net/icmpv6/icmpv6_netpoll.c:
##########
@@ -111,8 +111,7 @@ static uint16_t icmpv6_poll_eventhandler(FAR struct net_driver_s *dev,
if (eventset)
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#issuecomment-1257646355
@CV-Bowen @xiaoxiang781216
I noticed that `sim:usrsocktest` failed with this PR.
```
NuttX 10.4.0 40ef5bc6db Sep 26 2022 16:59:16 sim sim
nsh> usrsocktest
Starting unit-tests...
Testing group "char_dev" =>
Group "char_dev": [OK]
Testing group "no_daemon" =>
Group "no_daemon": [OK]
Testing group "basic_daemon" =>
Group "basic_daemon": [OK]
Testing group "basic_connect" =>
Group "basic_connect": [OK]
Testing group "basic_connect_delay" =>
Group "basic_connect_delay": [OK]
Testing group "no_block_connect" =>
Group "no_block_connect": [OK]
Testing group "basic_send" =>
Group "basic_send": [OK]
Testing group "no_block_send" =>
Group "no_block_send": [OK]
Testing group "block_send" =>
Group "block_send": [OK]
Testing group "no_block_recv" =>
Group "no_block_recv": [OK]
Testing group "block_recv" =>
Group "block_recv": [OK]
Testing group "remote_disconnect" =>
[TEST ASSERT FAILED!]
In function "remote_disconnect_poll":
line 688: Assertion `(ssize_t)((pfd.revents & (0x01))) == (ssize_t)(((0x01)))' failed.
got value: 0
should be: 1
Group "remote_disconnect": [FAILED]
Testing group "basic_setsockopt" =>
Group "basic_setsockopt": [OK]
Testing group "basic_getsockopt" =>
Group "basic_getsockopt": [OK]
Testing group "basic_getsockname" =>
Group "basic_getsockname": [OK]
Testing group "wake_with_signal" =>
Group "wake_with_signal": [OK]
Testing group "multithread" =>
Group "multithread": [OK]
Unit-test groups done... OK:16, FAILED:1, TOTAL:17
```
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#issuecomment-1257384621
Delete `if (eventset)` and add this judgement to fs_poll.c
modified: drivers/net/tun.c
modified: drivers/sensors/sensor.c
modified: drivers/serial/uart_bth4.c
modified: drivers/syslog/ramlog.c
modified: fs/mqueue/mq_open.c
modified: fs/vfs/fs_poll.c
modified: graphics/nxterm/nxterm_kbdin.c
modified: net/local/local_netpoll.c
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r978267822
##########
drivers/input/cypress_mbr3108.c:
##########
@@ -983,21 +983,8 @@ static int mbr3108_close(FAR struct file *filep)
static void mbr3108_poll_notify(FAR struct mbr3108_dev_s *priv)
Review Comment:
Done
##########
net/local/local_netpoll.c:
##########
@@ -215,12 +192,12 @@ int local_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
}
}
+ shadowfds[0] = *fds;
shadowfds[0].fd = 1; /* Does not matter */
- shadowfds[0].sem = fds->sem;
shadowfds[0].events = fds->events & ~POLLOUT;
Review Comment:
Done
##########
net/local/local_netpoll.c:
##########
@@ -215,12 +192,12 @@ int local_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
}
}
+ shadowfds[0] = *fds;
shadowfds[0].fd = 1; /* Does not matter */
- shadowfds[0].sem = fds->sem;
shadowfds[0].events = fds->events & ~POLLOUT;
+ shadowfds[1] = *fds;
shadowfds[1].fd = 0; /* Does not matter */
- shadowfds[1].sem = fds->sem;
shadowfds[1].events = fds->events & ~POLLIN;
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r975316188
##########
fs/vfs/fs_poll.c:
##########
@@ -276,6 +277,62 @@ static inline int poll_teardown(FAR struct pollfd *fds, nfds_t nfds,
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: poll_notify_common
+ *
+ * Description:
+ * Notify the poll, this function should be called by drivers to notify
+ * the caller the poll is ready.
+ *
+ * Input Parameters:
+ * afds - The fds array
+ * nfds - Number of fds array
+ * eventset - List of events to check for activity
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void poll_notify_common(FAR struct pollfd **afds, int nfds,
+ pollevent_t eventset, uint32_t option)
+{
+ int i;
+ int semcount;
+ FAR struct pollfd *fds;
+
+ DEBUGASSERT(nfds >= 1);
+
+ for (i = 0; i < nfds; i++)
+ {
+ fds = afds[i];
+ if (fds == NULL)
+ {
+ continue;
+ }
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#issuecomment-1252289466
> Do we have good test coverage for this change? I see that many places did not check `fd->sem` count before posting, but now in common function it is always checked. The impact seems to be quite high from the bird eye view.
The check is intent: it is unnecessary to post semaphore more than once because there is one and only one thread is waiting for it:
https://github.com/apache/incubator-nuttx/blob/master/fs/vfs/fs_poll.c#L423
you can see sem is a local variable defined at line 365.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#issuecomment-1256007777
I will double check the code to make sure all the poll notify places have been updated, Please do not merge. @xiaoxiang781216
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r977306509
##########
drivers/analog/comp.c:
##########
@@ -91,35 +91,12 @@ static const struct comp_callback_s g_comp_callback =
static void comp_pollnotify(FAR struct comp_dev_s *dev,
pollevent_t eventset)
{
- int i;
-
if (eventset & POLLERR)
{
eventset &= ~(POLLOUT | POLLIN);
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r979510556
##########
drivers/serial/uart_bth4.c:
##########
@@ -398,10 +383,10 @@ static int uart_bth4_poll(FAR struct file *filep, FAR struct pollfd *fds,
if (!circbuf_is_empty(&dev->circbuf))
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
- eventset |= (fds->events & POLLOUT);
+ eventset |= POLLOUT;
if (eventset)
Review Comment:
Done
##########
drivers/sensors/sensor.c:
##########
@@ -899,25 +883,25 @@ static int sensor_poll(FAR struct file *filep,
if (filep->f_oflags & O_NONBLOCK)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
else
{
nxsem_get_value(&user->buffersem, &semcount);
if (semcount > 0)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
}
}
else if (sensor_is_updated(upper, user))
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
if (user->changed)
{
- eventset |= (fds->events & POLLPRI);
+ eventset |= POLLPRI;
}
if (eventset)
Review Comment:
Done
##########
drivers/net/tun.c:
##########
@@ -1270,7 +1264,7 @@ int tun_poll(FAR struct file *filep, FAR struct pollfd *fds, bool setup)
if (priv->read_d_len != 0 || priv->write_d_len != 0)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
if (eventset)
Review Comment:
Done
##########
fs/mqueue/mq_open.c:
##########
@@ -127,12 +127,12 @@ static int nxmq_file_poll(FAR struct file *filep,
if (msgq->nmsgs < msgq->maxmsgs)
{
- eventset |= (fds->events & POLLOUT);
+ eventset |= POLLOUT;
}
if (msgq->nmsgs)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
if (eventset)
Review Comment:
Done
##########
fs/vfs/fs_poll.c:
##########
@@ -276,6 +277,60 @@ static inline int poll_teardown(FAR struct pollfd *fds, nfds_t nfds,
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: poll_notify
+ *
+ * Description:
+ * Notify the poll, this function should be called by drivers to notify
+ * the caller the poll is ready.
+ *
+ * Input Parameters:
+ * afds - The fds array
+ * nfds - Number of fds array
+ * eventset - List of events to check for activity
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void poll_notify(FAR struct pollfd **afds, int nfds, pollevent_t eventset)
+{
+ int i;
+ int semcount;
+ FAR struct pollfd *fds;
+
+ DEBUGASSERT(afds != NULL && nfds >= 1);
+
+ for (i = 0; i < nfds; i++)
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r975315293
##########
include/sys/poll.h:
##########
@@ -82,6 +82,16 @@
#define POLLSOCK (0x80)
#define POLLMASK (0xC0)
+/* poll_notify option definitions */
+
+#define POLL_NOTIFY_NOCHECK (0x00000001)
+
+#define poll_notify(sfds, nfds, eventset) \
Review Comment:
Done
##########
include/sys/poll.h:
##########
@@ -82,6 +82,16 @@
#define POLLSOCK (0x80)
#define POLLMASK (0xC0)
+/* poll_notify option definitions */
+
+#define POLL_NOTIFY_NOCHECK (0x00000001)
+
+#define poll_notify(sfds, nfds, eventset) \
+ poll_notify_common(sfds, nfds, eventset, 0)
+
+#define poll_notify_nocheck(sfds, nfds, eventset) \
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#issuecomment-1256102837
new modified files:
modified: drivers/power/battery/battery_charger.c
modified: drivers/power/battery/battery_gauge.c
modified: drivers/power/battery/battery_monitor.c
modified: drivers/rc/lirc_dev.c
modified: drivers/wireless/cc1101.c
modified: drivers/wireless/gs2200m.c
modified: drivers/wireless/lpwan/sx127x/sx127x.c
modified: drivers/wireless/nrf24l01.c
modified: fs/vfs/fs_epoll.c
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r977516989
##########
drivers/pipes/pipe_common.c:
##########
@@ -80,35 +80,12 @@ static int pipecommon_semtake(FAR sem_t *sem)
static void pipecommon_pollnotify(FAR struct pipe_dev_s *dev,
pollevent_t eventset)
{
- int i;
-
if (eventset & POLLERR)
{
eventset &= ~(POLLOUT | POLLIN);
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r977752212
##########
drivers/input/cypress_mbr3108.c:
##########
@@ -983,21 +983,8 @@ static int mbr3108_close(FAR struct file *filep)
static void mbr3108_poll_notify(FAR struct mbr3108_dev_s *priv)
Review Comment:
remove?
##########
net/local/local_netpoll.c:
##########
@@ -215,12 +192,12 @@ int local_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
}
}
+ shadowfds[0] = *fds;
shadowfds[0].fd = 1; /* Does not matter */
- shadowfds[0].sem = fds->sem;
shadowfds[0].events = fds->events & ~POLLOUT;
Review Comment:
shadowfds[0].events &= ~POLLOUT;
##########
net/local/local_netpoll.c:
##########
@@ -215,12 +192,12 @@ int local_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
}
}
+ shadowfds[0] = *fds;
shadowfds[0].fd = 1; /* Does not matter */
- shadowfds[0].sem = fds->sem;
shadowfds[0].events = fds->events & ~POLLOUT;
+ shadowfds[1] = *fds;
shadowfds[1].fd = 0; /* Does not matter */
- shadowfds[1].sem = fds->sem;
shadowfds[1].events = fds->events & ~POLLIN;
Review Comment:
shadowfds[0].events &= ~POLLIN;
##########
drivers/usbhost/usbhost_xboxcontroller.c:
##########
@@ -561,8 +561,6 @@ static void usbhost_destroy(FAR void *arg)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r976480245
##########
net/netlink/netlink_sockif.c:
##########
@@ -595,7 +593,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
if (revents != 0)
{
fds->revents = revents;
- nxsem_post(fds->sem);
+ poll_notify_nocheck(&fds, 1, 0);
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r978559626
##########
drivers/serial/uart_bth4.c:
##########
@@ -398,10 +383,10 @@ static int uart_bth4_poll(FAR struct file *filep, FAR struct pollfd *fds,
if (!circbuf_is_empty(&dev->circbuf))
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
- eventset |= (fds->events & POLLOUT);
+ eventset |= POLLOUT;
if (eventset)
Review Comment:
don't need
##########
drivers/sensors/sensor.c:
##########
@@ -899,25 +883,25 @@ static int sensor_poll(FAR struct file *filep,
if (filep->f_oflags & O_NONBLOCK)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
else
{
nxsem_get_value(&user->buffersem, &semcount);
if (semcount > 0)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
}
}
else if (sensor_is_updated(upper, user))
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
if (user->changed)
{
- eventset |= (fds->events & POLLPRI);
+ eventset |= POLLPRI;
}
if (eventset)
Review Comment:
don't need
##########
drivers/net/tun.c:
##########
@@ -1270,7 +1264,7 @@ int tun_poll(FAR struct file *filep, FAR struct pollfd *fds, bool setup)
if (priv->read_d_len != 0 || priv->write_d_len != 0)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
if (eventset)
Review Comment:
remove
##########
fs/vfs/fs_poll.c:
##########
@@ -276,6 +277,60 @@ static inline int poll_teardown(FAR struct pollfd *fds, nfds_t nfds,
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: poll_notify
+ *
+ * Description:
+ * Notify the poll, this function should be called by drivers to notify
+ * the caller the poll is ready.
+ *
+ * Input Parameters:
+ * afds - The fds array
+ * nfds - Number of fds array
+ * eventset - List of events to check for activity
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void poll_notify(FAR struct pollfd **afds, int nfds, pollevent_t eventset)
+{
+ int i;
+ int semcount;
+ FAR struct pollfd *fds;
+
+ DEBUGASSERT(afds != NULL && nfds >= 1);
+
+ for (i = 0; i < nfds; i++)
Review Comment:
```suggestion
for (i = 0; i < nfds && eventset; i++)
```
##########
fs/mqueue/mq_open.c:
##########
@@ -127,12 +127,12 @@ static int nxmq_file_poll(FAR struct file *filep,
if (msgq->nmsgs < msgq->maxmsgs)
{
- eventset |= (fds->events & POLLOUT);
+ eventset |= POLLOUT;
}
if (msgq->nmsgs)
{
- eventset |= (fds->events & POLLIN);
+ eventset |= POLLIN;
}
if (eventset)
Review Comment:
remove
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r978268443
##########
net/local/local_netpoll.c:
##########
@@ -125,21 +126,7 @@ void local_event_pollnotify(FAR struct local_conn_s *conn,
pollevent_t eventset)
{
#ifdef CONFIG_NET_LOCAL_STREAM
- int i;
-
- for (i = 0; i < LOCAL_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = conn->lc_event_fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & eventset);
- if (fds->revents != 0)
- {
- ninfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(conn->lc_event_fds, LOCAL_NPOLLWAITERS, eventset);
Review Comment:
Several places call local_event_pollnotify(), seems leave it here is better.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#issuecomment-1257671045
@masayuki2009 seems the poll_notify() (fs_poll()) should not clear the POLLIN if the POLLHUP set, i will discuss with @xiaoxiang781216 about the solution, thanks again.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r978284154
##########
net/rpmsg/rpmsg_sockif.c:
##########
@@ -910,7 +890,7 @@ static int rpmsg_socket_poll(FAR struct socket *psock,
if (eventset)
Review Comment:
remove?
##########
net/icmpv6/icmpv6_netpoll.c:
##########
@@ -111,8 +111,7 @@ static uint16_t icmpv6_poll_eventhandler(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
remove
##########
fs/vfs/fs_eventfd.c:
##########
@@ -540,7 +514,7 @@ static int eventfd_do_poll(FAR struct file *filep, FAR struct pollfd *fds,
if (eventset)
Review Comment:
remove
##########
net/can/can_sockif.c:
##########
@@ -143,15 +143,14 @@ static uint16_t can_poll_eventhandler(FAR struct net_driver_s *dev,
else if ((flags & CAN_POLL) != 0 &&
psock_can_cansend(info->psock) >= 0)
{
- eventset |= (POLLOUT & info->fds->events);
+ eventset |= POLLOUT;
}
/* Awaken the caller of poll() is requested event occurred. */
if (eventset)
Review Comment:
remove
##########
net/icmp/icmp_netpoll.c:
##########
@@ -111,8 +111,7 @@ static uint16_t icmp_poll_eventhandler(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
remove
##########
drivers/usrsock/usrsock_dev.c:
##########
@@ -514,7 +491,7 @@ static int usrsockdev_poll(FAR struct file *filep, FAR struct pollfd *fds,
if (eventset)
Review Comment:
remove
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org