Squashed commit of the following:

sched/wqueue/kwork_notifier.c:  Redesign some data structures.  struct works_s must appear at the beginning of the notifier entry structure.  That is because it contains the work queue indices.  This solves a harfault issue.

    net/tcp/tcp_netpoll.c:  tcp_iob_work() needs to free the allocated argument when it is finished.

    net/tcp/tcp_send_buffered.c:  Extend psock_tcp_cansend() so that it also requires that at least on IOB is also avaialble.

    mm/iob:  iob_navail() was returning the number of free IOB chain queue entries, not the number of free IOBs.  Completely misnamed.

    net/tcp/tcp_netpoll.c:  Add logic to receive notifications when IOBs are freed (Needs CONFIG_NET_TCP_WRITE_BUFFERS and CONFIG_IOB_NOTIFIER).  At present, does nothing because the logic in in psock_tcp_cansend() does not check for the availability of IOBs.  That will change.
This commit is contained in:
Gregory Nutt 2018-09-12 08:57:06 -06:00
parent 76eec53e4f
commit b3f0aab00a
5 changed files with 100 additions and 90 deletions

View File

@ -291,7 +291,11 @@ enum work_evtype_e
WORK_UDP_READAHEAD /* Notify that TCP read-ahead data is available */ WORK_UDP_READAHEAD /* Notify that TCP read-ahead data is available */
}; };
/* This structure describes one notification */ /* This structure describes one notification and is provided as input to
* to work_notifier_setup(). This input is copied by work_notifier_setup()
* into an allocated instance of struct work_notifier_entry_s and need not
* persist on the caller's side.
*/
struct work_notifier_s struct work_notifier_s
{ {
@ -302,6 +306,36 @@ struct work_notifier_s
worker_t worker; /* The worker function to schedule */ 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 * Public Data
****************************************************************************/ ****************************************************************************/

View File

@ -175,19 +175,23 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev,
#if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER) #if defined(CONFIG_NET_TCP_WRITE_BUFFERS) && defined(CONFIG_IOB_NOTIFIER)
static inline void tcp_iob_work(FAR void *arg) static inline void tcp_iob_work(FAR void *arg)
{ {
FAR struct work_notifier_s *ninfo = (FAR struct work_notifier_s *)arg; FAR struct work_notifier_entry_s *entry;
FAR struct work_notifier_s *ninfo;
FAR struct tcp_poll_s *pinfo; FAR struct tcp_poll_s *pinfo;
FAR struct socket *psock; FAR struct socket *psock;
FAR struct pollfd *fds; FAR struct pollfd *fds;
DEBUGASSERT(ninfo != NULL && ninfo->arg != NULL); 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 *)ninfo->arg;
DEBUGASSERT(pinfo->psock != NULL && pinfo->fds != NULL);
psock = pinfo->psock; psock = pinfo->psock;
DEBUGASSERT(psock != NULL);
fds = pinfo->fds; fds = pinfo->fds;
DEBUGASSERT(fds != NULL);
/* Verify that we still have a connection */ /* Verify that we still have a connection */
@ -222,6 +226,12 @@ static inline void tcp_iob_work(FAR void *arg)
pinfo->key = iob_notifier_setup(LPWORK, tcp_iob_work, pinfo); 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 #endif

View File

@ -40,7 +40,7 @@
#include <nuttx/config.h> #include <nuttx/config.h>
#include <stdint.h> #include <stdint.h>
#incldue <stdbool.h> #include <stdbool.h>
#include <debug.h> #include <debug.h>
#include <net/if.h> #include <net/if.h>

View File

@ -1251,19 +1251,31 @@ errout:
int psock_tcp_cansend(FAR struct socket *psock) int psock_tcp_cansend(FAR struct socket *psock)
{ {
/* Verify that we received a valid socket */
if (!psock || psock->s_crefs <= 0) if (!psock || psock->s_crefs <= 0)
{ {
nerr("ERROR: Invalid socket\n"); nerr("ERROR: Invalid socket\n");
return -EBADF; return -EBADF;
} }
/* Verify that this is connected TCP socket */
if (psock->s_type != SOCK_STREAM || !_SS_ISCONNECTED(psock->s_flags)) if (psock->s_type != SOCK_STREAM || !_SS_ISCONNECTED(psock->s_flags))
{ {
nerr("ERROR: Not connected\n"); nerr("ERROR: Not connected\n");
return -ENOTCONN; return -ENOTCONN;
} }
if (tcp_wrbuffer_test() < 0) /* In order to setup the send, we need to have at least one free write
* buffer head and at least one free IOB to initialize the write buffer head.
*
* REVISIT: The send will still block if we are unable to buffer the entire
* user-provided buffer which may be quite large. We will almost certainly
* need to have more than one free IOB, but we don't know how many more.
*/
if (tcp_wrbuffer_test() < 0 || iob_navail(false) <= 0)
{ {
return -EWOULDBLOCK; return -EWOULDBLOCK;
} }

View File

@ -55,35 +55,11 @@
#ifdef CONFIG_WQUEUE_NOTIFIER #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 and must be freed using
* kmm_free() by the work.
*/
struct work_notifier_entry_s
{
/* User notification information */
struct work_notifier_s info; /* The notification info */
/* Additional payload needed to manage the notification */
FAR struct work_notifier_entry_s *flink;
struct work_s work; /* Used for scheduling the work */
int16_t key; /* Unique ID for the notification */
};
/**************************************************************************** /****************************************************************************
* Private Data * Private Data
****************************************************************************/ ****************************************************************************/
/* This is a singly linked list of pending notifications. When an event /* 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 * 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 * be notified and the entry will be freed. If there are multiple waiters
* for some resource, then only the first to execute thread will get the * for some resource, then only the first to execute thread will get the
@ -91,7 +67,7 @@ struct work_notifier_entry_s
* once again. * once again.
*/ */
static FAR struct work_notifier_entry_s *g_notifier_pending; static dq_queue_t g_notifier_pending;
/* This semaphore is used as mutex to enforce mutually exclusive access to /* This semaphore is used as mutex to enforce mutually exclusive access to
* the notification data structures. * the notification data structures.
@ -116,28 +92,24 @@ static uint16_t g_notifier_key;
* *
****************************************************************************/ ****************************************************************************/
static FAR struct work_notifier_entry_s * static FAR struct work_notifier_entry_s *work_notifier_find(int16_t key)
work_notifier_find(int16_t key, FAR struct work_notifier_entry_s **pprev)
{ {
FAR struct work_notifier_entry_s *notifier; FAR struct work_notifier_entry_s *notifier;
FAR struct work_notifier_entry_s *prev; FAR dq_entry_t *entry;
/* Find the entry matching this key in the g_notifier_pending list. */ /* Find the entry matching this key in the g_notifier_pending list. */
for (prev = NULL, notifier = g_notifier_pending; for (entry = dq_peek(&g_notifier_pending);
notifier != NULL; entry != NULL;
prev = notifier, notifier = notifier->flink) entry = dq_next(entry))
{ {
notifier = (FAR struct work_notifier_entry_s *)entry;
/* Is this the one we were looking for? */ /* Is this the one we were looking for? */
if (notifier->key == key) if (notifier->key == key)
{ {
/* Return the previous entry if so requested */ /* Yes.. return a reference to it */
if (pprev != NULL)
{
*pprev = prev;
}
return notifier; return notifier;
} }
@ -169,7 +141,7 @@ static int16_t work_notifier_key(void)
key = (int16_t)++g_notifier_key; key = (int16_t)++g_notifier_key;
} }
while (work_notifier_find(key, NULL) != NULL); while (work_notifier_find(key) != NULL);
return key; return key;
} }
@ -203,7 +175,7 @@ int work_notifier_setup(FAR struct work_notifier_s *info)
int ret; int ret;
DEBUGASSERT(info != NULL && info->worker != NULL); DEBUGASSERT(info != NULL && info->worker != NULL);
DEBUGASSERT(info->qid == HPWORK && info->qid == LPWORK); DEBUGASSERT(info->qid == HPWORK || info->qid == LPWORK);
/* Get exclusive access to the notifier data structures */ /* Get exclusive access to the notifier data structures */
@ -228,20 +200,18 @@ int work_notifier_setup(FAR struct work_notifier_s *info)
/* Generate a unique key for this notification */ /* Generate a unique key for this notification */
notifier->key = work_notifier_key(); notifier->key = work_notifier_key();
/* Add the notification to the head of the pending list /* Add the notification to the tail of the pending list
* *
* REVISIT: Work will be processed in LIFO order. Perhaps * REVISIT: Work will be processed in FIFO order. Perhaps
* we should should consider saving the notification is the * we should should consider saving the notification is the
* order of the caller's execution priority so that the * order of the caller's execution priority so that the
* notifications executed in a saner order? * notifications executed in a saner order?
*/ */
notifier->flink = g_notifier_pending; dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_pending);
g_notifier_pending = notifier; ret = work_notifier_key();
ret = work_notifier_key();
} }
(void)nxsem_post(&g_notifier_sem); (void)nxsem_post(&g_notifier_sem);
@ -270,7 +240,6 @@ int work_notifier_setup(FAR struct work_notifier_s *info)
int work_notifier_teardown(int key) int work_notifier_teardown(int key)
{ {
FAR struct work_notifier_entry_s *notifier; FAR struct work_notifier_entry_s *notifier;
FAR struct work_notifier_entry_s *prev;
int ret; int ret;
DEBUGASSERT(key > 0 && key <= INT16_MAX); DEBUGASSERT(key > 0 && key <= INT16_MAX);
@ -287,7 +256,7 @@ int work_notifier_teardown(int key)
* assume that there is only one. * assume that there is only one.
*/ */
notifier = work_notifier_find(key, &prev); notifier = work_notifier_find(key);
if (notifier == NULL) if (notifier == NULL)
{ {
/* There is no notification with this key in the pending list */ /* There is no notification with this key in the pending list */
@ -298,14 +267,7 @@ int work_notifier_teardown(int key)
{ {
/* Found it! Remove the notification from the pending list */ /* Found it! Remove the notification from the pending list */
if (prev == NULL) dq_rem((FAR dq_entry_t *)notifier, &g_notifier_pending);
{
g_notifier_pending = notifier->flink;
}
else
{
prev->flink = notifier->flink;
}
/* Free the notification */ /* Free the notification */
@ -342,8 +304,8 @@ void work_notifier_signal(enum work_evtype_e evtype,
FAR void *qualifier) FAR void *qualifier)
{ {
FAR struct work_notifier_entry_s *notifier; FAR struct work_notifier_entry_s *notifier;
FAR struct work_notifier_entry_s *prev; FAR dq_entry_t *entry;
FAR struct work_notifier_entry_s *next; FAR dq_entry_t *next;
int ret; int ret;
/* Get exclusive access to the notifier data structure */ /* Get exclusive access to the notifier data structure */
@ -365,9 +327,9 @@ void work_notifier_signal(enum work_evtype_e evtype,
/* Find the entry matching this key in the g_notifier_pending list. */ /* Find the entry matching this key in the g_notifier_pending list. */
for (prev = NULL, notifier = g_notifier_pending; for (entry = dq_peek(&g_notifier_pending);
notifier != NULL; entry != NULL;
notifier = next) entry = next)
{ {
FAR struct work_notifier_s *info; FAR struct work_notifier_s *info;
@ -375,8 +337,12 @@ void work_notifier_signal(enum work_evtype_e evtype,
* removed from the list). * removed from the list).
*/ */
next = notifier->flink; next = entry->flink;
info = &notifier->info;
/* Set up some convenience pointers */
notifier = (FAR struct work_notifier_entry_s *)entry;
info = &notifier->info;
/* Check if this is the a notification request for the event that /* Check if this is the a notification request for the event that
* just occurred. * just occurred.
@ -386,27 +352,15 @@ void work_notifier_signal(enum work_evtype_e evtype,
{ {
/* Yes.. Remove the notification from the pending list */ /* Yes.. Remove the notification from the pending list */
if (prev == NULL) dq_rem((FAR dq_entry_t *)notifier, &g_notifier_pending);
{
g_notifier_pending = next;
}
else
{
prev->flink = next;
}
/* Schedule the work */ /* Schedule the work. The entire notifier entry is passed as an
* argument to the work function because that function is
(void)work_queue(info->qid, &notifier->work, info->worker, * responsible for freeing the allocated memory.
info, 0);
}
else
{
/* The entry was not removed, the current entry will be the
* next previous entry.
*/ */
prev = notifier; (void)work_queue(info->qid, &notifier->work, info->worker,
entry, 0);
} }
} }