Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sched/wdog: use small lock to protect g_wdactivelist #15130

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions drivers/note/note_driver.c
Original file line number Diff line number Diff line change
Expand Up @@ -1393,10 +1393,12 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
void sched_note_wdog(uint8_t event, FAR void *handler, FAR const void *arg)
{
FAR struct note_driver_s **driver;
irqstate_t flags;
struct note_wdog_s note;
bool formatted = false;
FAR struct tcb_s *tcb = this_task();

flags = enter_critical_section_wo_note();
for (driver = g_note_drivers; *driver; driver++)
{
if (note_wdog(*driver, event, handler, arg))
Expand All @@ -1421,6 +1423,8 @@ void sched_note_wdog(uint8_t event, FAR void *handler, FAR const void *arg)

note_add(*driver, &note, sizeof(note));
}

leave_critical_section_wo_note(flags);
}
#endif

Expand Down
20 changes: 20 additions & 0 deletions include/nuttx/irq.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,19 @@ int irqchain_detach(int irq, xcpt_t isr, FAR void *arg);
****************************************************************************/

#ifdef CONFIG_IRQCOUNT

# if (defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION) && \
CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0) || \
defined(CONFIG_SCHED_INSTRUMENTATION_CSECTION)
irqstate_t enter_critical_section(void) noinstrument_function;
# else
# define enter_critical_section() enter_critical_section_wo_note()
# endif

irqstate_t enter_critical_section_wo_note(void) noinstrument_function;
#else
# define enter_critical_section() up_irq_save()
# define enter_critical_section_wo_note() up_irq_save()
#endif

/****************************************************************************
Expand Down Expand Up @@ -288,9 +298,19 @@ irqstate_t enter_critical_section(void) noinstrument_function;
****************************************************************************/

#ifdef CONFIG_IRQCOUNT

# if (defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION) && \
CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0) || \
defined(CONFIG_SCHED_INSTRUMENTATION_CSECTION)
void leave_critical_section(irqstate_t flags) noinstrument_function;
# else
# define leave_critical_section(f) leave_critical_section_wo_note(f)
# endif

void leave_critical_section_wo_note(irqstate_t flags) noinstrument_function;
#else
# define leave_critical_section(f) up_irq_restore(f)
# define leave_critical_section_wo_note(f) up_irq_restore(f)
#endif

/****************************************************************************
Expand Down
94 changes: 53 additions & 41 deletions sched/irq/irq_csection.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ volatile uint8_t g_cpu_nestcount[CONFIG_SMP_NCPUS];
****************************************************************************/

/****************************************************************************
* Name: enter_critical_section
* Name: enter_critical_section_wo_note
*
* Description:
* Take the CPU IRQ lock and disable interrupts on all CPUs. A thread-
Expand All @@ -90,7 +90,7 @@ volatile uint8_t g_cpu_nestcount[CONFIG_SMP_NCPUS];
****************************************************************************/

#ifdef CONFIG_SMP
irqstate_t enter_critical_section(void)
irqstate_t enter_critical_section_wo_note(void)
{
FAR struct tcb_s *rtcb;
irqstate_t ret;
Expand Down Expand Up @@ -246,15 +246,6 @@ irqstate_t enter_critical_section(void)

cpu_irqlock_set(cpu);
rtcb->irqcount = 1;

/* Note that we have entered the critical section */

#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0
nxsched_critmon_csection(rtcb, true, return_address(0));
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
sched_note_csection(rtcb, true);
#endif
}
}

Expand All @@ -265,7 +256,7 @@ irqstate_t enter_critical_section(void)

#else

irqstate_t enter_critical_section(void)
irqstate_t enter_critical_section_wo_note(void)
{
irqstate_t ret;

Expand All @@ -285,10 +276,28 @@ irqstate_t enter_critical_section(void)
*/

DEBUGASSERT(rtcb->irqcount >= 0 && rtcb->irqcount < INT16_MAX);
if (++rtcb->irqcount == 1)
{
/* Note that we have entered the critical section */
rtcb->irqcount++;
}

/* Return interrupt status */

return ret;
}
#endif

#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0 ||\
defined(CONFIG_SCHED_INSTRUMENTATION_CSECTION)
irqstate_t enter_critical_section(void)
{
FAR struct tcb_s *rtcb;
irqstate_t flags;
flags = enter_critical_section_wo_note();

if (!up_interrupt_context())
{
rtcb = this_task();
if (rtcb->irqcount == 1)
{
#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0
nxsched_critmon_csection(rtcb, true, return_address(0));
#endif
Expand All @@ -298,14 +307,12 @@ irqstate_t enter_critical_section(void)
}
}

/* Return interrupt status */

return ret;
return flags;
}
#endif

/****************************************************************************
* Name: leave_critical_section
* Name: leave_critical_section_wo_note
*
* Description:
* Decrement the IRQ lock count and if it decrements to zero then release
Expand All @@ -314,7 +321,7 @@ irqstate_t enter_critical_section(void)
****************************************************************************/

#ifdef CONFIG_SMP
void leave_critical_section(irqstate_t flags)
void leave_critical_section_wo_note(irqstate_t flags)
{
int cpu;

Expand Down Expand Up @@ -388,14 +395,6 @@ void leave_critical_section(irqstate_t flags)
}
else
{
/* No.. Note that we have left the critical section */

#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0
nxsched_critmon_csection(rtcb, false, return_address(0));
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
sched_note_csection(rtcb, false);
#endif
/* Decrement our count on the lock. If all CPUs have
* released, then unlock the spinlock.
*/
Expand All @@ -421,10 +420,8 @@ void leave_critical_section(irqstate_t flags)

up_irq_restore(flags);
}

#else

void leave_critical_section(irqstate_t flags)
void leave_critical_section_wo_note(irqstate_t flags)
{
/* Check if we were called from an interrupt handler and that the tasks
* lists have been initialized.
Expand All @@ -440,22 +437,37 @@ void leave_critical_section(irqstate_t flags)
*/

DEBUGASSERT(rtcb->irqcount > 0);
if (--rtcb->irqcount <= 0)
{
/* Note that we have left the critical section */
--rtcb->irqcount;
}

#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0
nxsched_critmon_csection(rtcb, false, return_address(0));
/* Restore the previous interrupt state. */

up_irq_restore(flags);
}
#endif
#ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION

#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0 ||\
defined(CONFIG_SCHED_INSTRUMENTATION_CSECTION)
void leave_critical_section(irqstate_t flags)
{
FAR struct tcb_s *rtcb;

if (!up_interrupt_context())
{
rtcb = this_task();
if (rtcb->irqcount == 1)
{
# if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0
nxsched_critmon_csection(rtcb, false, return_address(0));
# endif
# ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
sched_note_csection(rtcb, false);
#endif
# endif
}
}

/* Restore the previous interrupt state. */

up_irq_restore(flags);
leave_critical_section_wo_note(flags);
}
#endif

#endif /* CONFIG_IRQCOUNT */
12 changes: 7 additions & 5 deletions sched/wdog/wd_cancel.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
#include "sched/sched.h"
#include "wdog/wdog.h"

spinlock_t g_wdog_spinlock = SP_UNLOCKED;

/****************************************************************************
* Public Functions
****************************************************************************/
Expand All @@ -60,15 +62,10 @@

int wd_cancel(FAR struct wdog_s *wdog)
{
irqstate_t flags;
int ret;

flags = enter_critical_section();

ret = wd_cancel_irq(wdog);

leave_critical_section(flags);

return ret;
}

Expand All @@ -91,12 +88,16 @@ int wd_cancel(FAR struct wdog_s *wdog)

int wd_cancel_irq(FAR struct wdog_s *wdog)
{
irqstate_t flags;
bool head;

flags = spin_lock_irqsave(&g_wdog_spinlock);

/* Make sure that the watchdog is valid and still active. */

if (wdog == NULL || !WDOG_ISACTIVE(wdog))
{
spin_unlock_irqrestore(&g_wdog_spinlock, flags);
return -EINVAL;
}

Expand All @@ -116,6 +117,7 @@ int wd_cancel_irq(FAR struct wdog_s *wdog)
/* Mark the watchdog inactive */

wdog->func = NULL;
spin_unlock_irqrestore(&g_wdog_spinlock, flags);

if (head)
{
Expand Down
27 changes: 19 additions & 8 deletions sched/wdog/wd_start.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,11 @@ static unsigned int g_wdtimernested;
static inline_function void wd_expiration(clock_t ticks)
{
FAR struct wdog_s *wdog;
wdparm_t arg;
irqstate_t flags;
wdentry_t func;

flags = enter_critical_section();
flags = spin_lock_irqsave(&g_wdog_spinlock);

#ifdef CONFIG_SCHED_TICKLESS
/* Increment the nested watchdog timer count to handle cases where wd_start
Expand Down Expand Up @@ -147,12 +148,17 @@ static inline_function void wd_expiration(clock_t ticks)
/* Indicate that the watchdog is no longer active. */

func = wdog->func;
arg = wdog->arg;
wdog->func = NULL;

/* Execute the watchdog function */

up_setpicbase(wdog->picbase);
CALL_FUNC(func, wdog->arg);
spin_unlock_irqrestore(&g_wdog_spinlock, flags);

CALL_FUNC(func, arg);

flags = spin_lock_irqsave(&g_wdog_spinlock);
}

#ifdef CONFIG_SCHED_TICKLESS
Expand All @@ -161,7 +167,7 @@ static inline_function void wd_expiration(clock_t ticks)
g_wdtimernested--;
#endif

leave_critical_section(flags);
spin_unlock_irqrestore(&g_wdog_spinlock, flags);
}

/****************************************************************************
Expand Down Expand Up @@ -293,7 +299,7 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks,
* the critical section is established.
*/

flags = enter_critical_section();
flags = spin_lock_irqsave(&g_wdog_spinlock);
#ifdef CONFIG_SCHED_TICKLESS
/* We need to reassess timer if the watchdog list head has changed. */

Expand All @@ -314,8 +320,13 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks,
* then this will pick that new delay.
*/

spin_unlock_irqrestore(&g_wdog_spinlock, flags);
nxsched_reassess_timer();
}
else
{
spin_unlock_irqrestore(&g_wdog_spinlock, flags);
}
#else
UNUSED(reassess);

Expand All @@ -328,8 +339,8 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks,
}

wd_insert(wdog, ticks, wdentry, arg);
spin_unlock_irqrestore(&g_wdog_spinlock, flags);
#endif
leave_critical_section(flags);

sched_note_wdog(NOTE_WDOG_START, wdentry, (FAR void *)(uintptr_t)ticks);
return OK;
Expand Down Expand Up @@ -425,13 +436,13 @@ clock_t wd_timer(clock_t ticks, bool noswitches)
wd_expiration(ticks);
}

flags = enter_critical_section();
flags = spin_lock_irqsave(&g_wdog_spinlock);

/* Return the delay for the next watchdog to expire */

if (list_is_empty(&g_wdactivelist))
{
leave_critical_section(flags);
spin_unlock_irqrestore(&g_wdog_spinlock, flags);
return 0;
}

Expand All @@ -442,7 +453,7 @@ clock_t wd_timer(clock_t ticks, bool noswitches)
wdog = list_first_entry(&g_wdactivelist, struct wdog_s, node);
ret = wdog->expired - ticks;

leave_critical_section(flags);
spin_unlock_irqrestore(&g_wdog_spinlock, flags);

/* Return the delay for the next watchdog to expire */

Expand Down
Loading
Loading