arch/: Correct critical section logic associated with the recent signal handler changes (yet again).

arch/arm/src/armv7-a/arm_sigdeliver.c

@@ -81,6 +81,15 @@ void up_sigdeliver(void)
 
   int saved_errno = rtcb->pterrno;
 
+#ifdef CONFIG_SMP
+  /* In the SMP case, we must terminate the critical section while the signal
+   * handler executes, but we also need to restore the irqcount when the
+   * we resume the main thread of the task.
+   */
+
+  int16_t saved_irqcount;
+#endif
+
   board_autoled_on(LED_SIGNAL);
 
   sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
@@ -103,22 +112,27 @@ void up_sigdeliver(void)
 
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
-   * 'irqcount' in order to force us into a critical section.  At a minimum,
+   * 'irqcount' in order to force us into a critical section.  Save the
-   * we must call leave_critical_section() at least once in order to
+   * pre-incremented irqcount.
-   * compensate for that.
    */
 
-  leave_critical_section(regs[REG_CPSR]);
+  saved_irqcount       = rtcb->irqcount - 1;
+  DEBUGASSERT(saved_irqcount >= 0);
+
+  /* Now we need call leave_critical_section() repeatedly to get the irqcount
+   * to zero, freeing all global spinlocks that enforce the critical section.
+   */
+
+  do
+    {
+      leave_critical_section(regs[REG_CPSR]);
+    }
+  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
   /* Then make sure that interrupts are enabled.  Signal handlers must always
    * run with interrupts enabled.
-   *
-   * REVISIT: 'irqcount' could still be greater than zero in the SMP case.
-   * This is an issue because (1) global spinlocks are still held and (2) if
-   * the signal handler were to suspend the the critical section would be
-   * re-established when the signal handler resumes.
    */
 
   up_irq_enable();
@@ -144,8 +158,23 @@ void up_sigdeliver(void)
   sinfo("Resuming\n");
 
   (void)up_irq_save();
+
+  /* Restore the saved errno value */
+
   rtcb->pterrno = saved_errno;
 
+#ifdef CONFIG_SMP
+  /* Restore the saved 'irqcount' and recover the critical section
+   * spinlocks.
+   */
+
+  DEBUGASSERT(rtcb->irqcount == 0);
+  while (rtcb->irqcount < saved_irqcount)
+    {
+      (void)enter_critical_section();
+    }
+#endif
+
   /* Then restore the correct state for this thread of execution. */
 
   board_autoled_off(LED_SIGNAL);

arch/arm/src/armv7-m/up_sigdeliver.c

@@ -82,6 +82,15 @@ void up_sigdeliver(void)
 
   int saved_errno = rtcb->pterrno;
 
+#ifdef CONFIG_SMP
+  /* In the SMP case, we must terminate the critical section while the signal
+   * handler executes, but we also need to restore the irqcount when the
+   * we resume the main thread of the task.
+   */
+
+  int16_t saved_irqcount;
+#endif
+
   board_autoled_on(LED_SIGNAL);
 
   sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
@@ -112,26 +121,31 @@ void up_sigdeliver(void)
 
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
-   * 'irqcount' in order to force us into a critical section.  At a minimum,
+   * 'irqcount' in order to force us into a critical section.  Save the
-   * we must call leave_critical_section() at least once in order to
+   * pre-incremented irqcount.
-   * compensate for that.
    */
 
+  saved_irqcount       = rtcb->irqcount - 1;
+  DEBUGASSERT(saved_irqcount >= 0);
+
+  /* Now we need call leave_critical_section() repeatedly to get the irqcount
+   * to zero, freeing all global spinlocks that enforce the critical section.
+   */
+
+  do
+    {
 #ifdef CONFIG_ARMV7M_USEBASEPRI
-  leave_critical_section((uint8_t)regs[REG_BASEPRI]);
+      leave_critical_section((uint8_t)regs[REG_BASEPRI]);
 #else
-  leave_critical_section((uint16_t)regs[REG_PRIMASK]);
+      leave_critical_section((uint16_t)regs[REG_PRIMASK]);
 #endif
+    }
+  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
   /* Then make sure that interrupts are enabled.  Signal handlers must always
    * run with interrupts enabled.
-   *
-   * REVISIT: 'irqcount' could still be greater than zero in the SMP case.
-   * This is an issue because (1) global spinlocks are still held and (2) if
-   * the signal handler were to suspend the the critical section would be
-   * re-established when the signal handler resumes.
    */
 
   up_irq_enable();
@@ -157,8 +171,23 @@ void up_sigdeliver(void)
   sinfo("Resuming\n");
 
   (void)up_irq_save();
+
+  /* Restore the saved errno value */
+
   rtcb->pterrno = saved_errno;
 
+#ifdef CONFIG_SMP
+  /* Restore the saved 'irqcount' and recover the critical section
+   * spinlocks.
+   */
+
+  DEBUGASSERT(rtcb->irqcount == 0);
+  while (rtcb->irqcount < saved_irqcount)
+    {
+      (void)enter_critical_section();
+    }
+#endif
+
   /* Then restore the correct state for this thread of
    * execution.
    */

arch/xtensa/src/common/xtensa_sigdeliver.c

@@ -80,6 +80,15 @@ void xtensa_sig_deliver(void)
 
   int saved_errno = rtcb->pterrno;
 
+#ifdef CONFIG_SMP
+  /* In the SMP case, we must terminate the critical section while the signal
+   * handler executes, but we also need to restore the irqcount when the
+   * we resume the main thread of the task.
+   */
+
+  int16_t saved_irqcount;
+#endif
+
   board_autoled_on(LED_SIGNAL);
 
   sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
@@ -102,22 +111,27 @@ void xtensa_sig_deliver(void)
 
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
-   * 'irqcount' in order to force us into a critical section.  At a minimum,
+   * 'irqcount' in order to force us into a critical section.  Save the
-   * we must call leave_critical_section() at least once in order to
+   * pre-incremented irqcount.
-   * compensate for that.
    */
 
-  leave_critical_section((regs[REG_PS]));
+  saved_irqcount       = rtcb->irqcount - 1;
+  DEBUGASSERT(saved_irqcount >= 0);
+
+  /* Now we need call leave_critical_section() repeatedly to get the irqcount
+   * to zero, freeing all global spinlocks that enforce the critical section.
+   */
+
+  do
+    {
+      leave_critical_section((regs[REG_PS]));
+    }
+  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
   /* Then make sure that interrupts are enabled.  Signal handlers must always
    * run with interrupts enabled.
-   *
-   * REVISIT: 'irqcount' could still be greater than zero in the SMP case.
-   * This is an issue because (1) global spinlocks are still held and (2) if
-   * the signal handler were to suspend the the critical section would be
-   * re-established when the signal handler resumes.
    */
 
   up_irq_enable();
@@ -134,8 +148,23 @@ void xtensa_sig_deliver(void)
 
   sinfo("Resuming\n");
   (void)up_irq_save();
+
+  /* Restore the saved errno value */
+
   rtcb->pterrno = saved_errno;
 
+#ifdef CONFIG_SMP
+  /* Restore the saved 'irqcount' and recover the critical section
+   * spinlocks.
+   */
+
+  DEBUGASSERT(rtcb->irqcount == 0);
+  while (rtcb->irqcount < saved_irqcount)
+    {
+      (void)enter_critical_section();
+    }
+#endif
+
   /* Then restore the correct state for this thread of execution.
    * NOTE: The co-processor state should already be correct.
    */