You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ac...@apache.org on 2020/04/05 15:32:17 UTC
[incubator-nuttx] 02/03: Check return from
nxsem_wait_uninterruptible()
This is an automated email from the ASF dual-hosted git repository.
acassis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit 6a43282197711a411d6598cdd0f4f1eefb1d7538
Author: Gregory Nutt <gn...@nuttx.org>
AuthorDate: Sun Apr 5 09:08:06 2020 -0600
Check return from nxsem_wait_uninterruptible()
Resolution of Issue 619 will require multiple steps, this part of the first step in that resolution: Every call to nxsem_wait_uninterruptible() must handle the return value from nxsem_wait_uninterruptible properly. This commit is for all remaining drivers in the Atmel SAM architectures.
---
arch/arm/src/imxrt/imxrt_ehci.c | 12 ++---
arch/arm/src/sama5/sam_can.c | 69 +++++++++++++++++++++----
arch/arm/src/sama5/sam_nand.c | 71 ++++++++++++++++++--------
arch/arm/src/sama5/sam_pmecc.c | 7 +--
arch/arm/src/sama5/sam_pmecc.h | 15 ++++--
arch/arm/src/samv7/sam_mcan.c | 108 ++++++++++++++++++++++++++++++++++------
6 files changed, 224 insertions(+), 58 deletions(-)
diff --git a/arch/arm/src/imxrt/imxrt_ehci.c b/arch/arm/src/imxrt/imxrt_ehci.c
index e3f6485..b3d379c 100644
--- a/arch/arm/src/imxrt/imxrt_ehci.c
+++ b/arch/arm/src/imxrt/imxrt_ehci.c
@@ -434,7 +434,7 @@ static int ehci_wait_usbsts(uint32_t maskbits, uint32_t donebits,
/* Semaphores ***************************************************************/
static int imxrt_takesem(sem_t *sem);
-static int imxrt_takesem_uninterruptible(sem_t *sem);
+static int imxrt_takesem_noncancelable(sem_t *sem);
#define imxrt_givesem(s) nxsem_post(s);
/* Allocators ***************************************************************/
@@ -1078,7 +1078,7 @@ static int imxrt_takesem(sem_t *sem)
}
/****************************************************************************
- * Name: imxrt_takesem_uninterruptible
+ * Name: imxrt_takesem_noncancelable
*
* Description:
* This is just a wrapper to handle the annoying behavior of semaphore
@@ -1087,7 +1087,7 @@ static int imxrt_takesem(sem_t *sem)
*
****************************************************************************/
-static int imxrt_takesem_uninterruptible(sem_t *sem)
+static int imxrt_takesem_noncancelable(sem_t *sem)
{
int result;
int ret = OK;
@@ -2591,7 +2591,7 @@ static ssize_t imxrt_transfer_wait(struct imxrt_epinfo_s *epinfo)
* this upon return.
*/
- ret2 = imxrt_takesem_uninterruptible(&g_ehci.exclsem);
+ ret2 = imxrt_takesem_noncancelable(&g_ehci.exclsem);
if (ret >= 0 && ret2 < 0)
{
ret = ret2;
@@ -2615,7 +2615,7 @@ static ssize_t imxrt_transfer_wait(struct imxrt_epinfo_s *epinfo)
}
#endif
- /* Did imxrt_ioc_wait() or imxrt_takesem_uninterruptible() report an
+ /* Did imxrt_ioc_wait() or imxrt_takesem_noncancelable() report an
* error?
*/
@@ -3338,7 +3338,7 @@ static void imxrt_ehci_bottomhalf(FAR void *arg)
* real option (other than to reschedule and delay).
*/
- imxrt_takesem_uninterruptible(&g_ehci.exclsem);
+ imxrt_takesem_noncancelable(&g_ehci.exclsem);
/* Handle all unmasked interrupt sources
* USB Interrupt (USBINT)
diff --git a/arch/arm/src/sama5/sam_can.c b/arch/arm/src/sama5/sam_can.c
index c08ab1f..aa24ef2 100644
--- a/arch/arm/src/sama5/sam_can.c
+++ b/arch/arm/src/sama5/sam_can.c
@@ -207,7 +207,8 @@ static void can_dumpmbregs(FAR struct sam_can_s *priv, FAR const char *msg);
/* Semaphore helpers */
-static void can_semtake(FAR struct sam_can_s *priv);
+static int can_semtake(FAR struct sam_can_s *priv);
+static int can_semtake_noncancelable(FAR struct sam_can_s *priv);
#define can_semgive(priv) nxsem_post(&priv->exclsem)
/* Mailboxes */
@@ -563,13 +564,48 @@ static void can_dumpmbregs(FAR struct sam_can_s *priv, FAR const char *msg)
* priv - A reference to the CAN peripheral state
*
* Returned Value:
- * None
+ * Normally success (OK) is returned, but the error -ECANCELED may be
+ * return in the event that task has been canceled.
+ *
+ ****************************************************************************/
+
+static int can_semtake(FAR struct sam_can_s *priv)
+{
+ return nxsem_wait_uninterruptible(&priv->exclsem);
+}
+
+/****************************************************************************
+ * Name: can_semtake_noncancelable
+ *
+ * Description:
+ * This is just a wrapper to handle the annoying behavior of semaphore
+ * waits that return due to the receipt of a signal. This version also
+ * ignores attempts to cancel the thread.
*
****************************************************************************/
-static void can_semtake(FAR struct sam_can_s *priv)
+static int can_semtake_noncancelable(FAR struct sam_can_s *priv)
{
- nxsem_wait_uninterruptible(&priv->exclsem);
+ int result;
+ int ret = OK;
+
+ do
+ {
+ result = nxsem_wait_uninterruptible(&priv->exclsem);
+
+ /* The only expected error is ECANCELED which would occur if the
+ * calling thread were canceled.
+ */
+
+ DEBUGASSERT(result == OK || result == -ECANCELED);
+ if (ret == OK && result < 0)
+ {
+ ret = result;
+ }
+ }
+ while (result < 0);
+
+ return ret;
}
/****************************************************************************
@@ -765,6 +801,7 @@ static void can_reset(FAR struct can_dev_s *dev)
{
FAR struct sam_can_s *priv;
FAR const struct sam_config_s *config;
+ int ret;
int i;
DEBUGASSERT(dev);
@@ -778,7 +815,7 @@ static void can_reset(FAR struct can_dev_s *dev)
/* Get exclusive access to the CAN peripheral */
- can_semtake(priv);
+ can_semtake_noncancelable();
/* Disable all interrupts */
@@ -834,7 +871,11 @@ static int can_setup(FAR struct can_dev_s *dev)
/* Get exclusive access to the CAN peripheral */
- can_semtake(priv);
+ ret = can_semtake(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
/* CAN hardware initialization */
@@ -915,7 +956,7 @@ static void can_shutdown(FAR struct can_dev_s *dev)
/* Get exclusive access to the CAN peripheral */
- can_semtake(priv);
+ can_semtake_noncancelable(priv);
/* Disable the CAN interrupts */
@@ -987,7 +1028,7 @@ static void can_txint(FAR struct can_dev_s *dev, bool enable)
/* Get exclusive access to the CAN peripheral */
- can_semtake(priv);
+ can_semtake_noncancelable(priv);
/* Support disabling interrupts on any mailboxes that are actively
* transmitting (txmbset); also suppress enabling new TX mailbox until
@@ -1090,7 +1131,11 @@ static int can_send(FAR struct can_dev_s *dev, FAR struct can_msg_s *msg)
/* Get exclusive access to the CAN peripheral */
- can_semtake(priv);
+ ret = can_semtake(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
/* Allocate a mailbox */
@@ -1206,7 +1251,11 @@ static bool can_txready(FAR struct can_dev_s *dev)
/* Get exclusive access to the CAN peripheral */
- can_semtake(priv);
+ ret = can_semtake(priv);
+ if (ret < 0)
+ {
+ return false;
+ }
/* Return true not all mailboxes are in-use */
diff --git a/arch/arm/src/sama5/sam_nand.c b/arch/arm/src/sama5/sam_nand.c
index 6b82aad..c8d93b1 100644
--- a/arch/arm/src/sama5/sam_nand.c
+++ b/arch/arm/src/sama5/sam_nand.c
@@ -141,10 +141,10 @@
/* Low-level HSMC Helpers */
#if NAND_NBANKS > 1
-void nand_lock(void);
+int nand_lock(void);
void nand_unlock(void);
#else
-# define nand_lock()
+# define nand_lock() (0)
# define nand_unlock()
#endif
@@ -306,14 +306,15 @@ struct sam_nand_s g_nand;
* None
*
* Returned Value:
- * None
+ * Normally success (OK) is returned, but the error -ECANCELED may be
+ * return in the event that task has been canceled.
*
****************************************************************************/
#if NAND_NBANKS > 1
-void nand_lock(void)
+static int nand_lock(void)
{
- nxsem_wait_uninterruptible(&g_nand.exclsem);
+ return nxsem_wait_uninterruptible(&g_nand.exclsem);
}
#endif
@@ -332,7 +333,7 @@ void nand_lock(void)
****************************************************************************/
#if NAND_NBANKS > 1
-void nand_unlock(void)
+static void nand_unlock(void)
{
nxsem_post(&g_nand.exclsem);
}
@@ -2033,7 +2034,12 @@ static int nand_readpage_pmecc(struct sam_nandcs_s *priv, off_t block,
* is properly configured for this CS.
*/
- pmecc_lock();
+ ret = pmecc_lock();
+ if (ret < 0)
+ {
+ return ret;
+ }
+
ret = pmecc_configure(priv, false);
if (ret < 0)
{
@@ -2312,7 +2318,7 @@ static int nand_writepage_pmecc(struct sam_nandcs_s *priv, off_t block,
unsigned int eccsize;
unsigned int sector;
unsigned int i;
- int ret = 0;
+ int ret;
finfo("block=%d page=%d data=%p\n", (int)block, page, data);
DEBUGASSERT(priv && data);
@@ -2321,7 +2327,12 @@ static int nand_writepage_pmecc(struct sam_nandcs_s *priv, off_t block,
* is properly configured for this CS.
*/
- pmecc_lock();
+ ret = pmecc_lock();
+ if (ret < 0)
+ {
+ return ret;
+ }
+
ret = pmecc_configure(priv, false);
if (ret < 0)
{
@@ -2542,17 +2553,21 @@ static int nand_eraseblock(struct nand_raw_s *raw, off_t block)
{
struct sam_nandcs_s *priv = (struct sam_nandcs_s *)raw;
int retries = NAND_ERASE_NRETRIES;
- int ret = OK;
+ int ret;
DEBUGASSERT(priv);
finfo("block=%d\n", (int)block);
- /* Get exclusvie access to the HSMC hardware.
+ /* Get exclusive access to the HSMC hardware.
* REVISIT: The scope of this exclusivity is just NAND.
*/
- nand_lock();
+ ret = nand_lock();
+ if (ret < 0)
+ {
+ return ret;
+ }
/* Try up to NAND_ERASE_NRETRIES times to erase the FLASH */
@@ -2606,9 +2621,13 @@ static int nand_rawread(struct nand_raw_s *raw, off_t block,
* REVISIT: The scope of this exclusivity is just NAND.
*/
- nand_lock();
- ret = nand_readpage_noecc(priv, block, page, data, spare);
- nand_unlock();
+ ret = nand_lock();
+ if (ret >= 0)
+ {
+ ret = nand_readpage_noecc(priv, block, page, data, spare);
+ nand_unlock();
+ }
+
return ret;
}
@@ -2644,9 +2663,13 @@ static int nand_rawwrite(struct nand_raw_s *raw, off_t block,
* REVISIT: The scope of this exclusivity is just NAND.
*/
- nand_lock();
- ret = nand_writepage_noecc(priv, block, page, data, spare);
- nand_unlock();
+ ret = nand_lock();
+ if (ret >= 0)
+ {
+ ret = nand_writepage_noecc(priv, block, page, data, spare);
+ nand_unlock();
+ }
+
return ret;
}
@@ -2683,7 +2706,11 @@ static int nand_readpage(struct nand_raw_s *raw, off_t block,
* REVISIT: The scope of this exclusivity is just NAND.
*/
- nand_lock();
+ ret = nand_lock();
+ if (ret < 0)
+ {
+ return ret;
+ }
/* Read the page */
@@ -2746,7 +2773,11 @@ static int nand_writepage(struct nand_raw_s *raw, off_t block,
* REVISIT: The scope of this exclusivity is just NAND.
*/
- nand_lock();
+ ret = nand_lock();
+ if (ret < 0)
+ {
+ return ret;
+ }
/* Write the page */
diff --git a/arch/arm/src/sama5/sam_pmecc.c b/arch/arm/src/sama5/sam_pmecc.c
index 58e6f3c..cc2f17e 100644
--- a/arch/arm/src/sama5/sam_pmecc.c
+++ b/arch/arm/src/sama5/sam_pmecc.c
@@ -1273,14 +1273,15 @@ int pmecc_configure(struct sam_nandcs_s *priv, bool protected)
* None
*
* Returned Value:
- * None
+ * Normally success (OK) is returned, but the error -ECANCELED may be
+ * return in the event that task has been canceled.
*
****************************************************************************/
#if NAND_NPMECC_BANKS > 1
-void pmecc_lock(void)
+int pmecc_lock(void)
{
- nxsem_wait_uninterruptible(&g_pmecc.exclsem);
+ return nxsem_wait_uninterruptible(&g_pmecc.exclsem);
}
#endif
diff --git a/arch/arm/src/sama5/sam_pmecc.h b/arch/arm/src/sama5/sam_pmecc.h
index c0d8944..133c33b 100644
--- a/arch/arm/src/sama5/sam_pmecc.h
+++ b/arch/arm/src/sama5/sam_pmecc.h
@@ -55,7 +55,9 @@
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
+
/* Configuration ************************************************************/
+
/* Block checking and H/W ECC support must be enabled for PMECC */
#ifndef CONFIG_MTD_NAND_HWECC
@@ -216,7 +218,7 @@ extern "C"
#endif
/****************************************************************************
- * Public Functions
+ * Public Function Prototypes
****************************************************************************/
/****************************************************************************
@@ -229,14 +231,15 @@ extern "C"
* None
*
* Returned Value:
- * None
+ * Normally success (OK) is returned, but the error -ECANCELED may be
+ * return in the event that task has been canceled.
*
****************************************************************************/
#if NAND_NPMECC_BANKS > 1
-void pmecc_lock(void);
+int pmecc_lock(void);
#else
-# define pmecc_lock()
+# define pmecc_lock() (0)
#endif
/****************************************************************************
@@ -394,7 +397,7 @@ uint32_t pmecc_get_pagesize(void);
****************************************************************************/
#ifdef CONFIG_SAMA5_PMECC_GALOIS_CUSTOM
-void pmecc_buildgf(uint32_t mm, int16_t* indexof, int16_t* alphato);
+void pmecc_buildgf(uint32_t mm, int16_t *indexof, int16_t *alphato);
#endif
#undef EXTERN
@@ -403,7 +406,9 @@ void pmecc_buildgf(uint32_t mm, int16_t* indexof, int16_t* alphato);
#endif
#else /* CONFIG_SAMA5_HAVE_PMECC */
+
/****************************************************************************/
+
/* Stub definitions to minimize conditional compilation when PMECC is
* disabled
*/
diff --git a/arch/arm/src/samv7/sam_mcan.c b/arch/arm/src/samv7/sam_mcan.c
index d9fb53d..adb189d 100644
--- a/arch/arm/src/samv7/sam_mcan.c
+++ b/arch/arm/src/samv7/sam_mcan.c
@@ -913,6 +913,7 @@ static void mcan_dumpregs(FAR struct sam_mcan_s *priv, FAR const char *msg);
/* Semaphore helpers */
static void mcan_dev_lock(FAR struct sam_mcan_s *priv);
+static void mcan_dev_lock_noncancelable(FAR struct sam_mcan_s *priv);
#define mcan_dev_unlock(priv) nxsem_post(&priv->locksem)
static void mcan_buffer_reserve(FAR struct sam_mcan_s *priv);
@@ -1367,13 +1368,48 @@ static void mcan_dumpregs(FAR struct sam_mcan_s *priv, FAR const char *msg)
* priv - A reference to the MCAN peripheral state
*
* Returned Value:
- * None
+ * Normally success (OK) is returned, but the error -ECANCELED may be
+ * return in the event that task has been canceled.
+ *
+ ****************************************************************************/
+
+static int mcan_dev_lock(FAR struct sam_mcan_s *priv)
+{
+ ret = nxsem_wait_uninterruptible(&priv->locksem);
+}
+
+/****************************************************************************
+ * Name: mcan_dev_lock_noncancelable
+ *
+ * Description:
+ * This is just a wrapper to handle the annoying behavior of semaphore
+ * waits that return due to the receipt of a signal. This version also
+ * ignores attempts to cancel the thread.
*
****************************************************************************/
-static void mcan_dev_lock(FAR struct sam_mcan_s *priv)
+static int mcan_dev_lock_noncancelable(FAR struct sam_can_s *priv)
{
- nxsem_wait_uninterruptible(&priv->locksem);
+ int result;
+ int ret = OK;
+
+ do
+ {
+ result = nxsem_wait_uninterruptible(&priv->exclsem);
+
+ /* The only expected error is ECANCELED which would occur if the
+ * calling thread were canceled.
+ */
+
+ DEBUGASSERT(result == OK || result == -ECANCELED);
+ if (ret == OK && result < 0)
+ {
+ ret = result;
+ }
+ }
+ while (result < 0);
+
+ return ret;
}
/****************************************************************************
@@ -1742,7 +1778,11 @@ static int mcan_add_extfilter(FAR struct sam_mcan_s *priv,
/* Get exclusive excess to the MCAN hardware */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
/* Find an unused standard filter */
@@ -1897,7 +1937,11 @@ static int mcan_del_extfilter(FAR struct sam_mcan_s *priv, int ndx)
/* Get exclusive excess to the MCAN hardware */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
word = ndx >> 5;
bit = ndx & 0x1f;
@@ -1997,13 +2041,18 @@ static int mcan_add_stdfilter(FAR struct sam_mcan_s *priv,
int word;
int bit;
int ndx;
+ int ret;
DEBUGASSERT(priv != NULL && priv->config != NULL);
config = priv->config;
/* Get exclusive excess to the MCAN hardware */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
/* Find an unused standard filter */
@@ -2137,6 +2186,7 @@ static int mcan_del_stdfilter(FAR struct sam_mcan_s *priv, int ndx)
uint32_t regval;
int word;
int bit;
+ int ret;
DEBUGASSERT(priv != NULL && priv->config != NULL);
config = priv->config;
@@ -2152,7 +2202,11 @@ static int mcan_del_stdfilter(FAR struct sam_mcan_s *priv, int ndx)
/* Get exclusive excess to the MCAN hardware */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
word = ndx >> 5;
bit = ndx & 0x1f;
@@ -2258,12 +2312,17 @@ static int mcan_del_stdfilter(FAR struct sam_mcan_s *priv, int ndx)
static int mcan_start_busoff_recovery_sequence(FAR struct sam_mcan_s *priv)
{
uint32_t regval;
+ int ret;
DEBUGASSERT(priv);
/* Get exclusive access to the MCAN peripheral */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
/* only start BUS-OFF recovery if we are in BUS-OFF state */
@@ -2315,7 +2374,7 @@ static void mcan_reset(FAR struct can_dev_s *dev)
/* Get exclusive access to the MCAN peripheral */
- mcan_dev_lock(priv);
+ mcan_dev_lock_noncancelable(priv);
/* Disable all interrupts */
@@ -2371,7 +2430,11 @@ static int mcan_setup(FAR struct can_dev_s *dev)
/* Get exclusive access to the MCAN peripheral */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return ret;
+ }
/* MCAN hardware initialization */
@@ -2450,7 +2513,7 @@ static void mcan_shutdown(FAR struct can_dev_s *dev)
/* Get exclusive access to the MCAN peripheral */
- mcan_dev_lock(priv);
+ mcan_dev_lock_noncancelable(priv);
/* Disable MCAN interrupts at the NVIC */
@@ -2861,6 +2924,7 @@ static int mcan_send(FAR struct can_dev_s *dev, FAR struct can_msg_s *msg)
unsigned int ndx;
unsigned int nbytes;
unsigned int i;
+ int ret;
DEBUGASSERT(dev);
priv = dev->cd_priv;
@@ -2897,7 +2961,14 @@ static int mcan_send(FAR struct can_dev_s *dev, FAR struct can_msg_s *msg)
/* Get exclusive access to the MCAN peripheral */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ mcan_buffer_release(priv);
+ sched_unlock();
+ return ret;
+ }
+
sched_unlock();
/* Get our reserved Tx FIFO/queue put index */
@@ -3016,10 +3087,15 @@ static bool mcan_txready(FAR struct can_dev_s *dev)
#ifdef CONFIG_DEBUG_FEATURES
int sval;
#endif
+ int ret;
/* Get exclusive access to the MCAN peripheral */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return false;
+ }
/* Return the state of the TX FIFOQ. Return TRUE if the TX FIFO/Queue is
* not full.
@@ -3077,7 +3153,11 @@ static bool mcan_txempty(FAR struct can_dev_s *dev)
/* Get exclusive access to the MCAN peripheral */
- mcan_dev_lock(priv);
+ ret = mcan_dev_lock(priv);
+ if (ret < 0)
+ {
+ return false;
+ }
/* Return the state of the TX FIFOQ. Return TRUE if the TX FIFO/Queue is
* empty. We don't have a reliable indication that the FIFO is empty, so