sigdeliver: fix system block when kill signal to idle in SMP

Bug description:

CONFIG_SMP=y

Suppose we have 2 cores in SMP, here is the ps return:

PID GROUP CPU PRI POLICY TYPE    NPX STATE     STACK   USED  FILLED COMMAND
  0     0   0   0 FIFO   Kthread N-- Assigned 004076 000748  18.3%  CPU0 IDLE
  1     0   1   0 FIFO   Kthread N-- Running  004096 000540  13.1%  CPU1 IDLE

nsh> kill -4 0
or:
nsh> kill -4 1

system blocked.

Reason:

In func xx_sigdeliver() restore stage, when saved_irqcount == 0, that means
rtcb NOT in critical_section before switch to xx_sigdeliver(), then we need
reset the critical_section state before swith back.

Fix:

Add condition to cover saved_irqcount == 0.

Change-Id: I4af7f95e47f6d78a4094c3757d39b01ac9d533b3
Signed-off-by: ligd <liguiding1@xiaomi.com>

arch/arm/src/armv6-m/arm_sigdeliver.c

@@ -1,35 +1,20 @@
 /****************************************************************************
  * arch/arm/src/armv6-m/arm_sigdeliver.c
  *
- *   Copyright (C) 2013-2015, 2018-2019 Gregory Nutt. All rights reserved.
+ * Licensed to the Apache Software Foundation (ASF) under one or more
- *   Author: Gregory Nutt <gnutt@nuttx.org>
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
  *
- * Redistribution and use in source and binary forms, with or without
+ *   http://www.apache.org/licenses/LICENSE-2.0
- * modification, are permitted provided that the following conditions
- * are met:
  *
- * 1. Redistributions of source code must retain the above copyright
+ * Unless required by applicable law or agreed to in writing, software
- *    notice, this list of conditions and the following disclaimer.
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * 2. Redistributions in binary form must reproduce the above copyright
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
- *    notice, this list of conditions and the following disclaimer in
+ * License for the specific language governing permissions and limitations
- *    the documentation and/or other materials provided with the
+ * under the License.
- *    distribution.
- * 3. Neither the name NuttX nor the names of its contributors may be
- *    used to endorse or promote products derived from this software
- *    without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
- * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
  *
  ****************************************************************************/
 
@@ -144,20 +129,19 @@ void arm_sigdeliver(void)
 
   sinfo("Resuming\n");
 
-  /* Call enter_critical_section() to disable local interrupts before
+#ifdef CONFIG_SMP
-   * restoring local context.
+  /* Restore the saved 'irqcount' and recover the critical section
-   *
+   * spinlocks.
-   * Here, we should not use up_irq_save() in SMP mode.
-   * For example, if we call up_irq_save() here and another CPU might
-   * have called up_cpu_pause() to this cpu, hence g_cpu_irqlock has
-   * been locked by the cpu, in this case, we would see a deadlock in
-   * later call of enter_critical_section() to restore irqcount.
-   * To avoid this situation, we need to call enter_critical_section().
    */
 
-#ifdef CONFIG_SMP
+  DEBUGASSERT(rtcb->irqcount == 0);
-  enter_critical_section();
+  while (rtcb->irqcount < saved_irqcount)
-#else
+    {
+      enter_critical_section();
+    }
+#endif
+
+#ifndef CONFIG_SUPPRESS_INTERRUPTS
   up_irq_save();
 #endif
 
@@ -183,22 +167,6 @@ void arm_sigdeliver(void)
 #endif
   rtcb->xcp.sigdeliver = NULL;  /* Allows next handler to be scheduled */
 
-#ifdef CONFIG_SMP
-  /* Restore the saved 'irqcount' and recover the critical section
-   * spinlocks.
-   *
-   * REVISIT:  irqcount should be one from the above call to
-   * enter_critical_section().  Could the saved_irqcount be zero?  That
-   * would be a problem.
-   */
-
-  DEBUGASSERT(rtcb->irqcount == 1);
-  while (rtcb->irqcount < saved_irqcount)
-    {
-      enter_critical_section();
-    }
-#endif
-
   /* Then restore the correct state for this thread of
    * execution.
    */

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

@@ -124,20 +124,19 @@ void arm_sigdeliver(void)
 
   sinfo("Resuming\n");
 
-  /* Call enter_critical_section() to disable local interrupts before
+#ifdef CONFIG_SMP
-   * restoring local context.
+  /* Restore the saved 'irqcount' and recover the critical section
-   *
+   * spinlocks.
-   * Here, we should not use up_irq_save() in SMP mode.
-   * For example, if we call up_irq_save() here and another CPU might
-   * have called up_cpu_pause() to this cpu, hence g_cpu_irqlock has
-   * been locked by the cpu, in this case, we would see a deadlock in
-   * later call of enter_critical_section() to restore irqcount.
-   * To avoid this situation, we need to call enter_critical_section().
    */
 
-#ifdef CONFIG_SMP
+  DEBUGASSERT(rtcb->irqcount == 0);
-  enter_critical_section();
+  while (rtcb->irqcount < saved_irqcount)
-#else
+    {
+      enter_critical_section();
+    }
+#endif
+
+#ifndef CONFIG_SUPPRESS_INTERRUPTS
   up_irq_save();
 #endif
 
@@ -159,22 +158,6 @@ void arm_sigdeliver(void)
   regs[REG_CPSR]       = rtcb->xcp.saved_cpsr;
   rtcb->xcp.sigdeliver = NULL;  /* Allows next handler to be scheduled */
 
-#ifdef CONFIG_SMP
-  /* Restore the saved 'irqcount' and recover the critical section
-   * spinlocks.
-   *
-   * REVISIT:  irqcount should be one from the above call to
-   * enter_critical_section().  Could the saved_irqcount be zero?  That
-   * would be a problem.
-   */
-
-  DEBUGASSERT(rtcb->irqcount == 1);
-  while (rtcb->irqcount < saved_irqcount)
-    {
-      enter_critical_section();
-    }
-#endif
-
   /* Then restore the correct state for this thread of execution. */
 
   board_autoled_off(LED_SIGNAL);

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

@@ -128,20 +128,19 @@ void arm_sigdeliver(void)
 
   sinfo("Resuming\n");
 
-  /* Call enter_critical_section() to disable local interrupts before
+#ifdef CONFIG_SMP
-   * restoring local context.
+  /* Restore the saved 'irqcount' and recover the critical section
-   *
+   * spinlocks.
-   * Here, we should not use up_irq_save() in SMP mode.
-   * For example, if we call up_irq_save() here and another CPU might
-   * have called up_cpu_pause() to this cpu, hence g_cpu_irqlock has
-   * been locked by the cpu, in this case, we would see a deadlock in
-   * later call of enter_critical_section() to restore irqcount.
-   * To avoid this situation, we need to call enter_critical_section().
    */
 
-#ifdef CONFIG_SMP
+  DEBUGASSERT(rtcb->irqcount == 0);
-  enter_critical_section();
+  while (rtcb->irqcount < saved_irqcount)
-#else
+    {
+      enter_critical_section();
+    }
+#endif
+
+#ifndef CONFIG_SUPPRESS_INTERRUPTS
   up_irq_save();
 #endif
 
@@ -171,22 +170,6 @@ void arm_sigdeliver(void)
 #endif
   rtcb->xcp.sigdeliver = NULL;  /* Allows next handler to be scheduled */
 
-#ifdef CONFIG_SMP
-  /* Restore the saved 'irqcount' and recover the critical section
-   * spinlocks.
-   *
-   * REVISIT:  irqcount should be one from the above call to
-   * enter_critical_section().  Could the saved_irqcount be zero?  That
-   * would be a problem.
-   */
-
-  DEBUGASSERT(rtcb->irqcount == 1);
-  while (rtcb->irqcount < saved_irqcount)
-    {
-      enter_critical_section();
-    }
-#endif
-
   /* Then restore the correct state for this thread of
    * execution.
    */

arch/arm/src/armv8-m/arm_sigdeliver.c

@@ -128,20 +128,19 @@ void arm_sigdeliver(void)
 
   sinfo("Resuming\n");
 
-  /* Call enter_critical_section() to disable local interrupts before
+#ifdef CONFIG_SMP
-   * restoring local context.
+  /* Restore the saved 'irqcount' and recover the critical section
-   *
+   * spinlocks.
-   * Here, we should not use up_irq_save() in SMP mode.
-   * For example, if we call up_irq_save() here and another CPU might
-   * have called up_cpu_pause() to this cpu, hence g_cpu_irqlock has
-   * been locked by the cpu, in this case, we would see a deadlock in
-   * later call of enter_critical_section() to restore irqcount.
-   * To avoid this situation, we need to call enter_critical_section().
    */
 
-#ifdef CONFIG_SMP
+  DEBUGASSERT(rtcb->irqcount == 0);
-  enter_critical_section();
+  while (rtcb->irqcount < saved_irqcount)
-#else
+    {
+      enter_critical_section();
+    }
+#endif
+
+#ifndef CONFIG_SUPPRESS_INTERRUPTS
   up_irq_save();
 #endif
 
@@ -171,22 +170,6 @@ void arm_sigdeliver(void)
 #endif
   rtcb->xcp.sigdeliver = NULL;  /* Allows next handler to be scheduled */
 
-#ifdef CONFIG_SMP
-  /* Restore the saved 'irqcount' and recover the critical section
-   * spinlocks.
-   *
-   * REVISIT:  irqcount should be one from the above call to
-   * enter_critical_section().  Could the saved_irqcount be zero?  That
-   * would be a problem.
-   */
-
-  DEBUGASSERT(rtcb->irqcount == 1);
-  while (rtcb->irqcount < saved_irqcount)
-    {
-      enter_critical_section();
-    }
-#endif
-
   /* Then restore the correct state for this thread of
    * execution.
    */

arch/risc-v/src/rv64gc/riscv_sigdeliver.c

@@ -144,20 +144,19 @@ void up_sigdeliver(void)
   sinfo("Resuming EPC: %016" PRIx64 " INT_CTX: %016" PRIx64 "\n",
         regs[REG_EPC], regs[REG_INT_CTX]);
 
-  /* Call enter_critical_section() to disable local interrupts before
+#ifdef CONFIG_SMP
-   * restoring local context.
+  /* Restore the saved 'irqcount' and recover the critical section
-   *
+   * spinlocks.
-   * Here, we should not use up_irq_save() in SMP mode.
-   * For example, if we call up_irq_save() here and another CPU might
-   * have called up_cpu_pause() to this cpu, hence g_cpu_irqlock has
-   * been locked by the cpu, in this case, we would see a deadlock in
-   * later call of enter_critical_section() to restore irqcount.
-   * To avoid this situation, we need to call enter_critical_section().
    */
 
-#ifdef CONFIG_SMP
+  DEBUGASSERT(rtcb->irqcount == 0);
-  enter_critical_section();
+  while (rtcb->irqcount < saved_irqcount)
-#else
+    {
+      (void)enter_critical_section();
+    }
+#endif
+
+#ifndef CONFIG_SUPPRESS_INTERRUPTS
   up_irq_save();
 #endif
 
@@ -179,22 +178,6 @@ void up_sigdeliver(void)
   regs[REG_INT_CTX]    = rtcb->xcp.saved_int_ctx;
   rtcb->xcp.sigdeliver = NULL;  /* Allows next handler to be scheduled */
 
-#ifdef CONFIG_SMP
-  /* Restore the saved 'irqcount' and recover the critical section
-   * spinlocks.
-   *
-   * REVISIT:  irqcount should be one from the above call to
-   * enter_critical_section().  Could the saved_irqcount be zero?  That
-   * would be a problem.
-   */
-
-  DEBUGASSERT(rtcb->irqcount == 1);
-  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

@@ -121,20 +121,19 @@ void xtensa_sig_deliver(void)
 
   sinfo("Resuming\n");
 
-  /* Call enter_critical_section() to disable local interrupts before
+#ifdef CONFIG_SMP
-   * restoring local context.
+  /* Restore the saved 'irqcount' and recover the critical section
-   *
+   * spinlocks.
-   * Here, we should not use up_irq_save() in SMP mode.
-   * For example, if we call up_irq_save() here and another CPU might
-   * have called up_cpu_pause() to this cpu, hence g_cpu_irqlock has
-   * been locked by the cpu, in this case, we would see a deadlock in
-   * later call of enter_critical_section() to restore irqcount.
-   * To avoid this situation, we need to call enter_critical_section().
    */
 
-#ifdef CONFIG_SMP
+  DEBUGASSERT(rtcb->irqcount == 0);
-  enter_critical_section();
+  while (rtcb->irqcount < saved_irqcount)
-#else
+    {
+      enter_critical_section();
+    }
+#endif
+
+#ifndef CONFIG_SUPPRESS_INTERRUPTS
   up_irq_save();
 #endif
 
@@ -190,22 +189,6 @@ void xtensa_sig_deliver(void)
 
   rtcb->xcp.regs[REG_A0] = regs[REG_A0];
 
-#ifdef CONFIG_SMP
-  /* Restore the saved 'irqcount' and recover the critical section
-   * spinlocks.
-   *
-   * REVISIT:  irqcount should be one from the above call to
-   * enter_critical_section().  Could the saved_irqcount be zero?  That
-   * would be a problem.
-   */
-
-  DEBUGASSERT(rtcb->irqcount == 1);
-  while (rtcb->irqcount < saved_irqcount)
-    {
-      enter_critical_section();
-    }
-#endif
-
   /* Then restore the correct state for this thread of execution.
    * NOTE: The co-processor state should already be correct.
    */