You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ag...@apache.org on 2021/03/03 15:03:41 UTC
[incubator-nuttx] 02/02: sigdeliver: fix system block when kill
signal to idle in SMP
This is an automated email from the ASF dual-hosted git repository.
aguettouche pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit f9d20ea4d2c5e5e6f173ddacbab08b6e252af79f
Author: ligd <li...@xiaomi.com>
AuthorDate: Wed Jan 20 21:26:28 2021 +0800
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 <li...@xiaomi.com>
---
arch/arm/src/armv6-m/arm_sigdeliver.c | 78 +++++++++---------------------
arch/arm/src/armv7-a/arm_sigdeliver.c | 39 +++++----------
arch/arm/src/armv7-m/arm_sigdeliver.c | 39 +++++----------
arch/arm/src/armv8-m/arm_sigdeliver.c | 39 +++++----------
arch/risc-v/src/rv64gc/riscv_sigdeliver.c | 39 +++++----------
arch/xtensa/src/common/xtensa_sigdeliver.c | 39 +++++----------
6 files changed, 78 insertions(+), 195 deletions(-)
diff --git a/arch/arm/src/armv6-m/arm_sigdeliver.c b/arch/arm/src/armv6-m/arm_sigdeliver.c
index 186b843..28b73eb 100644
--- a/arch/arm/src/armv6-m/arm_sigdeliver.c
+++ b/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.
- * Author: Gregory Nutt <gn...@nuttx.org>
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * 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
- * modification, are permitted provided that the following conditions
- * are met:
+ * http://www.apache.org/licenses/LICENSE-2.0
*
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in
- * the documentation and/or other materials provided with the
- * 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.
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
*
****************************************************************************/
@@ -144,20 +129,19 @@ void arm_sigdeliver(void)
sinfo("Resuming\n");
- /* Call enter_critical_section() to disable local interrupts before
- * restoring local context.
- *
- * 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
+ /* Restore the saved 'irqcount' and recover the critical section
+ * spinlocks.
*/
-#ifdef CONFIG_SMP
- enter_critical_section();
-#else
+ DEBUGASSERT(rtcb->irqcount == 0);
+ while (rtcb->irqcount < saved_irqcount)
+ {
+ 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.
*/
diff --git a/arch/arm/src/armv7-a/arm_sigdeliver.c b/arch/arm/src/armv7-a/arm_sigdeliver.c
index 8ceef66..20baf6a 100644
--- a/arch/arm/src/armv7-a/arm_sigdeliver.c
+++ b/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
- * restoring local context.
- *
- * 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
+ /* Restore the saved 'irqcount' and recover the critical section
+ * spinlocks.
*/
-#ifdef CONFIG_SMP
- enter_critical_section();
-#else
+ DEBUGASSERT(rtcb->irqcount == 0);
+ while (rtcb->irqcount < saved_irqcount)
+ {
+ 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);
diff --git a/arch/arm/src/armv7-m/arm_sigdeliver.c b/arch/arm/src/armv7-m/arm_sigdeliver.c
index d62743d..50dbec3 100644
--- a/arch/arm/src/armv7-m/arm_sigdeliver.c
+++ b/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
- * restoring local context.
- *
- * 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
+ /* Restore the saved 'irqcount' and recover the critical section
+ * spinlocks.
*/
-#ifdef CONFIG_SMP
- enter_critical_section();
-#else
+ DEBUGASSERT(rtcb->irqcount == 0);
+ while (rtcb->irqcount < saved_irqcount)
+ {
+ 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.
*/
diff --git a/arch/arm/src/armv8-m/arm_sigdeliver.c b/arch/arm/src/armv8-m/arm_sigdeliver.c
index 88b7f71..cb5a2de 100644
--- a/arch/arm/src/armv8-m/arm_sigdeliver.c
+++ b/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
- * restoring local context.
- *
- * 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
+ /* Restore the saved 'irqcount' and recover the critical section
+ * spinlocks.
*/
-#ifdef CONFIG_SMP
- enter_critical_section();
-#else
+ DEBUGASSERT(rtcb->irqcount == 0);
+ while (rtcb->irqcount < saved_irqcount)
+ {
+ 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.
*/
diff --git a/arch/risc-v/src/rv64gc/riscv_sigdeliver.c b/arch/risc-v/src/rv64gc/riscv_sigdeliver.c
index dc0cc43..4fd3cf6 100644
--- a/arch/risc-v/src/rv64gc/riscv_sigdeliver.c
+++ b/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
- * restoring local context.
- *
- * 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
+ /* Restore the saved 'irqcount' and recover the critical section
+ * spinlocks.
*/
-#ifdef CONFIG_SMP
- enter_critical_section();
-#else
+ DEBUGASSERT(rtcb->irqcount == 0);
+ while (rtcb->irqcount < saved_irqcount)
+ {
+ (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.
*/
diff --git a/arch/xtensa/src/common/xtensa_sigdeliver.c b/arch/xtensa/src/common/xtensa_sigdeliver.c
index 92879f7..94a2484 100644
--- a/arch/xtensa/src/common/xtensa_sigdeliver.c
+++ b/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
- * restoring local context.
- *
- * 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
+ /* Restore the saved 'irqcount' and recover the critical section
+ * spinlocks.
*/
-#ifdef CONFIG_SMP
- enter_critical_section();
-#else
+ DEBUGASSERT(rtcb->irqcount == 0);
+ while (rtcb->irqcount < saved_irqcount)
+ {
+ 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.
*/