diff --git a/arch/arm/src/armv7-a/arm_schedulesigaction.c b/arch/arm/src/armv7-a/arm_schedulesigaction.c index 2bb30374a39..752ae63667a 100644 --- a/arch/arm/src/armv7-a/arm_schedulesigaction.c +++ b/arch/arm/src/armv7-a/arm_schedulesigaction.c @@ -121,7 +121,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) if (!CURRENT_REGS) { - /* In this case just deliver the signal now. */ + /* In this case just deliver the signal now. + * REVISIT: Signal handler will run in a critical section! + */ sigdeliver(tcb); } @@ -227,7 +229,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) if (cpu == me && !CURRENT_REGS) { - /* In this case just deliver the signal now. */ + /* In this case just deliver the signal now. + * REVISIT: Signal handler will run in a critical section! + */ sigdeliver(tcb); } diff --git a/arch/arm/src/armv7-a/arm_sigdeliver.c b/arch/arm/src/armv7-a/arm_sigdeliver.c index 861e065f7d7..1dc74093cbf 100644 --- a/arch/arm/src/armv7-a/arm_sigdeliver.c +++ b/arch/arm/src/armv7-a/arm_sigdeliver.c @@ -101,9 +101,24 @@ void up_sigdeliver(void) sigdeliver = rtcb->xcp.sigdeliver; rtcb->xcp.sigdeliver = NULL; +#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. + */ + + leave_critical_section(regs[REG_CPSR]); +#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 would be an issue if the signal handler were to suspend because + * the critical section would be re-established when the signal handler + * resumes. */ up_irq_enable(); diff --git a/arch/arm/src/armv7-m/up_schedulesigaction.c b/arch/arm/src/armv7-m/up_schedulesigaction.c index 698b8a3c2c9..fd682294ba1 100644 --- a/arch/arm/src/armv7-m/up_schedulesigaction.c +++ b/arch/arm/src/armv7-m/up_schedulesigaction.c @@ -123,7 +123,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) if (!CURRENT_REGS) { - /* In this case just deliver the signal now. */ + /* In this case just deliver the signal now. + * REVISIT: Signal handle will run in a critical section! + */ sigdeliver(tcb); } @@ -255,7 +257,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) if (cpu == me && !CURRENT_REGS) { - /* In this case just deliver the signal now. */ + /* In this case just deliver the signal now. + * REVISIT: Signal handler will run in a critical section! + */ sigdeliver(tcb); } diff --git a/arch/arm/src/armv7-m/up_sigdeliver.c b/arch/arm/src/armv7-m/up_sigdeliver.c index 6ecd0ee66dc..eedb1f22f6d 100644 --- a/arch/arm/src/armv7-m/up_sigdeliver.c +++ b/arch/arm/src/armv7-m/up_sigdeliver.c @@ -110,9 +110,28 @@ void up_sigdeliver(void) sigdeliver = (sig_deliver_t)rtcb->xcp.sigdeliver; rtcb->xcp.sigdeliver = NULL; +#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. + */ + +#ifdef CONFIG_ARMV7M_USEBASEPRI + leave_critical_section((uint8_t)regs[REG_BASEPRI]); +#else + leave_critical_section((uint16_t)regs[REG_PRIMASK]); +#endif +#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 would be an issue if the signal handler were to suspend because + * the critical section would be re-established when the signal handler + * resumes. */ up_irq_enable(); diff --git a/arch/xtensa/src/common/xtensa_schedsigaction.c b/arch/xtensa/src/common/xtensa_schedsigaction.c index 9747d201d34..bbddd01ef6f 100644 --- a/arch/xtensa/src/common/xtensa_schedsigaction.c +++ b/arch/xtensa/src/common/xtensa_schedsigaction.c @@ -119,7 +119,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) if (!CURRENT_REGS) { - /* In this case just deliver the signal now. */ + /* In this case just deliver the signal now. + * REVISIT: Signal handler will run in a critical section! + */ sigdeliver(tcb); } @@ -235,7 +237,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) if (cpu == me && !CURRENT_REGS) { - /* In this case just deliver the signal now. */ + /* In this case just deliver the signal now. + * REVISIT: Signal handler will run in a critical section! + */ sigdeliver(tcb); } diff --git a/arch/xtensa/src/common/xtensa_sigdeliver.c b/arch/xtensa/src/common/xtensa_sigdeliver.c index 9aed74990bf..315903a11a6 100644 --- a/arch/xtensa/src/common/xtensa_sigdeliver.c +++ b/arch/xtensa/src/common/xtensa_sigdeliver.c @@ -100,9 +100,24 @@ void xtensa_sig_deliver(void) sigdeliver = rtcb->xcp.sigdeliver; rtcb->xcp.sigdeliver = NULL; +#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. + */ + + leave_critical_section((regs[REG_PS])); +#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 would be an issue if the signal handler were to suspend because + * the critical section would be re-established when the signal handler + * resumes. */ up_irq_enable(); diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h index 8f80165d762..f915e320104 100644 --- a/include/nuttx/sched.h +++ b/include/nuttx/sched.h @@ -615,7 +615,7 @@ struct tcb_s uint16_t flags; /* Misc. general status flags */ int16_t lockcount; /* 0=preemptable (not-locked) */ #ifdef CONFIG_SMP - int16_t irqcount; /* 0=interrupts enabled */ + int16_t irqcount; /* 0=Not in critical section */ #endif #ifdef CONFIG_CANCELLATION_POINTS int16_t cpcount; /* Nested cancellation point count */