diff --git a/arch/arm/src/armv7-a/arm_sigdeliver.c b/arch/arm/src/armv7-a/arm_sigdeliver.c index f6afdb45009..daef2f9d92e 100644 --- a/arch/arm/src/armv7-a/arm_sigdeliver.c +++ b/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, - * we must call leave_critical_section() at least once in order to - * compensate for that. + * 'irqcount' in order to force us into a critical section. Save the + * pre-incremented irqcount. */ - 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); diff --git a/arch/arm/src/armv7-m/up_sigdeliver.c b/arch/arm/src/armv7-m/up_sigdeliver.c index 577dff8ab48..b8c2fe42305 100644 --- a/arch/arm/src/armv7-m/up_sigdeliver.c +++ b/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, - * we must call leave_critical_section() at least once in order to - * compensate for that. + * 'irqcount' in order to force us into a critical section. Save the + * pre-incremented irqcount. */ + 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. */ diff --git a/arch/xtensa/src/common/xtensa_sigdeliver.c b/arch/xtensa/src/common/xtensa_sigdeliver.c index b298ac0ad9f..bc8de1f8ae1 100644 --- a/arch/xtensa/src/common/xtensa_sigdeliver.c +++ b/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, - * we must call leave_critical_section() at least once in order to - * compensate for that. + * 'irqcount' in order to force us into a critical section. Save the + * pre-incremented irqcount. */ - 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. */