From cb183d12bab6449cee502ae6897751939bdd432c Mon Sep 17 00:00:00 2001 From: hujun5 Date: Fri, 8 Mar 2024 13:54:13 +0800 Subject: [PATCH] signal: fix deadlock when sigdeliver call enter_critical_section cpu0 cpu1: user_main signest_test sched_unlock nxsched_merge_pending nxsched_add_readytorun up_cpu_pause arm_sigdeliver enter_critical_section Reason: In the SMP, cpu0 is already in the critical section and waiting for cpu1 to enter the suspended state. However, when cpu1 executes arm_sigdeliver, it is in the irq-disabled state but not in the critical section. At this point, cpu1 is unable to respond to interrupts and is continuously attempting to enter the critical section, resulting in a deadlock. Resolve: adjust the logic, do not entering the critical section when interrupt-disabled. test: We can use qemu for testing. compiling make distclean -j20; ./tools/configure.sh -l qemu-armv8a:nsh_smp ;make -j20 running qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic -machine virt,virtualization=on,gic-version=3 -net none -chardev stdio,id=con,mux=on -serial chardev:con -mon chardev=con,mode=readline -kernel ./nuttx Signed-off-by: hujun5 --- arch/arm/src/armv6-m/arm_sigdeliver.c | 12 ++++++------ arch/arm/src/armv7-a/arm_sigdeliver.c | 12 ++++++------ arch/arm/src/armv7-m/arm_sigdeliver.c | 16 ++++++++++------ arch/arm/src/armv7-r/arm_sigdeliver.c | 12 ++++++------ arch/arm/src/armv8-m/arm_sigdeliver.c | 16 ++++++++++------ arch/arm/src/armv8-r/arm_sigdeliver.c | 12 ++++++++---- arch/arm64/src/common/arm64_sigdeliver.c | 11 ++++++----- arch/risc-v/src/common/riscv_sigdeliver.c | 12 ++++++------ arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c | 10 +++++----- arch/xtensa/src/common/xtensa_sigdeliver.c | 12 ++++++------ 10 files changed, 69 insertions(+), 56 deletions(-) diff --git a/arch/arm/src/armv6-m/arm_sigdeliver.c b/arch/arm/src/armv6-m/arm_sigdeliver.c index ee0158701452f..d8390b375ca23 100644 --- a/arch/arm/src/armv6-m/arm_sigdeliver.c +++ b/arch/arm/src/armv6-m/arm_sigdeliver.c @@ -63,8 +63,6 @@ void arm_sigdeliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -81,17 +79,16 @@ void arm_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { leave_critical_section((uint16_t)regs[REG_PRIMASK]); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -122,7 +119,7 @@ void arm_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -135,6 +132,9 @@ void arm_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section((uint16_t)regs[REG_PRIMASK]); +#endif goto retry; } diff --git a/arch/arm/src/armv7-a/arm_sigdeliver.c b/arch/arm/src/armv7-a/arm_sigdeliver.c index 95b19259a12c5..e1777e5ae256e 100644 --- a/arch/arm/src/armv7-a/arm_sigdeliver.c +++ b/arch/arm/src/armv7-a/arm_sigdeliver.c @@ -63,8 +63,6 @@ void arm_sigdeliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -81,17 +79,16 @@ void arm_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { leave_critical_section(regs[REG_CPSR]); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -122,7 +119,7 @@ void arm_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -135,6 +132,9 @@ void arm_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section(regs[REG_CPSR]); +#endif goto retry; } diff --git a/arch/arm/src/armv7-m/arm_sigdeliver.c b/arch/arm/src/armv7-m/arm_sigdeliver.c index 414d6272bbf80..961ca31b59880 100644 --- a/arch/arm/src/armv7-m/arm_sigdeliver.c +++ b/arch/arm/src/armv7-m/arm_sigdeliver.c @@ -63,8 +63,6 @@ void arm_sigdeliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -81,13 +79,13 @@ void arm_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { #ifdef CONFIG_ARMV7M_USEBASEPRI leave_critical_section((uint8_t)regs[REG_BASEPRI]); @@ -95,7 +93,6 @@ void arm_sigdeliver(void) leave_critical_section((uint16_t)regs[REG_PRIMASK]); #endif } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -126,7 +123,7 @@ void arm_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -139,6 +136,13 @@ void arm_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP +# ifdef CONFIG_ARMV7M_USEBASEPRI + leave_critical_section((uint8_t)regs[REG_BASEPRI]); +# else + leave_critical_section((uint16_t)regs[REG_PRIMASK]); +# endif +#endif goto retry; } diff --git a/arch/arm/src/armv7-r/arm_sigdeliver.c b/arch/arm/src/armv7-r/arm_sigdeliver.c index b1504103a7330..c21b77ed10b17 100644 --- a/arch/arm/src/armv7-r/arm_sigdeliver.c +++ b/arch/arm/src/armv7-r/arm_sigdeliver.c @@ -63,8 +63,6 @@ void arm_sigdeliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -81,17 +79,16 @@ void arm_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { leave_critical_section(regs[REG_CPSR]); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -119,7 +116,7 @@ void arm_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -132,6 +129,9 @@ void arm_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section(regs[REG_CPSR]); +#endif goto retry; } diff --git a/arch/arm/src/armv8-m/arm_sigdeliver.c b/arch/arm/src/armv8-m/arm_sigdeliver.c index e41baa02a8f59..f8588136fbc97 100644 --- a/arch/arm/src/armv8-m/arm_sigdeliver.c +++ b/arch/arm/src/armv8-m/arm_sigdeliver.c @@ -63,8 +63,6 @@ void arm_sigdeliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -81,13 +79,13 @@ void arm_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { #ifdef CONFIG_ARMV8M_USEBASEPRI leave_critical_section((uint8_t)regs[REG_BASEPRI]); @@ -95,7 +93,6 @@ void arm_sigdeliver(void) leave_critical_section((uint16_t)regs[REG_PRIMASK]); #endif } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -126,7 +123,7 @@ void arm_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -139,6 +136,13 @@ void arm_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP +# ifdef CONFIG_ARMV8M_USEBASEPRI + leave_critical_section((uint8_t)regs[REG_BASEPRI]); +# else + leave_critical_section((uint16_t)regs[REG_PRIMASK]); +# endif +#endif goto retry; } diff --git a/arch/arm/src/armv8-r/arm_sigdeliver.c b/arch/arm/src/armv8-r/arm_sigdeliver.c index 809da73a1719a..e249869434dd1 100644 --- a/arch/arm/src/armv8-r/arm_sigdeliver.c +++ b/arch/arm/src/armv8-r/arm_sigdeliver.c @@ -79,17 +79,16 @@ void arm_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { leave_critical_section(regs[REG_CPSR]); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -117,7 +116,7 @@ void arm_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -130,6 +129,9 @@ void arm_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section(regs[REG_CPSR]); +#endif goto retry; } @@ -149,6 +151,8 @@ void arm_sigdeliver(void) board_autoled_off(LED_SIGNAL); #ifdef CONFIG_SMP + rtcb->irqcount++; + leave_critical_section(regs[REG_CPSR]); rtcb->irqcount--; #endif arm_fullcontextrestore(regs); diff --git a/arch/arm64/src/common/arm64_sigdeliver.c b/arch/arm64/src/common/arm64_sigdeliver.c index c3249aa6be16c..15e7946c7c007 100644 --- a/arch/arm64/src/common/arm64_sigdeliver.c +++ b/arch/arm64/src/common/arm64_sigdeliver.c @@ -71,7 +71,6 @@ void arm64_sigdeliver(void) struct regs_context *pctx = (struct regs_context *)rtcb->xcp.saved_reg; flags = (pctx->spsr & SPSR_DAIF_MASK); - enter_critical_section(); #endif sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n", @@ -86,17 +85,16 @@ void arm64_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { leave_critical_section(flags); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -127,7 +125,7 @@ void arm64_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -140,6 +138,9 @@ void arm64_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section(flags); +#endif goto retry; } diff --git a/arch/risc-v/src/common/riscv_sigdeliver.c b/arch/risc-v/src/common/riscv_sigdeliver.c index 6e1a70641ad3c..9caed498de767 100644 --- a/arch/risc-v/src/common/riscv_sigdeliver.c +++ b/arch/risc-v/src/common/riscv_sigdeliver.c @@ -64,8 +64,6 @@ void riscv_sigdeliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -82,17 +80,16 @@ void riscv_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { leave_critical_section(regs[REG_INT_CTX]); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -121,7 +118,7 @@ void riscv_sigdeliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -134,6 +131,9 @@ void riscv_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section(regs[REG_INT_CTX]); +#endif goto retry; } diff --git a/arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c b/arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c index 4c6f4295b6339..06fa3c5b7770f 100644 --- a/arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c +++ b/arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c @@ -72,8 +72,6 @@ void sparc_sigdeliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -94,17 +92,16 @@ void sparc_sigdeliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_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 + while (rtcb->irqcount > 0) { leave_critical_section((regs[REG_PSR])); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -150,6 +147,9 @@ void sparc_sigdeliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section((regs[REG_PSR])); +#endif goto retry; } diff --git a/arch/xtensa/src/common/xtensa_sigdeliver.c b/arch/xtensa/src/common/xtensa_sigdeliver.c index 39b9dab832a56..97c4f3ded9330 100644 --- a/arch/xtensa/src/common/xtensa_sigdeliver.c +++ b/arch/xtensa/src/common/xtensa_sigdeliver.c @@ -63,8 +63,6 @@ void xtensa_sig_deliver(void) */ int16_t saved_irqcount; - - enter_critical_section(); #endif board_autoled_on(LED_SIGNAL); @@ -81,18 +79,17 @@ void xtensa_sig_deliver(void) */ saved_irqcount = rtcb->irqcount; - DEBUGASSERT(saved_irqcount >= 1); + DEBUGASSERT(saved_irqcount >= 0); /* Now we need to call leave_critical_section() repeatedly to get the * irqcount to zero, freeing all global spinlocks that enforce the critical * section. */ - do + while (rtcb->irqcount > 0) { leave_critical_section((regs[REG_PS])); } - while (rtcb->irqcount > 0); #endif /* CONFIG_SMP */ #ifndef CONFIG_SUPPRESS_INTERRUPTS @@ -120,7 +117,7 @@ void xtensa_sig_deliver(void) */ DEBUGASSERT(rtcb->irqcount == 0); - while (rtcb->irqcount < saved_irqcount) + while (rtcb->irqcount < saved_irqcount + 1) { enter_critical_section(); } @@ -133,6 +130,9 @@ void xtensa_sig_deliver(void) if (!sq_empty(&rtcb->sigpendactionq) && (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0) { +#ifdef CONFIG_SMP + leave_critical_section((regs[REG_PS])); +#endif goto retry; }