diff --git a/include/nuttx/wqueue.h b/include/nuttx/wqueue.h index 7e176e07fa6..b20d5a1ac4d 100644 --- a/include/nuttx/wqueue.h +++ b/include/nuttx/wqueue.h @@ -291,7 +291,11 @@ enum work_evtype_e 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 { @@ -302,6 +306,36 @@ 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 e8179080ce8..aa32df6fc56 100644 --- a/net/tcp/tcp_netpoll.c +++ b/net/tcp/tcp_netpoll.c @@ -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) 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 socket *psock; 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; + DEBUGASSERT(pinfo->psock != NULL && pinfo->fds != NULL); psock = pinfo->psock; - DEBUGASSERT(psock != NULL); - fds = pinfo->fds; - DEBUGASSERT(fds != NULL); /* 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); } + + /* 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_recvwindow.c b/net/tcp/tcp_recvwindow.c index 7123001c188..30b5f352684 100644 --- a/net/tcp/tcp_recvwindow.c +++ b/net/tcp/tcp_recvwindow.c @@ -40,7 +40,7 @@ #include #include -#incldue +#include #include #include diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index 09d206e8cd7..2bc5485deb7 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -1251,19 +1251,31 @@ errout: int psock_tcp_cansend(FAR struct socket *psock) { + /* Verify that we received a valid socket */ + if (!psock || psock->s_crefs <= 0) { nerr("ERROR: Invalid socket\n"); return -EBADF; } + /* Verify that this is connected TCP socket */ + if (psock->s_type != SOCK_STREAM || !_SS_ISCONNECTED(psock->s_flags)) { nerr("ERROR: Not connected\n"); 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; } diff --git a/sched/wqueue/kwork_notifier.c b/sched/wqueue/kwork_notifier.c index 47454b0c1ee..83a2b28f5af 100644 --- a/sched/wqueue/kwork_notifier.c +++ b/sched/wqueue/kwork_notifier.c @@ -55,35 +55,11 @@ #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 ****************************************************************************/ -/* 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 * 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 @@ -91,7 +67,7 @@ struct work_notifier_entry_s * 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 * the notification data structures. @@ -116,28 +92,24 @@ static uint16_t g_notifier_key; * ****************************************************************************/ -static FAR struct work_notifier_entry_s * - work_notifier_find(int16_t key, FAR struct work_notifier_entry_s **pprev) +static FAR struct work_notifier_entry_s *work_notifier_find(int16_t key) { 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. */ - for (prev = NULL, notifier = g_notifier_pending; - notifier != NULL; - prev = notifier, notifier = notifier->flink) + for (entry = dq_peek(&g_notifier_pending); + entry != NULL; + entry = dq_next(entry)) { + notifier = (FAR struct work_notifier_entry_s *)entry; + /* Is this the one we were looking for? */ if (notifier->key == key) { - /* Return the previous entry if so requested */ - - if (pprev != NULL) - { - *pprev = prev; - } + /* Yes.. return a reference to it */ return notifier; } @@ -169,7 +141,7 @@ static int16_t work_notifier_key(void) key = (int16_t)++g_notifier_key; } - while (work_notifier_find(key, NULL) != NULL); + while (work_notifier_find(key) != NULL); return key; } @@ -203,7 +175,7 @@ int work_notifier_setup(FAR struct work_notifier_s *info) int ret; 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 */ @@ -228,20 +200,18 @@ int work_notifier_setup(FAR struct work_notifier_s *info) /* 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 * order of the caller's execution priority so that the * notifications executed in a saner order? */ - notifier->flink = g_notifier_pending; - g_notifier_pending = notifier; - - ret = work_notifier_key(); + dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_pending); + ret = work_notifier_key(); } (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) { FAR struct work_notifier_entry_s *notifier; - FAR struct work_notifier_entry_s *prev; int ret; DEBUGASSERT(key > 0 && key <= INT16_MAX); @@ -287,7 +256,7 @@ int work_notifier_teardown(int key) * assume that there is only one. */ - notifier = work_notifier_find(key, &prev); + notifier = work_notifier_find(key); if (notifier == NULL) { /* 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 */ - if (prev == NULL) - { - g_notifier_pending = notifier->flink; - } - else - { - prev->flink = notifier->flink; - } + dq_rem((FAR dq_entry_t *)notifier, &g_notifier_pending); /* Free the notification */ @@ -342,8 +304,8 @@ void work_notifier_signal(enum work_evtype_e evtype, FAR void *qualifier) { FAR struct work_notifier_entry_s *notifier; - FAR struct work_notifier_entry_s *prev; - FAR struct work_notifier_entry_s *next; + FAR dq_entry_t *entry; + FAR dq_entry_t *next; int ret; /* 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. */ - for (prev = NULL, notifier = g_notifier_pending; - notifier != NULL; - notifier = next) + for (entry = dq_peek(&g_notifier_pending); + entry != NULL; + entry = next) { FAR struct work_notifier_s *info; @@ -375,8 +337,12 @@ void work_notifier_signal(enum work_evtype_e evtype, * removed from the list). */ - next = notifier->flink; - info = ¬ifier->info; + next = entry->flink; + + /* Set up some convenience pointers */ + + notifier = (FAR struct work_notifier_entry_s *)entry; + info = ¬ifier->info; /* Check if this is the a notification request for the event that * just occurred. @@ -386,27 +352,15 @@ void work_notifier_signal(enum work_evtype_e evtype, { /* Yes.. Remove the notification from the pending list */ - if (prev == NULL) - { - g_notifier_pending = next; - } - else - { - prev->flink = next; - } + dq_rem((FAR dq_entry_t *)notifier, &g_notifier_pending); - /* Schedule the work */ - - (void)work_queue(info->qid, ¬ifier->work, info->worker, - info, 0); - } - else - { - /* The entry was not removed, the current entry will be the - * next previous entry. + /* Schedule the work. The entire notifier entry is passed as an + * argument to the work function because that function is + * responsible for freeing the allocated memory. */ - prev = notifier; + (void)work_queue(info->qid, ¬ifier->work, info->worker, + entry, 0); } }