mirror of
https://github.com/apache/nuttx.git
synced 2026-05-20 12:33:27 +08:00
sched/wdog: Revert the spin-lock to big-kernel-lock (enter_critical_section).
If the wdog use the fine-grained spin-lock, and allow the callback execution without the lock held, there will be incorrect interleaving.
E.g. The first `nxsem_timeout` callback function caused the second semaphore wait to fail.
Core 0 [nxsem_clockwait] | Core 1
enter_critical_section() | ...
wd_start(nxsem_timeout) | ...
nxsem_wait(sem) | wd_expiration() --> nxsem_timeout
wd_cancel(&rtcb->waitdog) | try enter_critical_section()
leave_critical_section() | Failed retry...
....nxsem_clockwait | Failed retry...
enter_critical_section() | Failed retry...
wd_start(nxsem_timeout) | Failed retry...
nxsem_wait(sem) | Core 1 enter the critical section
| nxsem_wait_irq(wtcb, ETIMEDOUT) -> incorrectly wake-up the rtcb.
Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit is contained in:
committed by
Xiang Xiao
parent
164b075d3f
commit
43e0c50aa7
@@ -63,13 +63,13 @@ int wd_cancel(FAR struct wdog_s *wdog)
|
||||
irqstate_t flags;
|
||||
bool head;
|
||||
|
||||
flags = spin_lock_irqsave(&g_wdspinlock);
|
||||
flags = enter_critical_section();
|
||||
|
||||
/* Make sure that the watchdog is valid and still active. */
|
||||
|
||||
if (wdog == NULL || !WDOG_ISACTIVE(wdog))
|
||||
{
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
leave_critical_section(flags);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
@@ -90,8 +90,6 @@ int wd_cancel(FAR struct wdog_s *wdog)
|
||||
|
||||
wdog->func = NULL;
|
||||
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
|
||||
if (head)
|
||||
{
|
||||
/* If the watchdog is at the head of the timer queue, then
|
||||
@@ -102,5 +100,6 @@ int wd_cancel(FAR struct wdog_s *wdog)
|
||||
nxsched_reassess_timer();
|
||||
}
|
||||
|
||||
leave_critical_section(flags);
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -34,8 +34,6 @@
|
||||
* Public Data
|
||||
****************************************************************************/
|
||||
|
||||
spinlock_t g_wdspinlock = SP_UNLOCKED;
|
||||
|
||||
/* The g_wdactivelist data structure is a singly linked list ordered by
|
||||
* watchdog expiration time. When watchdog timers expire,the functions on
|
||||
* this linked list are removed and the function is called.
|
||||
|
||||
+7
-16
@@ -111,7 +111,7 @@ static inline_function void wd_expiration(clock_t ticks)
|
||||
wdentry_t func;
|
||||
wdparm_t arg;
|
||||
|
||||
flags = spin_lock_irqsave(&g_wdspinlock);
|
||||
flags = enter_critical_section();
|
||||
|
||||
/* Process the watchdog at the head of the list as well as any
|
||||
* other watchdogs that became ready to run at this time
|
||||
@@ -143,14 +143,10 @@ static inline_function void wd_expiration(clock_t ticks)
|
||||
/* Execute the watchdog function */
|
||||
|
||||
up_setpicbase(wdog->picbase);
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
|
||||
CALL_FUNC(func, arg);
|
||||
|
||||
flags = spin_lock_irqsave(&g_wdspinlock);
|
||||
}
|
||||
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
leave_critical_section(flags);
|
||||
}
|
||||
|
||||
/****************************************************************************
|
||||
@@ -272,7 +268,7 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks,
|
||||
* the critical section is established.
|
||||
*/
|
||||
|
||||
flags = spin_lock_irqsave(&g_wdspinlock);
|
||||
flags = enter_critical_section();
|
||||
#ifdef CONFIG_SCHED_TICKLESS
|
||||
/* We need to reassess timer if the watchdog list head has changed. */
|
||||
|
||||
@@ -291,13 +287,8 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks,
|
||||
* then this will pick that new delay.
|
||||
*/
|
||||
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
nxsched_reassess_timer();
|
||||
}
|
||||
else
|
||||
{
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
}
|
||||
#else
|
||||
UNUSED(reassess);
|
||||
|
||||
@@ -309,8 +300,8 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks,
|
||||
}
|
||||
|
||||
wd_insert(wdog, ticks, wdentry, arg);
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
#endif
|
||||
leave_critical_section(flags);
|
||||
|
||||
sched_note_wdog(NOTE_WDOG_START, wdentry, (FAR void *)(uintptr_t)ticks);
|
||||
return OK;
|
||||
@@ -356,13 +347,13 @@ clock_t wd_timer(clock_t ticks, bool noswitches)
|
||||
wd_expiration(ticks);
|
||||
}
|
||||
|
||||
flags = spin_lock_irqsave(&g_wdspinlock);
|
||||
flags = enter_critical_section();
|
||||
|
||||
/* Return the delay for the next watchdog to expire */
|
||||
|
||||
if (list_is_empty(&g_wdactivelist))
|
||||
{
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
leave_critical_section(flags);
|
||||
return CLOCK_MAX;
|
||||
}
|
||||
|
||||
@@ -373,7 +364,7 @@ clock_t wd_timer(clock_t ticks, bool noswitches)
|
||||
wdog = list_first_entry(&g_wdactivelist, struct wdog_s, node);
|
||||
ret = wdog->expired - ticks;
|
||||
|
||||
spin_unlock_irqrestore(&g_wdspinlock, flags);
|
||||
leave_critical_section(flags);
|
||||
|
||||
/* Return the delay for the next watchdog to expire */
|
||||
|
||||
|
||||
@@ -36,7 +36,6 @@
|
||||
#include <nuttx/clock.h>
|
||||
#include <nuttx/queue.h>
|
||||
#include <nuttx/wdog.h>
|
||||
#include <nuttx/spinlock_type.h>
|
||||
|
||||
/****************************************************************************
|
||||
* Pre-processor Definitions
|
||||
@@ -60,7 +59,6 @@ extern "C"
|
||||
*/
|
||||
|
||||
extern struct list_node g_wdactivelist;
|
||||
extern spinlock_t g_wdspinlock;
|
||||
|
||||
/****************************************************************************
|
||||
* Public Function Prototypes
|
||||
|
||||
Reference in New Issue
Block a user