This is an automated email from the ASF dual-hosted git repository. jiuzhudong pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 493e2075a187074ebe95867915e6364e451ff4eb Author: ouyangxiangzhen <[email protected]> AuthorDate: Thu Jan 15 15:20:47 2026 +0800 sched/hrtimer: Fix functional corrctness issue in re-enqueuing the timer When a hrtimer is removed and then re-enqueued, if the removed hrtimer is the head node and the re-enqueued node is not the head node, the hardware timer still needs to be reset. This patch fixes this issue and simplifies the enqueuing. Signed-off-by: ouyangxiangzhen <[email protected]> --- sched/hrtimer/hrtimer.h | 54 +++++++++++++++++------------------------- sched/hrtimer/hrtimer_cancel.c | 4 +--- sched/hrtimer/hrtimer_start.c | 9 +++---- 3 files changed, 28 insertions(+), 39 deletions(-) diff --git a/sched/hrtimer/hrtimer.h b/sched/hrtimer/hrtimer.h index fcce87c15f7..f20801cb4d4 100644 --- a/sched/hrtimer/hrtimer.h +++ b/sched/hrtimer/hrtimer.h @@ -206,9 +206,17 @@ RB_PROTOTYPE(hrtimer_tree_s, hrtimer_s, node, hrtimer_compare); * * Description: * Remove a timer from the queue and mark it as dequeued. + * + * Returned Value: + * true if the timer is the head of the queue, otherwise false. + * + * Assumption: + * The caller must hold the queue lock. + * The caller must ensure that the timer is in the queue. + * ****************************************************************************/ -static inline_function void hrtimer_remove(FAR hrtimer_t *hrtimer) +static inline_function bool hrtimer_remove(FAR hrtimer_t *hrtimer) { #ifdef CONFIG_HRTIMER_TREE RB_REMOVE(hrtimer_tree_s, &g_hrtimer_tree, hrtimer); @@ -219,20 +227,29 @@ static inline_function void hrtimer_remove(FAR hrtimer_t *hrtimer) /* Explicitly mark the timer as dequeued. */ hrtimer->func = NULL; + return RB_LEFT(hrtimer, node) == NULL; } /**************************************************************************** * Name: hrtimer_insert * * Description: - * Insert a timer into the timer container according to its - * expiration time. + * Insert a timer into the hrtimer queue according to its expiration time. + * + * Returned Value: + * true if the timer is added to the head of the queue, otherwise false. + * + * Assumption: + * The caller must hold the queue lock. + * The caller must ensure that the timer is in the queue. + * ****************************************************************************/ -static inline_function void hrtimer_insert(FAR hrtimer_t *hrtimer) +static inline_function bool hrtimer_insert(FAR hrtimer_t *hrtimer) { #ifdef CONFIG_HRTIMER_TREE RB_INSERT(hrtimer_tree_s, &g_hrtimer_tree, hrtimer); + return RB_LEFT(hrtimer, node) == NULL; #else FAR hrtimer_t *curr; uint64_t expired = hrtimer->expired; @@ -248,6 +265,7 @@ static inline_function void hrtimer_insert(FAR hrtimer_t *hrtimer) } list_add_before(&curr->node, &hrtimer->node); + return list_is_head(&g_hrtimer_list, &hrtimer->node); #endif } @@ -270,34 +288,6 @@ static inline_function FAR hrtimer_t *hrtimer_get_first(void) #endif } -/**************************************************************************** - * Name: hrtimer_is_first - * - * Description: - * Test whether the given high-resolution timer is the earliest - * expiring timer in the container. - * - * In a container ordered by expiration time, the earliest timer - * is represented by the left-most node. Therefore, a timer is the - * earliest one if it has no left child. - * - * Input Parameters: - * hrtimer - Pointer to the high-resolution timer to be tested. - * - * Returned Value: - * true - The timer is the earliest expiring pending timer. - * false - The timer is not the earliest timer. - ****************************************************************************/ - -static inline_function bool hrtimer_is_first(FAR hrtimer_t *hrtimer) -{ -#ifdef CONFIG_HRTIMER_TREE - return RB_LEFT(hrtimer, node) == NULL; -#else - return hrtimer == list_first_entry(&g_hrtimer_list, hrtimer_t, node); -#endif -} - /**************************************************************************** * Name: hrtimer_read_64/32 * diff --git a/sched/hrtimer/hrtimer_cancel.c b/sched/hrtimer/hrtimer_cancel.c index 62833e5ccf8..930221fb2f7 100644 --- a/sched/hrtimer/hrtimer_cancel.c +++ b/sched/hrtimer/hrtimer_cancel.c @@ -93,11 +93,9 @@ int hrtimer_cancel(FAR hrtimer_t *hrtimer) if (hrtimer_is_pending(hrtimer)) { - hrtimer_remove(hrtimer); - /* Update the hardware timer if the queue head changed. */ - if (hrtimer_is_first(hrtimer)) + if (hrtimer_remove(hrtimer)) { first = hrtimer_get_first(); if (first != NULL) diff --git a/sched/hrtimer/hrtimer_start.c b/sched/hrtimer/hrtimer_start.c index b5d9f1b21d1..49553583201 100644 --- a/sched/hrtimer/hrtimer_start.c +++ b/sched/hrtimer/hrtimer_start.c @@ -63,7 +63,8 @@ int hrtimer_start_absolute(FAR hrtimer_t *hrtimer, hrtimer_entry_t func, uint64_t expired) { irqstate_t flags; - int ret = OK; + bool reprogram = false; + int ret = OK; DEBUGASSERT(hrtimer != NULL); @@ -77,7 +78,7 @@ int hrtimer_start_absolute(FAR hrtimer_t *hrtimer, hrtimer_entry_t func, if (hrtimer_is_pending(hrtimer)) { - hrtimer_remove(hrtimer); + reprogram = hrtimer_remove(hrtimer); } hrtimer->func = func; @@ -85,11 +86,11 @@ int hrtimer_start_absolute(FAR hrtimer_t *hrtimer, hrtimer_entry_t func, /* Insert the timer into the hrtimer queue. */ - hrtimer_insert(hrtimer); + reprogram |= hrtimer_insert(hrtimer); /* If the inserted timer is now the earliest, start hardware timer */ - if (hrtimer_is_first(hrtimer)) + if (reprogram) { hrtimer_reprogram(hrtimer->expired); }
