From 427b3b8fcb7bfedd2f5f367bf7b6a658cb98e25b Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sat, 7 Jul 2018 08:26:13 -0600 Subject: [PATCH] Squashed commit of the following: net/utils: return from net_breaklock() was being clobbered. net/: Replace all calls to iob_alloc() with calls to net_ioballoc() which will release the network lock, if necessary. net/utils, tcp, include/net: Separate out the special IOB allocation logic and place it in its own function. Prototype is available in a public header file where it can also be used by network drivers. net/utils: net_timedwait() now uses new net_breaklock() and net_restorelock(). --- include/nuttx/net/net.h | 34 ++++++++-- net/bluetooth/bluetooth_sendto.c | 3 +- net/ieee802154/ieee802154_sendto.c | 5 +- net/sixlowpan/sixlowpan_framelist.c | 7 ++- net/socket/socket.c | 2 +- net/tcp/tcp_wrbuffer.c | 18 +----- net/udp/udp_wrbuffer.c | 3 +- net/utils/net_lock.c | 97 ++++++++++++++++++++--------- 8 files changed, 110 insertions(+), 59 deletions(-) diff --git a/include/nuttx/net/net.h b/include/nuttx/net/net.h index 9aee838305c..e9f39c8de3d 100644 --- a/include/nuttx/net/net.h +++ b/include/nuttx/net/net.h @@ -290,12 +290,15 @@ void net_initialize(void); /**************************************************************************** * Critical section management. * - * Semaphore based locking is used: + * Re-entrant mutex based locking of the network is supported: * - * net_lock() - Takes the semaphore(). Implements a re-entrant mutex. - * net_unlock() - Gives the semaphore(). - * net_lockedwait() - Like pthread_cond_wait(); releases the semaphore - * momentarily to wait on another semaphore() + * net_lock() - Locks the network via a re-entrant mutex. + * net_unlock() - Unlocks the network. + * net_lockedwait() - Like pthread_cond_wait() except releases the + * network momentarily to wait on another semaphore. + * net_ioballoc() - Like iob_alloc() except releases the network + * momentarily to wait for an IOB to become + * available. * ****************************************************************************/ @@ -368,6 +371,27 @@ int net_timedwait(sem_t *sem, FAR const struct timespec *abstime); int net_lockedwait(sem_t *sem); +/**************************************************************************** + * Name: net_ioballoc + * + * Description: + * Allocate an IOB. If no IOBs are available, then atomically wait for + * for the IOB while temporarily releasing the lock on the network. + * + * Input Parameters: + * throttled - An indication of the IOB allocation is "throttled" + * + * Returned Value: + * A pointer to the newly allocated IOB is returned on success. NULL is + * returned on any allocation failure. + * + ****************************************************************************/ + +#ifdef CONFIG_MM_IOB +struct iob_s; /* Forward reference */ +FAR struct iob_s *net_ioballoc(bool throttled); +#endif + /**************************************************************************** * Name: net_setipid * diff --git a/net/bluetooth/bluetooth_sendto.c b/net/bluetooth/bluetooth_sendto.c index a411457f6f7..49633ef9289 100644 --- a/net/bluetooth/bluetooth_sendto.c +++ b/net/bluetooth/bluetooth_sendto.c @@ -61,6 +61,7 @@ #include #include +#include "utils/utils.h" #include "netdev/netdev.h" #include "devif/devif.h" #include "socket/socket.h" @@ -158,7 +159,7 @@ static uint16_t bluetooth_sendto_eventhandler(FAR struct net_driver_s *dev, /* Allocate an IOB to hold the frame data */ - iob = iob_alloc(0); + iob = net_ioballoc(false); if (iob == NULL) { nwarn("WARNING: Failed to allocate IOB\n"); diff --git a/net/ieee802154/ieee802154_sendto.c b/net/ieee802154/ieee802154_sendto.c index 64a6c77e089..2539562002c 100644 --- a/net/ieee802154/ieee802154_sendto.c +++ b/net/ieee802154/ieee802154_sendto.c @@ -1,7 +1,7 @@ /**************************************************************************** * net/ieee802154/ieee802154_sendto.c * - * Copyright (C) 2017 Gregory Nutt. All rights reserved. + * Copyright (C) 2017-2018 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -57,6 +57,7 @@ #include #include +#include "utils/utils.h" #include "netdev/netdev.h" #include "devif/devif.h" #include "socket/socket.h" @@ -345,7 +346,7 @@ static uint16_t ieee802154_sendto_eventhandler(FAR struct net_driver_s *dev, /* Allocate an IOB to hold the frame data */ - iob = iob_alloc(0); + iob = net_ioballoc(false); if (iob == NULL) { nwarn("WARNING: Failed to allocate IOB\n"); diff --git a/net/sixlowpan/sixlowpan_framelist.c b/net/sixlowpan/sixlowpan_framelist.c index 3ae1939c5ec..bd5b216bd39 100644 --- a/net/sixlowpan/sixlowpan_framelist.c +++ b/net/sixlowpan/sixlowpan_framelist.c @@ -1,7 +1,7 @@ /**************************************************************************** * net/sixlowpan/sixlowpan_framelist.c * - * Copyright (C) 2017 Gregory Nutt. All rights reserved. + * Copyright (C) 2017-2018 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Parts of this file derive from Contiki: @@ -59,6 +59,7 @@ #include #include +#include "utils/utils.h" #include "sixlowpan/sixlowpan_internal.h" #ifdef CONFIG_NET_6LOWPAN @@ -409,7 +410,7 @@ int sixlowpan_queue_frames(FAR struct radio_driver_s *radio, * necessary. */ - iob = iob_alloc(false); + iob = net_ioballoc(false); DEBUGASSERT(iob != NULL); /* Initialize the IOB */ @@ -633,7 +634,7 @@ int sixlowpan_queue_frames(FAR struct radio_driver_s *radio, * necessary. */ - iob = iob_alloc(false); + iob = net_ioballoc(false); DEBUGASSERT(iob != NULL); /* Initialize the IOB */ diff --git a/net/socket/socket.c b/net/socket/socket.c index 6974f62d9b0..bf11bbe31ad 100644 --- a/net/socket/socket.c +++ b/net/socket/socket.c @@ -212,7 +212,7 @@ int socket(int domain, int type, int protocol) sockfd = sockfd_allocate(0); if (sockfd < 0) { - nerr("ERROR: Failed to allodate a socket descriptor\n"); + nerr("ERROR: Failed to allocate a socket descriptor\n"); errcode = ENFILE; goto errout; } diff --git a/net/tcp/tcp_wrbuffer.c b/net/tcp/tcp_wrbuffer.c index c0eb6bfe4f7..4e56d1a7a05 100644 --- a/net/tcp/tcp_wrbuffer.c +++ b/net/tcp/tcp_wrbuffer.c @@ -161,23 +161,7 @@ FAR struct tcp_wrbuffer_s *tcp_wrbuffer_alloc(void) /* Now get the first I/O buffer for the write buffer structure */ - wrb->wb_iob = iob_tryalloc(false); - if (wrb->wb_iob == NULL) - { - unsigned int count; - int ret; - - /* There are no buffers available now. We will have to wait for one to - * become available. But let's not do that with the network locked. - */ - - ret = net_breaklock(&count); - wrb->wb_iob = iob_alloc(false); - if (ret >= 0) - { - net_restorelock(count); - } - } + wrb->wb_iob = net_ioballoc(false); /* Did we get an IOB? We should always get one except under some really weird * error conditions. diff --git a/net/udp/udp_wrbuffer.c b/net/udp/udp_wrbuffer.c index 5e0eca86779..434a0f42215 100644 --- a/net/udp/udp_wrbuffer.c +++ b/net/udp/udp_wrbuffer.c @@ -56,6 +56,7 @@ #include #include +#include "utils/utils.h" #include "udp/udp.h" /**************************************************************************** @@ -156,7 +157,7 @@ FAR struct udp_wrbuffer_s *udp_wrbuffer_alloc(void) /* Now get the first I/O buffer for the write buffer structure */ - wrb->wb_iob = iob_alloc(false); + wrb->wb_iob = net_ioballoc(false); if (!wrb->wb_iob) { nerr("ERROR: Failed to allocate I/O buffer\n"); diff --git a/net/utils/net_lock.c b/net/utils/net_lock.c index 6b32062c5fc..9bc62b0c247 100644 --- a/net/utils/net_lock.c +++ b/net/utils/net_lock.c @@ -48,6 +48,7 @@ #include #include #include +#include #include #include "utils/utils.h" @@ -283,48 +284,40 @@ int net_timedwait(sem_t *sem, FAR const struct timespec *abstime) { unsigned int count; irqstate_t flags; + int blresult; int ret; flags = enter_critical_section(); /* No interrupts */ + sched_lock(); /* No context switches */ - pid_t me = getpid(); + /* Release the network lock, remembering my count. net_breaklock will + * return a negated value if the caller does not hold the network lock. + */ - sched_lock(); /* No context switches */ - if (g_holder == me) + blresult = net_breaklock(&count); + + /* Now take the semaphore, waiting if so requested. */ + + if (abstime != NULL) { - /* Release the network lock, remembering my count */ + /* Wait until we get the lock or until the timeout expires */ - count = g_count; - g_holder = NO_HOLDER; - g_count = 0; - nxsem_post(&g_netlock); - - /* Now take the semaphore, waiting if so requested. */ - - if (abstime != NULL) - { - /* Wait until we get the lock or until the timeout expires */ - - ret = nxsem_timedwait(sem, abstime); - } - else - { - /* Wait as long as necessary to get the lock */ - - ret = nxsem_wait(sem); - } - - /* Recover the network lock at the proper count */ - - _net_takesem(); - g_holder = me; - g_count = count; + ret = nxsem_timedwait(sem, abstime); } else { + /* Wait as long as necessary to get the lock */ + ret = nxsem_wait(sem); } + /* Recover the network lock at the proper count (if we held it before) */ + + if (blresult >= 0) + { + net_restorelock(count); + } + sched_unlock(); leave_critical_section(flags); return ret; @@ -350,3 +343,49 @@ int net_lockedwait(sem_t *sem) return net_timedwait(sem, NULL); } +/**************************************************************************** + * Name: net_ioballoc + * + * Description: + * Allocate an IOB. If no IOBs are available, then atomically wait for + * for the IOB while temporarily releasing the lock on the network. + * + * Input Parameters: + * throttled - An indication of the IOB allocation is "throttled" + * + * Returned Value: + * A pointer to the newly allocated IOB is returned on success. NULL is + * returned on any allocation failure. + * + ****************************************************************************/ + +#ifdef CONFIG_MM_IOB +FAR struct iob_s *net_ioballoc(bool throttled) +{ + FAR struct iob_s *iob; + + iob = iob_tryalloc(throttled); + if (iob == NULL) + { + irqstate_t flags; + unsigned int count; + int blresult; + + /* There are no buffers available now. We will have to wait for one to + * become available. But let's not do that with the network locked. + */ + + flags = enter_critical_section(); + blresult = net_breaklock(&count); + iob = iob_alloc(throttled); + if (blresult >= 0) + { + net_restorelock(count); + } + + leave_critical_section(flags); + } + + return iob; +} +#endif