Most interrupt handling logic interacts with tasks via standard mechanism such as sem_post, sigqueue, mq_send, etc. This all call enter_critical_section and are assumed to be safe in the SMP case.

But certain logic interacts with tasks in different ways.  The only one that comes to mind are wdogs.  There is a tasking interface that to manipulate wdogs, and a different interface in the timer interrupt handling logic to manage wdog expirations.

In the normal case, this is fine.  Since the tasking level code calls enter_critical_section, interrupts are disabled an no conflicts can occur.  But that may not be the case in the SMP case.  Most architectures do not permit disabling interrupts on other CPUs so enter_critical_section must work differently:  Locks are required to protect code.

So this change adds locking (via enter_critical section) to wdog expiration logic for the the case if the SMP configuration.
This commit is contained in:
Gregory Nutt 2016-11-18 13:57:30 -06:00
parent cdbc66addd
commit 69e9f8638d
2 changed files with 60 additions and 9 deletions

View File

@ -263,6 +263,12 @@ irqstate_t enter_critical_section(void)
#else /* defined(CONFIG_SCHED_INSTRUMENTATION_CSECTION) */
irqstate_t enter_critical_section(void)
{
irqstate_t ret;
/* Disable interrupts */
ret = up_irq_save();
/* Check if we were called from an interrupt handler and that the task
* lists have been initialized.
*/
@ -272,14 +278,14 @@ irqstate_t enter_critical_section(void)
FAR struct tcb_s *rtcb = this_task();
DEBUGASSERT(rtcb != NULL);
/* No.. note that we have entered the critical section */
/* Yes.. Note that we have entered the critical section */
sched_note_csection(rtcb, true);
}
/* And disable interrupts */
/* Return interrupt status */
return up_irq_save();
return ret;
}
#endif
@ -386,9 +392,10 @@ void leave_critical_section(irqstate_t flags)
/* Check if there are pending tasks and that pre-emption
* is also enabled.
*
* REVISIT: There is an issue here! up_release_pending()
* REVISIT: Is there an issue here? up_release_pending()
* must be called from within a critical section but here
* we have just left the critical section.
* we have just left the critical section. At least we
* still have interrupts disabled on this CPU.
*/
if (g_pendingtasks.head != NULL &&
@ -429,7 +436,7 @@ void leave_critical_section(irqstate_t flags)
FAR struct tcb_s *rtcb = this_task();
DEBUGASSERT(rtcb != NULL);
/* Note that we have left the critical section */
/* Yes.. Note that we have left the critical section */
sched_note_csection(rtcb, false);
}

View File

@ -421,11 +421,28 @@ int wd_start(WDOG_ID wdog, int32_t delay, wdentry_t wdentry, int argc, ...)
unsigned int wd_timer(int ticks)
{
FAR struct wdog_s *wdog;
#ifdef CONFIG_SMP
irqstate_t flags;
#endif
unsigned int ret;
int decr;
#ifdef CONFIG_SMP
/* We are in an interrupt handler as, as a consequence, interrupts are
* disabled. But in the SMP case, interrupst MAY be disabled only on
* the local CPU since most architectures do not permit disabling
* interrupts on other CPUS.
*
* Hence, we must follow rules for critical sections even here in the
* SMP case.
*/
flags = enter_critical_section();
#endif
/* Check if there are any active watchdogs to process */
while (g_wdactivelist.head && ticks > 0)
while (g_wdactivelist.head != NULL && ticks > 0)
{
/* Get the watchdog at the head of the list */
@ -455,13 +472,36 @@ unsigned int wd_timer(int ticks)
/* Return the delay for the next watchdog to expire */
return g_wdactivelist.head ?
((FAR struct wdog_s *)g_wdactivelist.head)->lag : 0;
ret = g_wdactivelist.head ?
((FAR struct wdog_s *)g_wdactivelist.head)->lag : 0;
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
/* Return the delay for the next watchdog to expire */
return ret;
}
#else
void wd_timer(void)
{
#ifdef CONFIG_SMP
irqstate_t flags;
/* We are in an interrupt handler as, as a consequence, interrupts are
* disabled. But in the SMP case, interrupst MAY be disabled only on
* the local CPU since most architectures do not permit disabling
* interrupts on other CPUS.
*
* Hence, we must follow rules for critical sections even here in the
* SMP case.
*/
flags = enter_critical_section();
#endif
/* Check if there are any active watchdogs to process */
if (g_wdactivelist.head)
@ -474,5 +514,9 @@ void wd_timer(void)
wd_expiration();
}
#ifdef CONFIG_SMP
leave_critical_section(flags);
#endif
}
#endif /* CONFIG_SCHED_TICKLESS */