nuttx-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From acas...@apache.org
Subject [incubator-nuttx] 02/03: Check return from nxsem_wait_uninterruptible()
Date Sun, 05 Apr 2020 15:32:17 GMT
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 <gnutt@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


Mime
View raw message