From 12fbe0a26bc63b457523bc0b0a2c9344b1779654 Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Sat, 28 Dec 2019 22:59:17 +0800 Subject: [PATCH 1/5] Network stack shouldn't allocate memory in the poll implmentation to avoid the heap fragment in the long run, other modification include: 1.Select MM_IOB automatically since ICMP[v6] socket can't work without the read ahead buffer 2.Remove the net lock since xxx_callback_free already do the same thing 3.TCP/UDP poll should work even the read ahead buffer isn't enabled at all Change-Id: I5ee70d5face1e385ea220f1496fe1b8e89528228 Signed-off-by: Xiang Xiao --- net/icmp/Kconfig | 6 ++++- net/icmp/Make.defs | 5 +--- net/icmp/icmp.h | 25 ++++++++++++----- net/icmp/icmp_netpoll.c | 48 ++++++++++----------------------- net/icmp/icmp_sockif.c | 4 --- net/icmpv6/Kconfig | 6 ++++- net/icmpv6/Make.defs | 5 +--- net/icmpv6/icmpv6.h | 29 ++++++++++++++------ net/icmpv6/icmpv6_netpoll.c | 48 +++++++++++---------------------- net/icmpv6/icmpv6_sockif.c | 4 --- net/inet/inet_sockif.c | 28 +++++++++---------- net/local/local.h | 7 ++--- net/local/local_netpoll.c | 32 +++++++++++++--------- net/tcp/Kconfig | 4 +++ net/tcp/Make.defs | 5 +--- net/tcp/tcp.h | 38 +++++++++++++++----------- net/tcp/tcp_netpoll.c | 54 +++++++++++-------------------------- net/udp/Kconfig | 4 +++ net/udp/Make.defs | 5 +--- net/udp/udp.h | 39 ++++++++++++++++----------- net/udp/udp_netpoll.c | 51 +++++++++++------------------------ net/usrsock/Kconfig | 4 +++ net/usrsock/usrsock.h | 13 +++++++++ net/usrsock/usrsock_poll.c | 34 ++++++++--------------- 24 files changed, 235 insertions(+), 263 deletions(-) diff --git a/net/icmp/Kconfig b/net/icmp/Kconfig index f691a56011151..04d7b3f5d1678 100644 --- a/net/icmp/Kconfig +++ b/net/icmp/Kconfig @@ -25,6 +25,7 @@ if NET_ICMP && !NET_ICMP_NO_STACK config NET_ICMP_SOCKET bool "IPPROTO_ICMP socket support" default n + select MM_IOB ---help--- Enable support for IPPROTO_ICMP sockets. These sockets are needed for application level support for sending ECHO (ping) requests and @@ -35,7 +36,10 @@ if NET_ICMP_SOCKET config NET_ICMP_NCONNS int "Max ICMP packet sockets" default 4 - depends on MM_IOB + +config NET_ICMP_NPOLLWAITERS + int "Number of ICMP poll waiters" + default 1 endif # NET_ICMP_SOCKET endif # NET_ICMP && !NET_ICMP_NO_STACK diff --git a/net/icmp/Make.defs b/net/icmp/Make.defs index f714c40f6a11f..31dbf4d71a5cf 100644 --- a/net/icmp/Make.defs +++ b/net/icmp/Make.defs @@ -42,10 +42,7 @@ NET_CSRCS += icmp_input.c ifeq ($(CONFIG_NET_ICMP_SOCKET),y) SOCK_CSRCS += icmp_sockif.c icmp_poll.c icmp_conn.c icmp_sendto.c -SOCK_CSRCS += icmp_recvfrom.c -ifeq ($(CONFIG_MM_IOB),y) -SOCK_CSRCS += icmp_netpoll.c -endif +SOCK_CSRCS += icmp_recvfrom.c icmp_netpoll.c endif # Include ICMP build support diff --git a/net/icmp/icmp.h b/net/icmp/icmp.h index 024ced215dfdd..13690fc48acb2 100644 --- a/net/icmp/icmp.h +++ b/net/icmp/icmp.h @@ -70,11 +70,24 @@ * Public types ****************************************************************************/ +struct socket; /* Forward reference */ +struct sockaddr; /* Forward reference */ +struct pollfd; /* Forward reference */ + #ifdef CONFIG_NET_ICMP_SOCKET /* Representation of a IPPROTO_ICMP socket connection */ struct devif_callback_s; /* Forward reference */ +/* This is a container that holds the poll-related information */ + +struct icmp_poll_s +{ + FAR struct socket *psock; /* IPPROTO_ICMP socket structure */ + FAR struct pollfd *fds; /* Needed to handle poll events */ + FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ +}; + struct icmp_conn_s { /* Common prologue of all connection structures. */ @@ -97,14 +110,18 @@ struct icmp_conn_s FAR struct net_driver_s *dev; /* Needed to free the callback structure */ -#ifdef CONFIG_MM_IOB /* ICMP response read-ahead list. A singly linked list of type struct * iob_qentry_s where the ICMP read-ahead data for the current ID is * retained. */ struct iob_queue_s readahead; /* Read-ahead buffering */ -#endif + + /* The following is a list of poll structures of threads waiting for + * socket events. + */ + + struct icmp_poll_s pollinfo[CONFIG_NET_ICMP_NPOLLWAITERS]; }; #endif @@ -130,10 +147,6 @@ EXTERN const struct sock_intf_s g_icmp_sockif; * Public Function Prototypes ****************************************************************************/ -struct socket; /* Forward reference */ -struct sockaddr; /* Forward reference */ -struct pollfd; /* Forward reference */ - /**************************************************************************** * Name: icmp_input * diff --git a/net/icmp/icmp_netpoll.c b/net/icmp/icmp_netpoll.c index 105225902f006..99b299db903cd 100644 --- a/net/icmp/icmp_netpoll.c +++ b/net/icmp/icmp_netpoll.c @@ -43,28 +43,12 @@ #include #include -#include #include -#include +#include "devif/devif.h" #include "netdev/netdev.h" #include "icmp/icmp.h" -#ifdef CONFIG_MM_IOB - -/**************************************************************************** - * Private Types - ****************************************************************************/ - -/* This is an allocated container that holds the poll-related information */ - -struct icmp_poll_s -{ - FAR struct socket *psock; /* IPPROTO_ICMP socket structure */ - FAR struct pollfd *fds; /* Needed to handle poll events */ - FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ -}; - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -178,18 +162,22 @@ int icmp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) DEBUGASSERT(conn != NULL && fds != NULL); - /* Allocate a container to hold the poll information */ - - info = (FAR struct icmp_poll_s *)kmm_malloc(sizeof(struct icmp_poll_s)); - if (!info) - { - return -ENOMEM; - } - /* Some of the following must be atomic */ net_lock(); + /* Find a container to hold the poll information */ + + info = conn->pollinfo; + while (info->psock != NULL) + { + if (++info >= &conn->pollinfo[CONFIG_NET_ICMP_NPOLLWAITERS]) + { + ret = -ENOMEM; + goto errout_with_lock; + } + } + /* Allocate a ICMP callback structure */ cb = icmp_callback_alloc(conn->dev, conn); @@ -249,11 +237,7 @@ int icmp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) nxsem_post(fds->sem); } - net_unlock(); - return OK; - errout_with_lock: - kmm_free(info); net_unlock(); return ret; } @@ -293,9 +277,7 @@ int icmp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) { /* Release the callback */ - net_lock(); icmp_callback_free(conn->dev, conn, info->cb); - net_unlock(); /* Release the poll/select data slot */ @@ -303,10 +285,8 @@ int icmp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) /* Then free the poll info container */ - kmm_free(info); + info->psock = NULL; } return OK; } - -#endif /* !CONFIG_MM_IOB */ diff --git a/net/icmp/icmp_sockif.c b/net/icmp/icmp_sockif.c index 47c1d4d91bbad..80277b4dc7dae 100644 --- a/net/icmp/icmp_sockif.c +++ b/net/icmp/icmp_sockif.c @@ -445,7 +445,6 @@ int icmp_listen(FAR struct socket *psock, int backlog) static int icmp_netpoll(FAR struct socket *psock, FAR struct pollfd *fds, bool setup) { -#ifdef CONFIG_MM_IOB /* Check if we are setting up or tearing down the poll */ if (setup) @@ -460,9 +459,6 @@ static int icmp_netpoll(FAR struct socket *psock, FAR struct pollfd *fds, return icmp_pollteardown(psock, fds); } -#else - return -ENOSYS; -#endif /* CONFIG_MM_IOB */ } /**************************************************************************** diff --git a/net/icmpv6/Kconfig b/net/icmpv6/Kconfig index 3c7afc3442211..1ef9e0a88efad 100644 --- a/net/icmpv6/Kconfig +++ b/net/icmpv6/Kconfig @@ -26,6 +26,7 @@ if NET_ICMPv6 && !NET_ICMPv6_NO_STACK config NET_ICMPv6_SOCKET bool "IPPROTO_ICMP6 socket support" default n + select MM_IOB ---help--- Enable support for IPPROTO_ICMP6 sockets. These sockets are needed for application level support for sending ICMPv7 ECHO requests and @@ -203,7 +204,10 @@ if NET_ICMPv6_SOCKET config NET_ICMPv6_NCONNS int "Max ICMPv6 packet sockets" default 4 - depends on MM_IOB + +config NET_ICMPv6_NPOLLWAITERS + int "Number of ICMPv6 poll waiters" + default 1 endif # NET_ICMPv6_SOCKET diff --git a/net/icmpv6/Make.defs b/net/icmpv6/Make.defs index 9ecec70a26445..d273447f20579 100644 --- a/net/icmpv6/Make.defs +++ b/net/icmpv6/Make.defs @@ -43,10 +43,7 @@ NET_CSRCS += icmpv6_linkipaddr.c ifeq ($(CONFIG_NET_ICMPv6_SOCKET),y) SOCK_CSRCS += icmpv6_sockif.c icmpv6_conn.c icmpv6_sendto.c -SOCK_CSRCS += icmpv6_recvfrom.c -ifeq ($(CONFIG_MM_IOB),y) -SOCK_CSRCS += icmpv6_netpoll.c -endif +SOCK_CSRCS += icmpv6_recvfrom.c icmpv6_netpoll.c endif ifeq ($(CONFIG_NET_ICMPv6_NEIGHBOR),y) diff --git a/net/icmpv6/icmpv6.h b/net/icmpv6/icmpv6.h index 189249ca009fe..f5f2ab5fcec3c 100644 --- a/net/icmpv6/icmpv6.h +++ b/net/icmpv6/icmpv6.h @@ -71,11 +71,26 @@ * Public Type Definitions ****************************************************************************/ +struct timespec; /* Forward reference */ +struct net_driver_s; /* Forward reference */ +struct socket; /* Forward reference */ +struct sockaddr; /* Forward reference */ +struct pollfd; /* Forward reference */ + #ifdef CONFIG_NET_ICMPv6_SOCKET /* Representation of a IPPROTO_ICMP socket connection */ struct devif_callback_s; /* Forward reference */ +/* This is a container that holds the poll-related information */ + +struct icmpv6_poll_s +{ + FAR struct socket *psock; /* IPPROTO_ICMP6 socket structure */ + FAR struct pollfd *fds; /* Needed to handle poll events */ + FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ +}; + struct icmpv6_conn_s { /* Common prologue of all connection structures. */ @@ -98,14 +113,18 @@ struct icmpv6_conn_s FAR struct net_driver_s *dev; /* Needed to free the callback structure */ -#ifdef CONFIG_MM_IOB /* ICMPv6 response read-ahead list. A singly linked list of type struct * iob_qentry_s where the ICMPv6 read-ahead data for the current ID is * retained. */ struct iob_queue_s readahead; /* Read-ahead buffering */ -#endif + + /* The following is a list of poll structures of threads waiting for + * socket events. + */ + + struct icmpv6_poll_s pollinfo[CONFIG_NET_ICMPv6_NPOLLWAITERS]; }; #endif @@ -155,12 +174,6 @@ EXTERN const struct sock_intf_s g_icmpv6_sockif; * Public Function Prototypes ****************************************************************************/ -struct timespec; /* Forward reference */ -struct net_driver_s; /* Forward reference */ -struct socket; /* Forward reference */ -struct sockaddr; /* Forward reference */ -struct pollfd; /* Forward reference */ - /**************************************************************************** * Name: icmpv6_input * diff --git a/net/icmpv6/icmpv6_netpoll.c b/net/icmpv6/icmpv6_netpoll.c index 1fbf964a80690..82f6dda646d33 100644 --- a/net/icmpv6/icmpv6_netpoll.c +++ b/net/icmpv6/icmpv6_netpoll.c @@ -43,28 +43,12 @@ #include #include -#include #include -#include +#include "devif/devif.h" #include "netdev/netdev.h" #include "icmpv6/icmpv6.h" -#ifdef CONFIG_MM_IOB - -/**************************************************************************** - * Private Types - ****************************************************************************/ - -/* This is an allocated container that holds the poll-related information */ - -struct icmpv6_poll_s -{ - FAR struct socket *psock; /* IPPROTO_ICMP6 socket structure */ - FAR struct pollfd *fds; /* Needed to handle poll events */ - FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ -}; - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -178,17 +162,23 @@ int icmpv6_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) DEBUGASSERT(conn != NULL && fds != NULL); - /* Allocate a container to hold the poll information */ + /* Some of the following must be atomic */ + + net_lock(); + + /* Find a container to hold the poll information */ - info = (FAR struct icmpv6_poll_s *)kmm_malloc(sizeof(struct icmpv6_poll_s)); - if (!info) + info = conn->pollinfo; + while (info->psock != NULL) { - return -ENOMEM; + if (++info >= &conn->pollinfo[CONFIG_NET_ICMPv6_NPOLLWAITERS]) + { + ret = -ENOMEM; + goto errout_with_lock; + } } - /* Some of the following must be atomic */ - - net_lock(); + /* Allocate a ICMP callback structure */ cb = icmpv6_callback_alloc(conn->dev, conn); if (cb == NULL) @@ -247,11 +237,7 @@ int icmpv6_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) nxsem_post(fds->sem); } - net_unlock(); - return OK; - errout_with_lock: - kmm_free(info); net_unlock(); return ret; } @@ -291,9 +277,7 @@ int icmpv6_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) { /* Release the callback */ - net_lock(); icmpv6_callback_free(conn->dev, conn, info->cb); - net_unlock(); /* Release the poll/select data slot */ @@ -301,10 +285,8 @@ int icmpv6_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) /* Then free the poll info container */ - kmm_free(info); + info->psock = NULL; } return OK; } - -#endif /* !CONFIG_MM_IOB */ diff --git a/net/icmpv6/icmpv6_sockif.c b/net/icmpv6/icmpv6_sockif.c index 1a21749b43ccd..f229320f54996 100644 --- a/net/icmpv6/icmpv6_sockif.c +++ b/net/icmpv6/icmpv6_sockif.c @@ -445,7 +445,6 @@ int icmpv6_listen(FAR struct socket *psock, int backlog) static int icmpv6_netpoll(FAR struct socket *psock, FAR struct pollfd *fds, bool setup) { -#ifdef CONFIG_MM_IOB /* Check if we are setting up or tearing down the poll */ if (setup) @@ -460,9 +459,6 @@ static int icmpv6_netpoll(FAR struct socket *psock, FAR struct pollfd *fds, return icmpv6_pollteardown(psock, fds); } -#else - return -ENOSYS; -#endif /* CONFIG_MM_IOB */ } /**************************************************************************** diff --git a/net/inet/inet_sockif.c b/net/inet/inet_sockif.c index b327398b41210..2f03075672d0a 100644 --- a/net/inet/inet_sockif.c +++ b/net/inet/inet_sockif.c @@ -956,29 +956,29 @@ static int inet_accept(FAR struct socket *psock, FAR struct sockaddr *addr, * ****************************************************************************/ -#if defined(HAVE_TCP_POLL) || defined(HAVE_UDP_POLL) +#if defined(NET_TCP_HAVE_STACK) || defined(NET_UDP_HAVE_STACK) static inline int inet_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) { -#ifdef HAVE_TCP_POLL +#ifdef NET_TCP_HAVE_STACK if (psock->s_type == SOCK_STREAM) { return tcp_pollsetup(psock, fds); } else -#endif /* HAVE_TCP_POLL */ -#ifdef HAVE_UDP_POLL +#endif /* NET_TCP_HAVE_STACK */ +#ifdef NET_UDP_HAVE_STACK if (psock->s_type != SOCK_STREAM) { return udp_pollsetup(psock, fds); } else -#endif /* HAVE_UDP_POLL */ +#endif /* NET_UDP_HAVE_STACK */ { return -ENOSYS; } } -#endif /* HAVE_TCP_POLL || HAVE_UDP_POLL */ +#endif /* NET_TCP_HAVE_STACK || NET_UDP_HAVE_STACK */ /**************************************************************************** * Name: inet_pollteardown @@ -996,29 +996,29 @@ static inline int inet_pollsetup(FAR struct socket *psock, * ****************************************************************************/ -#if defined(HAVE_TCP_POLL) || defined(HAVE_UDP_POLL) +#if defined(NET_TCP_HAVE_STACK) || defined(NET_UDP_HAVE_STACK) static inline int inet_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) { -#ifdef HAVE_TCP_POLL +#ifdef NET_TCP_HAVE_STACK if (psock->s_type == SOCK_STREAM) { return tcp_pollteardown(psock, fds); } else -#endif /* HAVE_TCP_POLL */ -#ifdef HAVE_UDP_POLL +#endif /* NET_TCP_HAVE_STACK */ +#ifdef NET_UDP_HAVE_STACK if (psock->s_type == SOCK_DGRAM) { return udp_pollteardown(psock, fds); } else -#endif /* HAVE_UDP_POLL */ +#endif /* NET_UDP_HAVE_STACK */ { return -ENOSYS; } } -#endif /* HAVE_TCP_POLL || HAVE_UDP_POLL */ +#endif /* NET_TCP_HAVE_STACK || NET_UDP_HAVE_STACK */ /**************************************************************************** * Name: inet_poll @@ -1041,7 +1041,7 @@ static inline int inet_pollteardown(FAR struct socket *psock, static int inet_poll(FAR struct socket *psock, FAR struct pollfd *fds, bool setup) { -#if defined(HAVE_TCP_POLL) || defined(HAVE_UDP_POLL) +#if defined(NET_TCP_HAVE_STACK) || defined(NET_UDP_HAVE_STACK) /* Check if we are setting up or tearing down the poll */ @@ -1061,7 +1061,7 @@ static int inet_poll(FAR struct socket *psock, FAR struct pollfd *fds, { return -ENOSYS; } -#endif /* HAVE_TCP_POLL || !HAVE_UDP_POLL */ +#endif /* NET_TCP_HAVE_STACK || !NET_UDP_HAVE_STACK */ } /**************************************************************************** diff --git a/net/local/local.h b/net/local/local.h index 944f02ff7236c..4eb0aaa5c6927 100644 --- a/net/local/local.h +++ b/net/local/local.h @@ -61,7 +61,7 @@ ****************************************************************************/ #define HAVE_LOCAL_POLL 1 -#define LOCAL_ACCEPT_NPOLLWAITERS 2 +#define LOCAL_NPOLLWAITERS 2 /* Packet format in FIFO: * @@ -168,10 +168,11 @@ struct local_conn_s #ifdef HAVE_LOCAL_POLL /* The following is a list if poll structures of threads waiting for - * socket accept events. + * socket events. */ - struct pollfd *lc_accept_fds[LOCAL_ACCEPT_NPOLLWAITERS]; + struct pollfd *lc_accept_fds[LOCAL_NPOLLWAITERS]; + struct pollfd lc_inout_fds[2*LOCAL_NPOLLWAITERS]; #endif /* Union of fields unique to SOCK_STREAM client, server, and connected diff --git a/net/local/local_netpoll.c b/net/local/local_netpoll.c index 81aff7c051730..ebcd978d5232d 100644 --- a/net/local/local_netpoll.c +++ b/net/local/local_netpoll.c @@ -44,7 +44,6 @@ #include #include -#include #include #include @@ -73,7 +72,7 @@ static int local_accept_pollsetup(FAR struct local_conn_s *conn, * slot for the poll structure reference */ - for (i = 0; i < LOCAL_ACCEPT_NPOLLWAITERS; i++) + for (i = 0; i < LOCAL_NPOLLWAITERS; i++) { /* Find an available slot */ @@ -87,7 +86,7 @@ static int local_accept_pollsetup(FAR struct local_conn_s *conn, } } - if (i >= LOCAL_ACCEPT_NPOLLWAITERS) + if (i >= LOCAL_NPOLLWAITERS) { fds->priv = NULL; ret = -EBUSY; @@ -143,7 +142,7 @@ void local_accept_pollnotify(FAR struct local_conn_s *conn, #ifdef CONFIG_NET_LOCAL_STREAM int i; - for (i = 0; i < LOCAL_ACCEPT_NPOLLWAITERS; i++) + for (i = 0; i < LOCAL_NPOLLWAITERS; i++) { struct pollfd *fds = conn->lc_accept_fds[i]; if (fds) @@ -215,22 +214,31 @@ int local_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) goto pollerr; } - /* Allocate shadow pollfds. */ + /* Find shadow pollfds. */ - shadowfds = kmm_zalloc(2 * sizeof(struct pollfd)); - if (!shadowfds) + net_lock(); + + shadowfds = conn->lc_inout_fds; + while (shadowfds->fd != 0) { - return -ENOMEM; + shadowfds += 2; + if (shadowfds >= &conn->lc_inout_fds[2*LOCAL_NPOLLWAITERS]) + { + net_unlock(); + return -ENOMEM; + } } - shadowfds[0].fd = 0; /* Does not matter */ + shadowfds[0].fd = 1; /* Does not matter */ shadowfds[0].sem = fds->sem; shadowfds[0].events = fds->events & ~POLLOUT; - shadowfds[1].fd = 1; /* Does not matter */ + shadowfds[1].fd = 0; /* Does not matter */ shadowfds[1].sem = fds->sem; shadowfds[1].events = fds->events & ~POLLIN; + net_unlock(); + /* Setup poll for both shadow pollfds. */ ret = file_poll(&conn->lc_infile, &shadowfds[0], true); @@ -245,7 +253,7 @@ int local_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) if (ret < 0) { - kmm_free(shadowfds); + shadowfds[0].fd = 0; fds->priv = NULL; goto pollerr; } @@ -367,7 +375,7 @@ int local_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) fds->revents |= shadowfds[0].revents | shadowfds[1].revents; fds->priv = NULL; - kmm_free(shadowfds); + shadowfds[0].fd = 0; } break; diff --git a/net/tcp/Kconfig b/net/tcp/Kconfig index 239a53e63dfd1..ca428058ac3a8 100644 --- a/net/tcp/Kconfig +++ b/net/tcp/Kconfig @@ -55,6 +55,10 @@ config NET_TCP_CONNS ---help--- Maximum number of TCP/IP connections (all tasks) +config NET_TCP_NPOLLWAITERS + int "Number of TCP poll waiters" + default 1 + config NET_TCP_RTO int "RTO of TCP/IP connections" default 3 diff --git a/net/tcp/Make.defs b/net/tcp/Make.defs index 6d67b02c58f05..5eec9e37c67d6 100644 --- a/net/tcp/Make.defs +++ b/net/tcp/Make.defs @@ -52,15 +52,12 @@ ifeq ($(CONFIG_NET_SENDFILE),y) SOCK_CSRCS += tcp_sendfile.c endif -ifeq ($(CONFIG_NET_TCP_READAHEAD),y) -SOCK_CSRCS += tcp_netpoll.c ifeq ($(CONFIG_TCP_NOTIFIER),y) SOCK_CSRCS += tcp_notifier.c ifeq ($(CONFIG_NET_TCP_WRITE_BUFFERS),y) SOCK_CSRCS += tcp_txdrain.c endif endif -endif ifeq ($(CONFIG_NET_TCPPROTO_OPTIONS),y) SOCK_CSRCS += tcp_setsockopt.c tcp_getsockopt.c @@ -71,7 +68,7 @@ endif NET_CSRCS += tcp_conn.c tcp_seqno.c tcp_devpoll.c tcp_finddev.c tcp_timer.c NET_CSRCS += tcp_send.c tcp_input.c tcp_appsend.c tcp_listen.c NET_CSRCS += tcp_monitor.c tcp_callback.c tcp_backlog.c tcp_ipselect.c -NET_CSRCS += tcp_recvwindow.c +NET_CSRCS += tcp_recvwindow.c tcp_netpoll.c # TCP write buffering diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index 53135f8692ed7..b242a9a18b75b 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -61,12 +61,6 @@ #define NET_TCP_HAVE_STACK 1 -/* Conditions for support TCP poll/select operations */ - -#ifdef CONFIG_NET_TCP_READAHEAD -# define HAVE_TCP_POLL -#endif - /* Allocate a new TCP data callback */ /* These macros allocate and free callback structures used for receiving @@ -110,6 +104,11 @@ * Public Type Definitions ****************************************************************************/ +struct file; /* Forward reference */ +struct sockaddr; /* Forward reference */ +struct socket; /* Forward reference */ +struct pollfd; /* Forward reference */ + /* Representation of a TCP connection. * * The tcp_conn_s structure is used for identifying a connection. All @@ -124,6 +123,18 @@ struct devif_callback_s; /* Forward reference */ struct tcp_backlog_s; /* Forward reference */ struct tcp_hdr_s; /* Forward reference */ +/* This is a container that holds the poll-related information */ + +struct tcp_poll_s +{ + FAR struct socket *psock; /* Needed to handle loss of connection */ + struct pollfd *fds; /* Needed to handle poll events */ + FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ +#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER) + int16_t key; /* Needed to cancel pending notification */ +#endif +}; + struct tcp_conn_s { /* Common prologue of all connection structures. */ @@ -274,6 +285,12 @@ struct tcp_conn_s FAR void *accept_private; int (*accept)(FAR struct tcp_conn_s *listener, FAR struct tcp_conn_s *conn); + + /* The following is a list of poll structures of threads waiting for + * socket events. + */ + + struct tcp_poll_s pollinfo[CONFIG_NET_TCP_NPOLLWAITERS]; }; /* This structure supports TCP write buffering */ @@ -335,11 +352,6 @@ EXTERN struct net_driver_s *g_netdevices; * Public Function Prototypes ****************************************************************************/ -struct file; /* Forward reference */ -struct sockaddr; /* Forward reference */ -struct socket; /* Forward reference */ -struct pollfd; /* Forward reference */ - /**************************************************************************** * Name: tcp_initialize * @@ -1535,9 +1547,7 @@ void tcp_wrbuffer_dump(FAR const char *msg, FAR struct tcp_wrbuffer_s *wrb, * ****************************************************************************/ -#ifdef HAVE_TCP_POLL int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds); -#endif /**************************************************************************** * Name: tcp_pollteardown @@ -1555,9 +1565,7 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds); * ****************************************************************************/ -#ifdef HAVE_TCP_POLL int tcp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds); -#endif /**************************************************************************** * Name: tcp_readahead_notifier_setup diff --git a/net/tcp/tcp_netpoll.c b/net/tcp/tcp_netpoll.c index eea06ee64114f..ca287155c1b88 100644 --- a/net/tcp/tcp_netpoll.c +++ b/net/tcp/tcp_netpoll.c @@ -55,24 +55,6 @@ #include "inet/inet.h" #include "tcp/tcp.h" -#ifdef HAVE_TCP_POLL - -/**************************************************************************** - * Private Types - ****************************************************************************/ - -/* This is an allocated container that holds the poll-related information */ - -struct tcp_poll_s -{ - FAR struct socket *psock; /* Needed to handle loss of connection */ - struct pollfd *fds; /* Needed to handle poll events */ - FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ -#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER) - int16_t key; /* Needed to cancel pending notification */ -#endif -}; - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -276,18 +258,22 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) } #endif - /* Allocate a container to hold the poll information */ - - info = (FAR struct tcp_poll_s *)kmm_malloc(sizeof(struct tcp_poll_s)); - if (!info) - { - return -ENOMEM; - } - /* Some of the following must be atomic */ net_lock(); + /* Find a container to hold the poll information */ + + info = conn->pollinfo; + while (info->psock != NULL) + { + if (++info >= &conn->pollinfo[CONFIG_NET_TCP_NPOLLWAITERS]) + { + ret = -ENOMEM; + goto errout_with_lock; + } + } + /* Allocate a TCP/IP callback structure */ cb = tcp_callback_alloc(conn); @@ -331,14 +317,14 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) fds->priv = (FAR void *)info; -#ifdef CONFIG_NET_TCPBACKLOG +#ifdef CONFIG_NET_TCP_READAHEAD /* Check for read data or backlogged connection availability now */ if (!IOB_QEMPTY(&conn->readahead) || tcp_backlogavailable(conn)) #else - /* Check for read data availability now */ + /* Check for backlogged connection now */ - if (!IOB_QEMPTY(&conn->readahead)) + if (tcp_backlogavailable(conn)) #endif { /* Normal data may be read without blocking. */ @@ -423,11 +409,7 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) } #endif - net_unlock(); - return OK; - errout_with_lock: - kmm_free(info); net_unlock(); return ret; } @@ -481,9 +463,7 @@ int tcp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) /* Release the callback */ - net_lock(); tcp_callback_free(conn, info->cb); - net_unlock(); /* Release the poll/select data slot */ @@ -491,10 +471,8 @@ int tcp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) /* Then free the poll info container */ - kmm_free(info); + info->psock = NULL; } return OK; } - -#endif /* HAVE_TCP_POLL */ diff --git a/net/udp/Kconfig b/net/udp/Kconfig index ec9609c361745..45ddcd4de9e52 100644 --- a/net/udp/Kconfig +++ b/net/udp/Kconfig @@ -55,6 +55,10 @@ config NET_UDP_CONNS ---help--- The maximum amount of open concurrent UDP sockets +config NET_UDP_NPOLLWAITERS + int "Number of UDP poll waiters" + default 1 + config NET_UDP_READAHEAD bool "Enable UDP/IP read-ahead buffering" default y diff --git a/net/udp/Make.defs b/net/udp/Make.defs index f7d09fc51b64a..541b336d19b88 100644 --- a/net/udp/Make.defs +++ b/net/udp/Make.defs @@ -52,11 +52,8 @@ else SOCK_CSRCS += udp_psock_sendto_unbuffered.c endif -ifeq ($(CONFIG_NET_UDP_READAHEAD),y) -SOCK_CSRCS += udp_netpoll.c ifeq ($(CONFIG_UDP_NOTIFIER),y) SOCK_CSRCS += udp_notifier.c -endif ifeq ($(CONFIG_NET_UDP_WRITE_BUFFERS),y) SOCK_CSRCS += udp_txdrain.c endif @@ -65,7 +62,7 @@ endif # Transport layer NET_CSRCS += udp_conn.c udp_devpoll.c udp_send.c udp_input.c udp_finddev.c -NET_CSRCS += udp_callback.c udp_ipselect.c +NET_CSRCS += udp_callback.c udp_ipselect.c udp_netpoll.c # UDP write buffering diff --git a/net/udp/udp.h b/net/udp/udp.h index 0dd327cec7adf..b497b2a5c4b4d 100644 --- a/net/udp/udp.h +++ b/net/udp/udp.h @@ -65,12 +65,6 @@ #define NET_UDP_HAVE_STACK 1 -/* Conditions for support UDP poll/select operations */ - -#ifdef CONFIG_NET_UDP_READAHEAD -# define HAVE_UDP_POLL -#endif - #ifdef CONFIG_NET_UDP_WRITE_BUFFERS /* UDP write buffer dump macros */ @@ -99,11 +93,29 @@ * Public Type Definitions ****************************************************************************/ +struct sockaddr; /* Forward reference */ +struct socket; /* Forward reference */ +struct net_driver_s; /* Forward reference */ +struct pollfd; /* Forward reference */ + /* Representation of a UDP connection */ struct devif_callback_s; /* Forward reference */ struct udp_hdr_s; /* Forward reference */ +/* This is a container that holds the poll-related information */ + +struct udp_poll_s +{ + FAR struct socket *psock; /* Needed to handle loss of connection */ + FAR struct net_driver_s *dev; /* Needed to free the callback structure */ + struct pollfd *fds; /* Needed to handle poll events */ + FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ +#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER) + int16_t key; /* Needed to cancel pending notification */ +#endif +}; + struct udp_conn_s { /* Common prologue of all connection structures. */ @@ -151,6 +163,12 @@ struct udp_conn_s sq_queue_t write_q; /* Write buffering for UDP packets */ FAR struct net_driver_s *dev; /* Last device */ #endif + + /* The following is a list of poll structures of threads waiting for + * socket events. + */ + + struct udp_poll_s pollinfo[CONFIG_NET_UDP_NPOLLWAITERS]; }; /* This structure supports UDP write buffering. It is simply a container @@ -185,11 +203,6 @@ extern "C" * Public Function Prototypes ****************************************************************************/ -struct sockaddr; /* Forward reference */ -struct socket; /* Forward reference */ -struct net_driver_s; /* Forward reference */ -struct pollfd; /* Forward reference */ - /**************************************************************************** * Name: udp_initialize * @@ -653,9 +666,7 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, * ****************************************************************************/ -#ifdef HAVE_UDP_POLL int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds); -#endif /**************************************************************************** * Name: udp_pollteardown @@ -673,9 +684,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds); * ****************************************************************************/ -#ifdef HAVE_UDP_POLL int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds); -#endif /**************************************************************************** * Name: udp_readahead_notifier_setup diff --git a/net/udp/udp_netpoll.c b/net/udp/udp_netpoll.c index b6fcc85403496..a266b8217c8ad 100644 --- a/net/udp/udp_netpoll.c +++ b/net/udp/udp_netpoll.c @@ -54,25 +54,6 @@ #include "socket/socket.h" #include "udp/udp.h" -#ifdef HAVE_UDP_POLL - -/**************************************************************************** - * Private Types - ****************************************************************************/ - -/* This is an allocated container that holds the poll-related information */ - -struct udp_poll_s -{ - FAR struct socket *psock; /* Needed to handle loss of connection */ - FAR struct net_driver_s *dev; /* Needed to free the callback structure */ - struct pollfd *fds; /* Needed to handle poll events */ - FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ -#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER) - int16_t key; /* Needed to cancel pending notification */ -#endif -}; - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -249,18 +230,22 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) } #endif - /* Allocate a container to hold the poll information */ - - info = (FAR struct udp_poll_s *)kmm_malloc(sizeof(struct udp_poll_s)); - if (!info) - { - return -ENOMEM; - } - /* Some of the following must be atomic */ net_lock(); + /* Find a container to hold the poll information */ + + info = conn->pollinfo; + while (info->psock != NULL) + { + if (++info >= &conn->pollinfo[CONFIG_NET_UDP_NPOLLWAITERS]) + { + ret = -ENOMEM; + goto errout_with_lock; + } + } + /* Get the device that will provide the provide the NETDEV_DOWN event. * NOTE: in the event that the local socket is bound to INADDR_ANY, the * dev value will be zero and there will be no NETDEV_DOWN notifications. @@ -311,6 +296,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) fds->priv = (FAR void *)info; +#ifdef CONFIG_NET_UDP_READAHEAD /* Check for read data availability now */ if (!IOB_QEMPTY(&conn->readahead)) @@ -319,6 +305,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) fds->revents |= (POLLRDNORM & fds->events); } +#endif if (psock_udp_cansend(psock) >= 0) { @@ -350,11 +337,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) } #endif - net_unlock(); - return OK; - errout_with_lock: - kmm_free(info); net_unlock(); return ret; } @@ -408,9 +391,7 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) /* Release the callback */ - net_lock(); udp_callback_free(info->dev, conn, info->cb); - net_unlock(); /* Release the poll/select data slot */ @@ -418,10 +399,8 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) /* Then free the poll info container */ - kmm_free(info); + info->psock = NULL; } return OK; } - -#endif /* !HAVE_UDP_POLL */ diff --git a/net/usrsock/Kconfig b/net/usrsock/Kconfig index 0d965dab35209..a8b1a195b4212 100644 --- a/net/usrsock/Kconfig +++ b/net/usrsock/Kconfig @@ -23,6 +23,10 @@ config NET_USRSOCK_CONNS Note: Usrsock daemon can impose additional restrictions for maximum number of concurrent connections supported. +config NET_USRSOCK_NPOLLWAITERS + int "Number of usrsock poll waiters" + default 1 + config NET_USRSOCK_NO_INET bool "Disable PF_INET for usrsock" default n diff --git a/net/usrsock/usrsock.h b/net/usrsock/usrsock.h index b2f0722662aa6..1bacb9c1384df 100644 --- a/net/usrsock/usrsock.h +++ b/net/usrsock/usrsock.h @@ -86,6 +86,13 @@ enum usrsock_conn_state_e USRSOCK_CONN_STATE_CONNECTING, }; +struct usrsock_poll_s +{ + FAR struct socket *psock; /* Needed to handle loss of connection */ + struct pollfd *fds; /* Needed to handle poll events */ + FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ +}; + struct usrsock_conn_s { /* Common prologue of all connection structures. */ @@ -126,6 +133,12 @@ struct usrsock_conn_s size_t pos; /* Writer position on input buffer */ } datain; } resp; + + /* The following is a list of poll structures of threads waiting for + * socket events. + */ + + struct usrsock_poll_s pollinfo[CONFIG_NET_USRSOCK_NPOLLWAITERS]; }; struct usrsock_reqstate_s diff --git a/net/usrsock/usrsock_poll.c b/net/usrsock/usrsock_poll.c index e1b35c240caee..1e5e6cff0195b 100644 --- a/net/usrsock/usrsock_poll.c +++ b/net/usrsock/usrsock_poll.c @@ -53,21 +53,9 @@ #include #include #include -#include #include "usrsock/usrsock.h" -/**************************************************************************** - * Private Data - ****************************************************************************/ - -struct usrsock_poll_s -{ - FAR struct socket *psock; /* Needed to handle loss of connection */ - struct pollfd *fds; /* Needed to handle poll events */ - FAR struct devif_callback_s *cb; /* Needed to teardown the poll */ -}; - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -182,24 +170,26 @@ static int usrsock_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) } #endif - /* Allocate a container to hold the poll information */ + net_lock(); - info = (FAR struct usrsock_poll_s *) - kmm_malloc(sizeof(struct usrsock_poll_s)); - if (!info) + /* Find a container to hold the poll information */ + + info = conn->pollinfo; + while (info->psock != NULL) { - return -ENOMEM; + if (++info >= &conn->pollinfo[CONFIG_NET_USRSOCK_NPOLLWAITERS]) + { + ret = -ENOMEM; + goto errout_unlock; + } } - net_lock(); - /* Allocate a usrsock callback structure */ cb = devif_callback_alloc(NULL, &conn->list); if (cb == NULL) { ret = -EBUSY; - kmm_free(info); /* fds->priv not set, so we need to free info here. */ goto errout_unlock; } @@ -338,9 +328,7 @@ static int usrsock_pollteardown(FAR struct socket *psock, { /* Release the callback */ - net_lock(); devif_conn_callback_free(NULL, info->cb, &conn->list); - net_unlock(); /* Release the poll/select data slot */ @@ -348,7 +336,7 @@ static int usrsock_pollteardown(FAR struct socket *psock, /* Then free the poll info container */ - kmm_free(info); + info->psock = NULL; } return OK; From dbd12a82cded9f6b8af66dfe96b8915e5ad1cd4d Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Fri, 27 Dec 2019 15:18:43 +0800 Subject: [PATCH 2/5] Put the work notifier into free list to avoid the heap fragment in the long run Since the allocation strategy is encapsulated internally, we can even refine the implementation later. Change-Id: I7f244561058f5504f122af8ab2e77862a7936c70 Signed-off-by: Xiang Xiao --- include/nuttx/wqueue.h | 30 -------------- net/tcp/tcp_netpoll.c | 21 ++-------- net/tcp/tcp_txdrain.c | 20 +--------- net/udp/udp_netpoll.c | 21 ++-------- net/udp/udp_txdrain.c | 20 +--------- sched/wqueue/kwork_notifier.c | 75 ++++++++++++++++++++++++++++++++--- 6 files changed, 79 insertions(+), 108 deletions(-) diff --git a/include/nuttx/wqueue.h b/include/nuttx/wqueue.h index 31867ad1c99ac..42d73f76fa1b0 100644 --- a/include/nuttx/wqueue.h +++ b/include/nuttx/wqueue.h @@ -309,36 +309,6 @@ struct work_notifier_s worker_t worker; /* The worker function to schedule */ }; -/* This structure describes one notification list entry. It is cast- - * compatible with struct work_notifier_s. This structure is an allocated - * container for the user notification data. It is allocated because it - * must persist until the work is executed and must be freed using - * kmm_free() by the work. - * - * With the work notification is scheduled, the work function will receive - * the allocated instance of struct work_notifier_entry_s as its input - * argument. When it completes the notification operation, the work function - * is responsible for freeing that instance. - */ - -struct work_notifier_entry_s -{ - /* This must appear at the beginning of the structure. A reference to - * the struct work_notifier_entry_s instance must be cast-compatible with - * struct dq_entry_s. - */ - - struct work_s work; /* Used for scheduling the work */ - - /* User notification information */ - - struct work_notifier_s info; /* The notification info */ - - /* Additional payload needed to manage the notification */ - - int16_t key; /* Unique ID for the notification */ -}; - /**************************************************************************** * Public Data ****************************************************************************/ diff --git a/net/tcp/tcp_netpoll.c b/net/tcp/tcp_netpoll.c index ca287155c1b88..3890d06609185 100644 --- a/net/tcp/tcp_netpoll.c +++ b/net/tcp/tcp_netpoll.c @@ -41,13 +41,12 @@ #include #include +#include #include #include -#include -#include -#include #include +#include #include "devif/devif.h" #include "netdev/netdev.h" @@ -154,19 +153,11 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev, #if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER) static inline void tcp_iob_work(FAR void *arg) { - FAR struct work_notifier_entry_s *entry; - FAR struct work_notifier_s *ninfo; FAR struct tcp_poll_s *pinfo; FAR struct socket *psock; FAR struct pollfd *fds; - entry = (FAR struct work_notifier_entry_s *)arg; - DEBUGASSERT(entry != NULL); - - ninfo = &entry->info; - DEBUGASSERT(ninfo->arg != NULL); - - pinfo = (FAR struct tcp_poll_s *)ninfo->arg; + pinfo = (FAR struct tcp_poll_s *)arg; DEBUGASSERT(pinfo->psock != NULL && pinfo->fds != NULL); psock = pinfo->psock; @@ -213,12 +204,6 @@ static inline void tcp_iob_work(FAR void *arg) pinfo->key = iob_notifier_setup(LPWORK, tcp_iob_work, pinfo); } } - - /* Protocol for the use of the IOB notifier is that we free the argument - * after the notification has been processed. - */ - - kmm_free(arg); } #endif diff --git a/net/tcp/tcp_txdrain.c b/net/tcp/tcp_txdrain.c index 552c6312830fa..9191259a4c929 100644 --- a/net/tcp/tcp_txdrain.c +++ b/net/tcp/tcp_txdrain.c @@ -46,10 +46,6 @@ #include #include -#include -#include -#include - #include "tcp/tcp.h" #if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_TCP_NOTIFIER) @@ -74,21 +70,9 @@ static void txdrain_worker(FAR void *arg) { - /* The entire notifier entry is passed to us. That is because we are - * responsible for disposing of the entry via kmm_free() when we are - * finished with it. - */ - - FAR struct work_notifier_entry_s *notifier = - (FAR struct work_notifier_entry_s *)arg; - FAR sem_t *waitsem; - - DEBUGASSERT(notifier != NULL && notifier->info.arg != NULL); - waitsem = (FAR sem_t *)notifier->info.arg; - - /* Free the notifier entry */ + FAR sem_t *waitsem = (FAR sem_t *)arg; - kmm_free(notifier); + DEBUGASSERT(waitsem != NULL); /* Then just post the semaphore, waking up tcp_txdrain() */ diff --git a/net/udp/udp_netpoll.c b/net/udp/udp_netpoll.c index a266b8217c8ad..0bf042b35cae3 100644 --- a/net/udp/udp_netpoll.c +++ b/net/udp/udp_netpoll.c @@ -41,13 +41,12 @@ #include #include +#include #include #include -#include -#include -#include #include +#include #include "devif/devif.h" #include "netdev/netdev.h" @@ -144,19 +143,11 @@ static uint16_t udp_poll_eventhandler(FAR struct net_driver_s *dev, #if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER) static inline void udp_iob_work(FAR void *arg) { - FAR struct work_notifier_entry_s *entry; - FAR struct work_notifier_s *ninfo; FAR struct udp_poll_s *pinfo; FAR struct socket *psock; FAR struct pollfd *fds; - entry = (FAR struct work_notifier_entry_s *)arg; - DEBUGASSERT(entry != NULL); - - ninfo = &entry->info; - DEBUGASSERT(ninfo->arg != NULL); - - pinfo = (FAR struct udp_poll_s *)ninfo->arg; + pinfo = (FAR struct udp_poll_s *)arg; DEBUGASSERT(pinfo->psock != NULL && pinfo->fds != NULL); psock = pinfo->psock; @@ -185,12 +176,6 @@ static inline void udp_iob_work(FAR void *arg) pinfo->key = iob_notifier_setup(LPWORK, udp_iob_work, pinfo); } } - - /* Protocol for the use of the IOB notifier is that we free the argument - * after the notification has been processed. - */ - - kmm_free(arg); } #endif diff --git a/net/udp/udp_txdrain.c b/net/udp/udp_txdrain.c index df899f6172926..1c53a1422d1e5 100644 --- a/net/udp/udp_txdrain.c +++ b/net/udp/udp_txdrain.c @@ -46,10 +46,6 @@ #include #include -#include -#include -#include - #include "udp/udp.h" #if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_UDP_NOTIFIER) @@ -74,21 +70,9 @@ static void txdrain_worker(FAR void *arg) { - /* The entire notifier entry is passed to us. That is because we are - * responsible for disposing of the entry via kmm_free() when we are - * finished with it. - */ - - FAR struct work_notifier_entry_s *notifier = - (FAR struct work_notifier_entry_s *)arg; - FAR sem_t *waitsem; - - DEBUGASSERT(notifier != NULL && notifier->info.arg != NULL); - waitsem = (FAR sem_t *)notifier->info.arg; - - /* Free the notifier entry */ + FAR sem_t *waitsem = (FAR sem_t *)arg; - kmm_free(notifier); + DEBUGASSERT(waitsem != NULL); /* Then just post the semaphore, waking up tcp_txdrain() */ diff --git a/sched/wqueue/kwork_notifier.c b/sched/wqueue/kwork_notifier.c index 706aed4127509..66bc5731a6393 100644 --- a/sched/wqueue/kwork_notifier.c +++ b/sched/wqueue/kwork_notifier.c @@ -55,10 +55,42 @@ #ifdef CONFIG_WQUEUE_NOTIFIER +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/* This structure describes one notification list entry. It is cast- + * compatible with struct work_notifier_s. This structure is an allocated + * container for the user notification data. It is allocated because it + * must persist until the work is executed. + */ + +struct work_notifier_entry_s +{ + /* This must appear at the beginning of the structure. A reference to + * the struct work_notifier_entry_s instance must be cast-compatible with + * struct dq_entry_s. + */ + + struct work_s work; /* Used for scheduling the work */ + + /* User notification information */ + + struct work_notifier_s info; /* The notification info */ + + /* Additional payload needed to manage the notification */ + + int16_t key; /* Unique ID for the notification */ +}; + /**************************************************************************** * Private Data ****************************************************************************/ +/* This is a doubly linked list of free notifications. */ + +static dq_queue_t g_notifier_free; + /* This is a doubly linked list of pending notifications. When an event * occurs available, *all* of the waiters for that event in this list will * be notified and the entry will be freed. If there are multiple waiters @@ -146,6 +178,30 @@ static int16_t work_notifier_key(void) return key; } +/**************************************************************************** + * Name: work_notifier_worker + * + * Description: + * Forward to the real worker and free the notification. + * + ****************************************************************************/ + +static void work_notifier_worker(FAR void *arg) +{ + FAR struct work_notifier_entry_s *notifier = + (FAR struct work_notifier_entry_s *)arg; + + /* Forward to the real worker */ + + notifier->info.worker(notifier->info.arg); + + /* Put the notification to the free list */ + + while (nxsem_wait(&g_notifier_sem) < 0); + dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_free); + nxsem_post(&g_notifier_sem); +} + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -185,9 +241,16 @@ int work_notifier_setup(FAR struct work_notifier_s *info) return ret; } - /* Allocate a new notification entry */ + /* Try to get the entry from the free list */ + + notifier = (FAR struct work_notifier_entry_s *)dq_remfirst(&g_notifier_free); + if (notifier == NULL) + { + /* Allocate a new notification entry */ + + notifier = kmm_malloc(sizeof(struct work_notifier_entry_s)); + } - notifier = kmm_malloc(sizeof(struct work_notifier_entry_s)); if (notifier == NULL) { ret = -ENOMEM; @@ -269,9 +332,9 @@ int work_notifier_teardown(int key) dq_rem((FAR dq_entry_t *)notifier, &g_notifier_pending); - /* Free the notification */ + /* Put the notification to the free list */ - kmm_free(notifier); + dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_free); ret = OK; } @@ -359,8 +422,8 @@ void work_notifier_signal(enum work_evtype_e evtype, * responsible for freeing the allocated memory. */ - (void)work_queue(info->qid, ¬ifier->work, info->worker, - entry, 0); + (void)work_queue(info->qid, ¬ifier->work, + work_notifier_worker, entry, 0); } } From 26ecb061449b3ae1e921a4567dbf4c81daab6053 Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Thu, 26 Dec 2019 18:06:36 +0800 Subject: [PATCH 3/5] Fix the semaphore usage issue found in tcp/udp 1.The count semaphore need disable priority inheritance 2.Loop again if net_lockedwait return -EINTR 3.Call nxsem_trywait to avoid the race condition 4.Call nxsem_post instead of sem_post Change-Id: I0f9d2ded208f419746c5ae9d38ffe248dfa593a2 Signed-off-by: Xiang Xiao --- net/tcp/tcp_txdrain.c | 3 ++- net/tcp/tcp_wrbuffer.c | 9 +++------ net/udp/udp_txdrain.c | 3 ++- net/udp/udp_wrbuffer.c | 3 ++- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/net/tcp/tcp_txdrain.c b/net/tcp/tcp_txdrain.c index 9191259a4c929..2b3c7ddf55166 100644 --- a/net/tcp/tcp_txdrain.c +++ b/net/tcp/tcp_txdrain.c @@ -76,7 +76,7 @@ static void txdrain_worker(FAR void *arg) /* Then just post the semaphore, waking up tcp_txdrain() */ - sem_post(waitsem); + nxsem_post(waitsem); } /**************************************************************************** @@ -114,6 +114,7 @@ int tcp_txdrain(FAR struct socket *psock, /* Initialize the wait semaphore */ nxsem_init(&waitsem, 0, 0); + nxsem_setprotocol(&waitsem, SEM_PRIO_NONE); /* The following needs to be done with the network stable */ diff --git a/net/tcp/tcp_wrbuffer.c b/net/tcp/tcp_wrbuffer.c index 285d13e268198..44dcad2612287 100644 --- a/net/tcp/tcp_wrbuffer.c +++ b/net/tcp/tcp_wrbuffer.c @@ -119,6 +119,7 @@ void tcp_wrbuffer_initialize(void) } nxsem_init(&g_wrbuffer.sem, 0, CONFIG_NET_TCP_NWRBCHAINS); + nxsem_setprotocol(&g_wrbuffer.sem, SEM_PRIO_NONE); } /**************************************************************************** @@ -149,7 +150,7 @@ FAR struct tcp_wrbuffer_s *tcp_wrbuffer_alloc(void) * buffer */ - DEBUGVERIFY(net_lockedwait(&g_wrbuffer.sem)); /* TODO: Handle EINTR. */ + while (net_lockedwait(&g_wrbuffer.sem) < 0); /* Now, we are guaranteed to have a write buffer structure reserved * for us in the free list. @@ -207,11 +208,7 @@ FAR struct tcp_wrbuffer_s *tcp_wrbuffer_tryalloc(void) * buffer */ - if (tcp_wrbuffer_test() == OK) - { - DEBUGVERIFY(net_lockedwait(&g_wrbuffer.sem)); - } - else + if (nxsem_trywait(&g_wrbuffer.sem) != OK) { return NULL; } diff --git a/net/udp/udp_txdrain.c b/net/udp/udp_txdrain.c index 1c53a1422d1e5..7952aa935b385 100644 --- a/net/udp/udp_txdrain.c +++ b/net/udp/udp_txdrain.c @@ -76,7 +76,7 @@ static void txdrain_worker(FAR void *arg) /* Then just post the semaphore, waking up tcp_txdrain() */ - sem_post(waitsem); + nxsem_post(waitsem); } /**************************************************************************** @@ -114,6 +114,7 @@ int udp_txdrain(FAR struct socket *psock, /* Initialize the wait semaphore */ nxsem_init(&waitsem, 0, 0); + nxsem_setprotocol(&waitsem, SEM_PRIO_NONE); /* The following needs to be done with the network stable */ diff --git a/net/udp/udp_wrbuffer.c b/net/udp/udp_wrbuffer.c index 399da0de7ea0a..b380c591b88fb 100644 --- a/net/udp/udp_wrbuffer.c +++ b/net/udp/udp_wrbuffer.c @@ -116,6 +116,7 @@ void udp_wrbuffer_initialize(void) } nxsem_init(&g_wrbuffer.sem, 0, CONFIG_NET_UDP_NWRBCHAINS); + nxsem_setprotocol(&g_wrbuffer.sem, SEM_PRIO_NONE); } /**************************************************************************** @@ -146,7 +147,7 @@ FAR struct udp_wrbuffer_s *udp_wrbuffer_alloc(void) * buffer */ - DEBUGVERIFY(net_lockedwait(&g_wrbuffer.sem)); /* TODO: Handle EINTR. */ + while (net_lockedwait(&g_wrbuffer.sem) < 0); /* Now, we are guaranteed to have a write buffer structure reserved * for us in the free list. From b9bc7477015080ebd0a2c31e630af4299c83c81b Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Mon, 30 Dec 2019 12:16:40 +0800 Subject: [PATCH 4/5] Add NET_ prefix for UDP_NOTIFIER and TCP_NOTIFIER option to align with other UDP/TCP option convention Change-Id: I788da5f2a3f95abdec617d64440242160d5e16ad Signed-off-by: Xiang Xiao --- .../lpc4088-devkit/configs/nsh/defconfig | 2 +- net/socket/Kconfig | 4 ++-- net/tcp/Kconfig | 2 +- net/tcp/Make.defs | 2 +- net/tcp/tcp.h | 18 +++++++++--------- net/tcp/tcp_callback.c | 6 +++--- net/tcp/tcp_notifier.c | 4 ++-- net/tcp/tcp_send_buffered.c | 2 +- net/tcp/tcp_txdrain.c | 4 ++-- net/udp/Kconfig | 6 +++--- net/udp/Make.defs | 2 +- net/udp/udp.h | 14 +++++++------- net/udp/udp_callback.c | 2 +- net/udp/udp_notifier.c | 4 ++-- net/udp/udp_psock_sendto_buffered.c | 2 +- net/udp/udp_txdrain.c | 4 ++-- 16 files changed, 39 insertions(+), 39 deletions(-) diff --git a/boards/arm/lpc17xx_40xx/lpc4088-devkit/configs/nsh/defconfig b/boards/arm/lpc17xx_40xx/lpc4088-devkit/configs/nsh/defconfig index f57a24cd9a796..01b2c0d3988f8 100644 --- a/boards/arm/lpc17xx_40xx/lpc4088-devkit/configs/nsh/defconfig +++ b/boards/arm/lpc17xx_40xx/lpc4088-devkit/configs/nsh/defconfig @@ -57,6 +57,7 @@ CONFIG_NET_BROADCAST=y CONFIG_NET_ICMP=y CONFIG_NET_SOCKOPTS=y CONFIG_NET_TCP=y +CONFIG_NET_TCP_NOTIFIER=y CONFIG_NET_TCP_WRITE_BUFFERS=y CONFIG_NET_UDP=y CONFIG_NFILE_DESCRIPTORS=8 @@ -90,7 +91,6 @@ CONFIG_SYMTAB_ORDEREDBYNAME=y CONFIG_SYSTEM_MDIO=y CONFIG_SYSTEM_NSH=y CONFIG_TASK_NAME_SIZE=0 -CONFIG_TCP_NOTIFIER=y CONFIG_UART0_SERIAL_CONSOLE=y CONFIG_USERMAIN_STACKSIZE=4096 CONFIG_USER_ENTRYPOINT="nsh_main" diff --git a/net/socket/Kconfig b/net/socket/Kconfig index a22948bd6ab79..c627ea551fe01 100644 --- a/net/socket/Kconfig +++ b/net/socket/Kconfig @@ -43,8 +43,8 @@ config NET_SOLINGER bool "SO_LINGER socket option" default n depends on NET_TCP_WRITE_BUFFERS || NET_UDP_WRITE_BUFFERS - select TCP_NOTIFIER if NET_TCP - select UDP_NOTIFIER if NET_UDP + select NET_TCP_NOTIFIER if NET_TCP + select NET_UDP_NOTIFIER if NET_UDP ---help--- Enable or disable support for the SO_LINGER socket option. Requires write buffer support. diff --git a/net/tcp/Kconfig b/net/tcp/Kconfig index ca428058ac3a8..9319c62bc968a 100644 --- a/net/tcp/Kconfig +++ b/net/tcp/Kconfig @@ -78,7 +78,7 @@ config NET_MAX_LISTENPORTS ---help--- Maximum number of listening TCP/IP ports (all tasks). Default: 20 -config TCP_NOTIFIER +config NET_TCP_NOTIFIER bool "Support TCP notifications" default n depends on SCHED_WORKQUEUE diff --git a/net/tcp/Make.defs b/net/tcp/Make.defs index 5eec9e37c67d6..4cc27dbb95dfc 100644 --- a/net/tcp/Make.defs +++ b/net/tcp/Make.defs @@ -52,7 +52,7 @@ ifeq ($(CONFIG_NET_SENDFILE),y) SOCK_CSRCS += tcp_sendfile.c endif -ifeq ($(CONFIG_TCP_NOTIFIER),y) +ifeq ($(CONFIG_NET_TCP_NOTIFIER),y) SOCK_CSRCS += tcp_notifier.c ifeq ($(CONFIG_NET_TCP_WRITE_BUFFERS),y) SOCK_CSRCS += tcp_txdrain.c diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index b242a9a18b75b..f786d3dd36ef2 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -49,7 +49,7 @@ #include #include -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER # include #endif @@ -1594,7 +1594,7 @@ int tcp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds); * ****************************************************************************/ -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER int tcp_readahead_notifier_setup(worker_t worker, FAR struct tcp_conn_s *conn, FAR void *arg); @@ -1627,7 +1627,7 @@ int tcp_readahead_notifier_setup(worker_t worker, * ****************************************************************************/ -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER int tcp_writebuffer_notifier_setup(worker_t worker, FAR struct tcp_conn_s *conn, FAR void *arg); @@ -1658,7 +1658,7 @@ int tcp_writebuffer_notifier_setup(worker_t worker, * ****************************************************************************/ -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER int tcp_disconnect_notifier_setup(worker_t worker, FAR struct tcp_conn_s *conn, FAR void *arg); @@ -1683,7 +1683,7 @@ int tcp_disconnect_notifier_setup(worker_t worker, * ****************************************************************************/ -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER int tcp_notifier_teardown(int key); #endif @@ -1708,7 +1708,7 @@ int tcp_notifier_teardown(int key); * ****************************************************************************/ -#if defined(CONFIG_NET_TCP_READAHEAD) && defined(CONFIG_TCP_NOTIFIER) +#if defined(CONFIG_NET_TCP_READAHEAD) && defined(CONFIG_NET_TCP_NOTIFIER) void tcp_readahead_signal(FAR struct tcp_conn_s *conn); #endif @@ -1733,7 +1733,7 @@ void tcp_readahead_signal(FAR struct tcp_conn_s *conn); * ****************************************************************************/ -#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_TCP_NOTIFIER) +#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_NET_TCP_NOTIFIER) void tcp_writebuffer_signal(FAR struct tcp_conn_s *conn); #endif @@ -1752,7 +1752,7 @@ void tcp_writebuffer_signal(FAR struct tcp_conn_s *conn); * ****************************************************************************/ -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER void tcp_disconnect_signal(FAR struct tcp_conn_s *conn); #endif @@ -1772,7 +1772,7 @@ void tcp_disconnect_signal(FAR struct tcp_conn_s *conn); * ****************************************************************************/ -#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_TCP_NOTIFIER) +#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_NET_TCP_NOTIFIER) struct timespec; int tcp_txdrain(FAR struct socket *psock, FAR const struct timespec *abstime); diff --git a/net/tcp/tcp_callback.c b/net/tcp/tcp_callback.c index d04a2c93b423c..c9f8c55d9209f 100644 --- a/net/tcp/tcp_callback.c +++ b/net/tcp/tcp_callback.c @@ -145,7 +145,7 @@ tcp_data_event(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn, uint16_t tcp_callback(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn, uint16_t flags) { -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER uint16_t orig = flags; #endif @@ -203,7 +203,7 @@ uint16_t tcp_callback(FAR struct net_driver_s *dev, flags = devif_conn_event(dev, conn, flags, conn->connevents); } -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER /* Provide notification(s) if the TCP connection has been lost. */ if ((orig & TCP_DISCONN_EVENTS) != 0) @@ -287,7 +287,7 @@ uint16_t tcp_datahandler(FAR struct tcp_conn_s *conn, FAR uint8_t *buffer, return 0; } -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER /* Provide notification(s) that additional TCP read-ahead data is * available. */ diff --git a/net/tcp/tcp_notifier.c b/net/tcp/tcp_notifier.c index d106b5adc0a7e..23673cfef536d 100644 --- a/net/tcp/tcp_notifier.c +++ b/net/tcp/tcp_notifier.c @@ -49,7 +49,7 @@ #include "tcp/tcp.h" -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER /**************************************************************************** * Public Functions @@ -330,4 +330,4 @@ void tcp_disconnect_signal(FAR struct tcp_conn_s *conn) return work_notifier_signal(WORK_TCP_DISCONNECT, conn); } -#endif /* CONFIG_TCP_NOTIFIER */ +#endif /* CONFIG_NET_TCP_NOTIFIER */ diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index d6ca21bc03f32..0a888bbbc392a 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -173,7 +173,7 @@ static void psock_insert_segment(FAR struct tcp_wrbuffer_s *wrb, * ****************************************************************************/ -#ifdef CONFIG_TCP_NOTIFIER +#ifdef CONFIG_NET_TCP_NOTIFIER static void psock_writebuffer_notify(FAR struct tcp_conn_s *conn) { /* Check if all write buffers have been sent and ACKed */ diff --git a/net/tcp/tcp_txdrain.c b/net/tcp/tcp_txdrain.c index 2b3c7ddf55166..b3fd7220f0ba8 100644 --- a/net/tcp/tcp_txdrain.c +++ b/net/tcp/tcp_txdrain.c @@ -48,7 +48,7 @@ #include "tcp/tcp.h" -#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_TCP_NOTIFIER) +#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_NET_TCP_NOTIFIER) /**************************************************************************** * Private Functions @@ -180,4 +180,4 @@ int tcp_txdrain(FAR struct socket *psock, return ret; } -#endif /* CONFIG_NET_TCP_WRITE_BUFFERS && CONFIG_TCP_NOTIFIER */ +#endif /* CONFIG_NET_TCP_WRITE_BUFFERS && CONFIG_NET_TCP_NOTIFIER */ diff --git a/net/udp/Kconfig b/net/udp/Kconfig index 45ddcd4de9e52..095d86af754a8 100644 --- a/net/udp/Kconfig +++ b/net/udp/Kconfig @@ -113,15 +113,15 @@ config NET_UDP_WRBUFFER_DUMP endif # NET_UDP_WRITE_BUFFERS -config UDP_NOTIFIER +config NET_UDP_NOTIFIER bool "Support UDP read-ahead notifications" default n depends on SCHED_WORKQUEUE select WQUEUE_NOTIFIER ---help--- Enable building of UDP read-ahead notifier logic that will execute a - worker function on the high priority work queue when read-ahead data - is available. This is is a general purpose notifier, but was + worker function on the low priority work queue when read-ahead data + is available. This is a general purpose notifier, but was developed specifically to support poll() logic where the poll must wait for read-ahead data to become available. diff --git a/net/udp/Make.defs b/net/udp/Make.defs index 541b336d19b88..edb0e31b26669 100644 --- a/net/udp/Make.defs +++ b/net/udp/Make.defs @@ -52,7 +52,7 @@ else SOCK_CSRCS += udp_psock_sendto_unbuffered.c endif -ifeq ($(CONFIG_UDP_NOTIFIER),y) +ifeq ($(CONFIG_NET_UDP_NOTIFIER),y) SOCK_CSRCS += udp_notifier.c ifeq ($(CONFIG_NET_UDP_WRITE_BUFFERS),y) SOCK_CSRCS += udp_txdrain.c diff --git a/net/udp/udp.h b/net/udp/udp.h index b497b2a5c4b4d..2c95052c79c27 100644 --- a/net/udp/udp.h +++ b/net/udp/udp.h @@ -53,7 +53,7 @@ # include #endif -#ifdef CONFIG_UDP_NOTIFIER +#ifdef CONFIG_NET_UDP_NOTIFIER # include #endif @@ -712,7 +712,7 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds); * ****************************************************************************/ -#ifdef CONFIG_UDP_NOTIFIER +#ifdef CONFIG_NET_UDP_NOTIFIER int udp_readahead_notifier_setup(worker_t worker, FAR struct udp_conn_s *conn, FAR void *arg); @@ -744,7 +744,7 @@ int udp_readahead_notifier_setup(worker_t worker, * ****************************************************************************/ -#ifdef CONFIG_UDP_NOTIFIER +#ifdef CONFIG_NET_UDP_NOTIFIER int udp_writebuffer_notifier_setup(worker_t worker, FAR struct udp_conn_s *conn, FAR void *arg); @@ -769,7 +769,7 @@ int udp_writebuffer_notifier_setup(worker_t worker, * ****************************************************************************/ -#ifdef CONFIG_UDP_NOTIFIER +#ifdef CONFIG_NET_UDP_NOTIFIER int udp_notifier_teardown(int key); #endif @@ -793,7 +793,7 @@ int udp_notifier_teardown(int key); * ****************************************************************************/ -#if defined(CONFIG_NET_UDP_READAHEAD) && defined(CONFIG_UDP_NOTIFIER) +#if defined(CONFIG_NET_UDP_READAHEAD) && defined(CONFIG_NET_UDP_NOTIFIER) void udp_readahead_signal(FAR struct udp_conn_s *conn); #endif @@ -818,7 +818,7 @@ void udp_readahead_signal(FAR struct udp_conn_s *conn); * ****************************************************************************/ -#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_UDP_NOTIFIER) +#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_NET_UDP_NOTIFIER) void udp_writebuffer_signal(FAR struct udp_conn_s *conn); #endif @@ -838,7 +838,7 @@ void udp_writebuffer_signal(FAR struct udp_conn_s *conn); * ****************************************************************************/ -#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_UDP_NOTIFIER) +#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_NET_UDP_NOTIFIER) struct timespec; int udp_txdrain(FAR struct socket *psock, FAR const struct timespec *abstime); diff --git a/net/udp/udp_callback.c b/net/udp/udp_callback.c index e6b38b9598077..3343f40d0e1f0 100644 --- a/net/udp/udp_callback.c +++ b/net/udp/udp_callback.c @@ -234,7 +234,7 @@ static uint16_t udp_datahandler(FAR struct net_driver_s *dev, return 0; } -#ifdef CONFIG_UDP_NOTIFIER +#ifdef CONFIG_NET_UDP_NOTIFIER /* Provided notification(s) that additional UDP read-ahead data is * available. */ diff --git a/net/udp/udp_notifier.c b/net/udp/udp_notifier.c index a9919cf3f05a7..e2c909a2fc4bf 100644 --- a/net/udp/udp_notifier.c +++ b/net/udp/udp_notifier.c @@ -47,7 +47,7 @@ #include "udp/udp.h" -#ifdef CONFIG_UDP_NOTIFIER +#ifdef CONFIG_NET_UDP_NOTIFIER /**************************************************************************** * Public Functions @@ -254,4 +254,4 @@ void udp_writebuffer_signal(FAR struct udp_conn_s *conn) } #endif -#endif /* CONFIG_UDP_NOTIFIER */ +#endif /* CONFIG_NET_UDP_NOTIFIER */ diff --git a/net/udp/udp_psock_sendto_buffered.c b/net/udp/udp_psock_sendto_buffered.c index 73d8f446c0c4b..46b9bb9da2b72 100644 --- a/net/udp/udp_psock_sendto_buffered.c +++ b/net/udp/udp_psock_sendto_buffered.c @@ -162,7 +162,7 @@ static void sendto_writebuffer_release(FAR struct socket *psock, psock->s_sndcb->event = NULL; wrb = NULL; -#ifdef CONFIG_UDP_NOTIFIER +#ifdef CONFIG_NET_UDP_NOTIFIER /* Notify any waiters that the write buffers have been drained. */ udp_writebuffer_signal(conn); diff --git a/net/udp/udp_txdrain.c b/net/udp/udp_txdrain.c index 7952aa935b385..ed8f2e5841385 100644 --- a/net/udp/udp_txdrain.c +++ b/net/udp/udp_txdrain.c @@ -48,7 +48,7 @@ #include "udp/udp.h" -#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_UDP_NOTIFIER) +#if defined(CONFIG_NET_UDP_WRITE_BUFFERS) && defined(CONFIG_NET_UDP_NOTIFIER) /**************************************************************************** * Private Functions @@ -145,4 +145,4 @@ int udp_txdrain(FAR struct socket *psock, return ret; } -#endif /* CONFIG_NET_UDP_WRITE_BUFFERS && CONFIG_UDP_NOTIFIER */ +#endif /* CONFIG_NET_UDP_WRITE_BUFFERS && CONFIG_NET_UDP_NOTIFIER */ From d7c7ed7f88fff6c59cc30a8fbe6d06f839dcd85a Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Tue, 31 Dec 2019 20:13:32 +0800 Subject: [PATCH 5/5] Remove the unused _SF_[IDLE|ACCEPT|SEND|RECV|MASK] flags since there are code to set/clear these flags, but nobody check them. Change-Id: I8fb2d5a608c197d1141716b72a7d03069504fcd8 Signed-off-by: Xiang Xiao --- net/bluetooth/bluetooth_recvfrom.c | 7 ------- net/bluetooth/bluetooth_sendto.c | 8 -------- net/ieee802154/ieee802154_recvfrom.c | 7 ------- net/ieee802154/ieee802154_sendto.c | 8 -------- net/pkt/pkt_recvfrom.c | 8 -------- net/pkt/pkt_send.c | 8 -------- net/sixlowpan/sixlowpan_tcpsend.c | 9 --------- net/sixlowpan/sixlowpan_udpsend.c | 7 ------- net/socket/recvfrom.c | 13 +------------ net/socket/socket.h | 11 ----------- net/tcp/tcp_accept.c | 8 -------- net/tcp/tcp_send_buffered.c | 8 -------- net/tcp/tcp_send_unbuffered.c | 8 -------- net/tcp/tcp_sendfile.c | 8 -------- net/udp/udp_psock_sendto_buffered.c | 8 -------- net/udp/udp_psock_sendto_unbuffered.c | 8 -------- 16 files changed, 1 insertion(+), 133 deletions(-) diff --git a/net/bluetooth/bluetooth_recvfrom.c b/net/bluetooth/bluetooth_recvfrom.c index 9de4764652e32..cb781e3f53c29 100644 --- a/net/bluetooth/bluetooth_recvfrom.c +++ b/net/bluetooth/bluetooth_recvfrom.c @@ -387,10 +387,6 @@ ssize_t bluetooth_recvfrom(FAR struct socket *psock, FAR void *buf, (void)nxsem_init(&state.ir_sem, 0, 0); /* Doesn't really fail */ (void)nxsem_setprotocol(&state.ir_sem, SEM_PRIO_NONE); - /* Set the socket state to receiving */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_RECV); - /* Set up the callback in the connection */ state.ir_cb = bluetooth_callback_alloc(&radio->r_dev, conn); @@ -418,9 +414,6 @@ ssize_t bluetooth_recvfrom(FAR struct socket *psock, FAR void *buf, ret = -EBUSY; } - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); nxsem_destroy(&state.ir_sem); errout_with_lock: diff --git a/net/bluetooth/bluetooth_sendto.c b/net/bluetooth/bluetooth_sendto.c index bdfa90a8e7f2c..03ec6a2ad543b 100644 --- a/net/bluetooth/bluetooth_sendto.c +++ b/net/bluetooth/bluetooth_sendto.c @@ -283,10 +283,6 @@ ssize_t psock_bluetooth_sendto(FAR struct socket *psock, FAR const void *buf, return -ENODEV; } - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* Perform the send operation */ /* Initialize the state structure. This is done with the network locked @@ -345,10 +341,6 @@ ssize_t psock_bluetooth_sendto(FAR struct socket *psock, FAR const void *buf, nxsem_destroy(&state.is_sem); net_unlock(); - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Check for a errors, Errors are signaled by negative errno values * for the send length */ diff --git a/net/ieee802154/ieee802154_recvfrom.c b/net/ieee802154/ieee802154_recvfrom.c index bdb50a5c59afb..2ab12fb3ef3b6 100644 --- a/net/ieee802154/ieee802154_recvfrom.c +++ b/net/ieee802154/ieee802154_recvfrom.c @@ -385,10 +385,6 @@ ssize_t ieee802154_recvfrom(FAR struct socket *psock, FAR void *buf, (void)nxsem_init(&state.ir_sem, 0, 0); /* Doesn't really fail */ (void)nxsem_setprotocol(&state.ir_sem, SEM_PRIO_NONE); - /* Set the socket state to receiving */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_RECV); - /* Set up the callback in the connection */ state.ir_cb = ieee802154_callback_alloc(&radio->r_dev, conn); @@ -416,9 +412,6 @@ ssize_t ieee802154_recvfrom(FAR struct socket *psock, FAR void *buf, ret = -EBUSY; } - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); nxsem_destroy(&state.ir_sem); errout_with_lock: diff --git a/net/ieee802154/ieee802154_sendto.c b/net/ieee802154/ieee802154_sendto.c index 7556f234a0e47..66c35d73cd885 100644 --- a/net/ieee802154/ieee802154_sendto.c +++ b/net/ieee802154/ieee802154_sendto.c @@ -471,10 +471,6 @@ ssize_t psock_ieee802154_sendto(FAR struct socket *psock, FAR const void *buf, return -ENODEV; } - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* Perform the send operation */ /* Initialize the state structure. This is done with the network @@ -534,10 +530,6 @@ ssize_t psock_ieee802154_sendto(FAR struct socket *psock, FAR const void *buf, nxsem_destroy(&state.is_sem); net_unlock(); - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Check for a errors, Errors are signaled by negative errno values * for the send length */ diff --git a/net/pkt/pkt_recvfrom.c b/net/pkt/pkt_recvfrom.c index a859083e23f96..8b8a5106ed683 100644 --- a/net/pkt/pkt_recvfrom.c +++ b/net/pkt/pkt_recvfrom.c @@ -410,10 +410,6 @@ ssize_t pkt_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len, } #endif - /* Set the socket state to receiving */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_RECV); - /* Set up the callback in the connection */ state.pr_cb = pkt_callback_alloc(dev, conn); @@ -441,10 +437,6 @@ ssize_t pkt_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len, ret = -EBUSY; } - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - errout_with_state: net_unlock(); pkt_recvfrom_uninitialize(&state); diff --git a/net/pkt/pkt_send.c b/net/pkt/pkt_send.c index cd0ec8e7253fe..aec0e724db452 100644 --- a/net/pkt/pkt_send.c +++ b/net/pkt/pkt_send.c @@ -191,10 +191,6 @@ ssize_t psock_pkt_send(FAR struct socket *psock, FAR const void *buf, return -ENODEV; } - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* Perform the send operation */ /* Initialize the state structure. This is done with the network locked @@ -249,10 +245,6 @@ ssize_t psock_pkt_send(FAR struct socket *psock, FAR const void *buf, nxsem_destroy(&state.snd_sem); net_unlock(); - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Check for a errors, Errors are signalled by negative errno values * for the send length */ diff --git a/net/sixlowpan/sixlowpan_tcpsend.c b/net/sixlowpan/sixlowpan_tcpsend.c index 8a86a7bfc2d61..67d83e4bb7fca 100644 --- a/net/sixlowpan/sixlowpan_tcpsend.c +++ b/net/sixlowpan/sixlowpan_tcpsend.c @@ -854,10 +854,6 @@ ssize_t psock_6lowpan_tcp_send(FAR struct socket *psock, FAR const void *buf, return (ssize_t)ret; } - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* Send the TCP packets, breaking down the potential large user buffer * into smaller packets that can be reassembled in the allocated MTU * packet buffer. @@ -874,14 +870,9 @@ ssize_t psock_6lowpan_tcp_send(FAR struct socket *psock, FAR const void *buf, if (ret < 0) { nerr("ERROR: sixlowpan_send_packet() failed: %d\n", ret); - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); return (ssize_t)ret; } - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); return (ssize_t)buflen; } diff --git a/net/sixlowpan/sixlowpan_udpsend.c b/net/sixlowpan/sixlowpan_udpsend.c index 56684d7117fea..ffb62bd44d411 100644 --- a/net/sixlowpan/sixlowpan_udpsend.c +++ b/net/sixlowpan/sixlowpan_udpsend.c @@ -302,10 +302,6 @@ ssize_t psock_6lowpan_udp_sendto(FAR struct socket *psock, return (ssize_t)ret; } - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* If routable, then call sixlowpan_send() to format and send the 6LoWPAN * packet. */ @@ -324,9 +320,6 @@ ssize_t psock_6lowpan_udp_sendto(FAR struct socket *psock, nerr("ERROR: sixlowpan_send() failed: %d\n", ret); } - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); return (ssize_t)ret; } diff --git a/net/socket/recvfrom.c b/net/socket/recvfrom.c index 79944c21b513b..60a95352e118d 100644 --- a/net/socket/recvfrom.c +++ b/net/socket/recvfrom.c @@ -88,8 +88,6 @@ ssize_t psock_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len, int flags, FAR struct sockaddr *from, FAR socklen_t *fromlen) { - ssize_t ret; - /* Verify that non-NULL pointers were passed */ #ifdef CONFIG_DEBUG_FEATURES @@ -111,10 +109,6 @@ ssize_t psock_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len, return -EBADF; } - /* Set the socket state to receiving */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_RECV); - /* Let logic specific to this address family handle the recvfrom() * operation. */ @@ -122,12 +116,7 @@ ssize_t psock_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len, DEBUGASSERT(psock->s_sockif != NULL && psock->s_sockif->si_recvfrom != NULL); - ret = psock->s_sockif->si_recvfrom(psock, buf, len, flags, from, fromlen); - - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - return ret; + return psock->s_sockif->si_recvfrom(psock, buf, len, flags, from, fromlen); } /**************************************************************************** diff --git a/net/socket/socket.h b/net/socket/socket.h index f826337baed34..7f7abe96ce2e0 100644 --- a/net/socket/socket.h +++ b/net/socket/socket.h @@ -56,13 +56,6 @@ /* Definitions of 8-bit socket flags */ - /* Bits 0-2: Socket state */ -#define _SF_IDLE 0x00 /* - There is no socket activity */ -#define _SF_ACCEPT 0x01 /* - Socket is waiting to accept a connection */ -#define _SF_RECV 0x02 /* - Waiting for recv action to complete */ -#define _SF_SEND 0x03 /* - Waiting for send action to complete */ -#define _SF_MASK 0x03 /* - Mask to isolate the above actions */ - #define _SF_NONBLOCK 0x08 /* Bit 3: Don't block if no data (TCP/READ only) */ #define _SF_LISTENING 0x10 /* Bit 4: SOCK_STREAM is listening */ #define _SF_BOUND 0x20 /* Bit 5: SOCK_STREAM is bound to an address */ @@ -79,10 +72,6 @@ /* Macro to manage the socket state and flags */ -#define _SS_SETSTATE(s,f) (((s) & ~_SF_MASK) | (f)) -#define _SS_GETSTATE(s) ((s) & _SF_MASK) -#define _SS_ISBUSY(s) (_SS_GETSTATE(s) != _SF_IDLE) - #define _SS_ISNONBLOCK(s) (((s) & _SF_NONBLOCK) != 0) #define _SS_ISLISTENING(s) (((s) & _SF_LISTENING) != 0) #define _SS_ISBOUND(s) (((s) & _SF_BOUND) != 0) diff --git a/net/tcp/tcp_accept.c b/net/tcp/tcp_accept.c index 602375bd6114f..1b74d8e71e809 100644 --- a/net/tcp/tcp_accept.c +++ b/net/tcp/tcp_accept.c @@ -264,10 +264,6 @@ int psock_tcp_accept(FAR struct socket *psock, FAR struct sockaddr *addr, else #endif { - /* Set the socket state to accepting */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_ACCEPT); - /* Perform the TCP accept operation */ /* Initialize the state structure. This is done with the network @@ -306,10 +302,6 @@ int psock_tcp_accept(FAR struct socket *psock, FAR struct sockaddr *addr, nxsem_destroy(&state. acpt_sem); - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Check for a errors. Errors are signalled by negative errno values * for the send length. */ diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index 0a888bbbc392a..601747f7093f4 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -994,10 +994,6 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, BUF_DUMP("psock_tcp_send", buf, len); - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - if (len > 0) { /* Allocate a write buffer. Careful, the network will be momentarily @@ -1124,10 +1120,6 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, net_unlock(); } - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Check for errors. Errors are signaled by negative errno values * for the send length */ diff --git a/net/tcp/tcp_send_unbuffered.c b/net/tcp/tcp_send_unbuffered.c index 03b199a1e336a..df9d80b418fa8 100644 --- a/net/tcp/tcp_send_unbuffered.c +++ b/net/tcp/tcp_send_unbuffered.c @@ -707,10 +707,6 @@ ssize_t psock_tcp_send(FAR struct socket *psock, } #endif /* CONFIG_NET_ARP_SEND || CONFIG_NET_ICMPv6_NEIGHBOR */ - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* Perform the TCP send operation */ /* Initialize the state structure. This is done with the network @@ -780,10 +776,6 @@ ssize_t psock_tcp_send(FAR struct socket *psock, nxsem_destroy(&state.snd_sem); net_unlock(); - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Check for a errors. Errors are signalled by negative errno values * for the send length */ diff --git a/net/tcp/tcp_sendfile.c b/net/tcp/tcp_sendfile.c index 6f2ec570c102e..c7fcaed41b5a9 100644 --- a/net/tcp/tcp_sendfile.c +++ b/net/tcp/tcp_sendfile.c @@ -562,10 +562,6 @@ ssize_t tcp_sendfile(FAR struct socket *psock, FAR struct file *infile, } #endif /* CONFIG_NET_ARP_SEND || CONFIG_NET_ICMPv6_NEIGHBOR */ - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* Initialize the state structure. This is done with the network * locked because we don't want anything to happen until we are * ready. @@ -643,10 +639,6 @@ ssize_t tcp_sendfile(FAR struct socket *psock, FAR struct file *infile, } while (state.snd_sent >= 0 && state.snd_acked < state.snd_flen); - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - tcp_callback_free(conn, state.snd_ackcb); errout_datacb: diff --git a/net/udp/udp_psock_sendto_buffered.c b/net/udp/udp_psock_sendto_buffered.c index 46b9bb9da2b72..45ef0432b2355 100644 --- a/net/udp/udp_psock_sendto_buffered.c +++ b/net/udp/udp_psock_sendto_buffered.c @@ -694,10 +694,6 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, BUF_DUMP("psock_udp_send", buf, len); - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - if (len > 0) { /* Allocate a write buffer. Careful, the network will be momentarily @@ -825,10 +821,6 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, net_unlock(); } - /* Set the socket state to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Return the number of bytes that will be sent */ return len; diff --git a/net/udp/udp_psock_sendto_unbuffered.c b/net/udp/udp_psock_sendto_unbuffered.c index 25c74ce620248..1522d9d36e403 100644 --- a/net/udp/udp_psock_sendto_unbuffered.c +++ b/net/udp/udp_psock_sendto_unbuffered.c @@ -458,10 +458,6 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, } #endif /* CONFIG_NET_ARP_SEND || CONFIG_NET_ICMPv6_NEIGHBOR */ - /* Set the socket state to sending */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - /* Initialize the state structure. This is done with the network * locked because we don't want anything to happen until we are * ready. @@ -566,10 +562,6 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf, nxsem_destroy(&state.st_sem); - /* Set the socket state back to idle */ - - psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); - /* Unlock the network and return the result of the sendto() operation */ net_unlock();