Skip to content

[kernel][src] reload timeout_tick with absolute base value #5402

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[kernel][src] reload timeout_tick with absolute base value #5402

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a101d3d
reload timeout_tick with absolute base value
thewon86 Dec 20, 2021
6342521
Merge branch 'master' into timer
thewon86 Jan 20, 2022
eb6d4e6
Merge branch 'master' into timer
thewon86 Jan 21, 2022
fb49155
Update timer.c
thewon86 Jan 21, 2022
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
125 changes: 52 additions & 73 deletions src/timer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* 2014-07-12 Bernard does not lock scheduler when invoking soft-timer
* timeout function.
* 2021-08-15 supperthomas add the comment
* 2021-12-20 THEWON reload timeout_tick with absolute base value
* 2022-01-07 Gabriel Moving __on_rt_xxxxx_hook to timer.c
*/

Expand All @@ -28,9 +29,6 @@ static rt_list_t _timer_list[RT_TIMER_SKIP_LIST_LEVEL];

#ifdef RT_USING_TIMER_SOFT

#define RT_SOFT_TIMER_IDLE 1
#define RT_SOFT_TIMER_BUSY 0

#ifndef RT_TIMER_THREAD_STACK_SIZE
#define RT_TIMER_THREAD_STACK_SIZE 512
#endif /* RT_TIMER_THREAD_STACK_SIZE */
Expand All @@ -39,8 +37,6 @@ static rt_list_t _timer_list[RT_TIMER_SKIP_LIST_LEVEL];
#define RT_TIMER_THREAD_PRIO 0
#endif /* RT_TIMER_THREAD_PRIO */

/* soft timer status */
static rt_uint8_t _soft_timer_status = RT_SOFT_TIMER_IDLE;
/* soft timer list */
static rt_list_t _soft_timer_list[RT_TIMER_SKIP_LIST_LEVEL];
static struct rt_thread _timer_thread;
Expand Down Expand Up @@ -380,28 +376,19 @@ rt_err_t rt_timer_delete(rt_timer_t timer)
RTM_EXPORT(rt_timer_delete);
#endif /* RT_USING_HEAP */

/**
* @brief This function will start the timer
*
* @param timer the timer to be started
*
* @return the operation status, RT_EOK on OK, -RT_ERROR on error
*/
rt_err_t rt_timer_start(rt_timer_t timer)
static rt_err_t _timer_start(rt_timer_t timer, rt_tick_t current_tick)
{
unsigned int row_lvl;
rt_list_t *timer_list;
register rt_base_t level;
register rt_bool_t need_schedule;
rt_list_t *row_head[RT_TIMER_SKIP_LIST_LEVEL];
unsigned int tst_nr;
static unsigned int random_nr;

/* parameter check */
RT_ASSERT(timer != RT_NULL);
RT_ASSERT(rt_object_get_type(&timer->parent) == RT_Object_Class_Timer);

need_schedule = RT_FALSE;
RT_ASSERT(timer->init_tick < RT_TICK_MAX / 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个assert可以删掉了，在pr #5533 中，我已经把这个参数有效性的判断放在了init和create函数，上来就判断有效性。
https://github.com/RT-Thread/rt-thread/pull/5533/files#diff-340e9ee0d7971fa052b504957c48a3a313f76cb85490ff00afb3e9293d09f53aR335
https://github.com/RT-Thread/rt-thread/pull/5533/files#diff-340e9ee0d7971fa052b504957c48a3a313f76cb85490ff00afb3e9293d09f53aR266
https://github.com/RT-Thread/rt-thread/pull/5533/files#diff-340e9ee0d7971fa052b504957c48a3a313f76cb85490ff00afb3e9293d09f53aL404-L409


/* stop timer firstly */
level = rt_hw_interrupt_disable();
Expand All @@ -412,7 +399,11 @@ rt_err_t rt_timer_start(rt_timer_t timer)

RT_OBJECT_HOOK_CALL(rt_object_take_hook, (&(timer->parent)));

timer->timeout_tick = rt_tick_get() + timer->init_tick;
/*
* get timeout tick,
* the max timeout tick shall not great than RT_TICK_MAX/2
*/
Copy link
Member

@mysterywolf mysterywolf Jan 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个注释应该删掉 这个是给之前那个assert的注释

timer->timeout_tick = current_tick + timer->init_tick;

#ifdef RT_USING_TIMER_SOFT
if (timer->parent.flag & RT_TIMER_FLAG_SOFT_TIMER)
Expand Down Expand Up @@ -484,26 +475,40 @@ rt_err_t rt_timer_start(rt_timer_t timer)
if (timer->parent.flag & RT_TIMER_FLAG_SOFT_TIMER)
{
/* check whether timer thread is ready */
if ((_soft_timer_status == RT_SOFT_TIMER_IDLE) &&
((_timer_thread.stat & RT_THREAD_STAT_MASK) == RT_THREAD_SUSPEND))
if ((_timer_thread.stat & RT_THREAD_STAT_MASK) == RT_THREAD_SUSPEND)
{
/* resume timer thread to check soft timer */
rt_thread_resume(&_timer_thread);
need_schedule = RT_TRUE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need_schedule 标记是防止 A 定时器回调中 sleep 等操作导致线程挂起。B 定时器启动时，意外将 timer 线程唤醒。导致 A 定时器意外唤醒

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need_schedule 这个变量的作用不是你说那样，跟你说的那个没联系，rt_timer_start 最后有两种情况，一种是需要进行任务调度切换到 timer 线程，一种是不需要。
需要任务调度的流程是： 开中断；调用rt_schedule；return 返回。
不需要任务调度的流程是： 开中断；return 返回。
之前就是把上述两种流程合并到一起，用 need_schedule 变量控制中间 rt_schedule 是否被执行的开关。这样的用法在多处有 return 返回的情况下使用比较合适，但是这里只在函数最后紧挨着的地方有两个返回点，分开写代码语义比较清晰。
你可能想说的是 _soft_timer_status 这个变量，用来限制“某些”情况下任务调度，关于这个 pr 的详细说明，请移步论坛，那里有更全面的理论分析。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

能否贴一下论坛链接？

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_soft_timer_status 变量去掉后，timer 线程被异常唤醒的问题，这个问题是咋解决的？

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的，这问题是怎么解决的？

/* enable interrupt */
rt_hw_interrupt_enable(level);

rt_schedule();

return RT_EOK;
}
}
#endif /* RT_USING_TIMER_SOFT */

/* enable interrupt */
rt_hw_interrupt_enable(level);

if (need_schedule)
{
rt_schedule();
}

return RT_EOK;
}

/**
* @brief This function will start the timer
*
* @param timer the timer to be started
*
* @return the operation status, RT_EOK on OK, -RT_ERROR on error
*/
rt_err_t rt_timer_start(rt_timer_t timer)
{
rt_tick_t current_tick;

current_tick = rt_tick_get();
return _timer_start(timer, current_tick);
}
RTM_EXPORT(rt_timer_start);

/**
Expand Down Expand Up @@ -611,9 +616,6 @@ void rt_timer_check(void)
struct rt_timer *t;
rt_tick_t current_tick;
register rt_base_t level;
rt_list_t list;

rt_list_init(&list);

RT_DEBUG_LOG(RT_DEBUG_TIMER, ("timer check enter\n"));

Expand All @@ -633,16 +635,22 @@ void rt_timer_check(void)
*/
if ((current_tick - t->timeout_tick) < RT_TICK_MAX / 2)
{
RT_OBJECT_HOOK_CALL(rt_timer_enter_hook, (t));

/* remove timer from timer list firstly */
_timer_remove(t);
if (!(t->parent.flag & RT_TIMER_FLAG_PERIODIC))

if ((t->parent.flag & RT_TIMER_FLAG_PERIODIC) &&
(t->parent.flag & RT_TIMER_FLAG_ACTIVATED))
{
/* start it */
_timer_start(t, t->timeout_tick);
}
else
{
t->parent.flag &= ~RT_TIMER_FLAG_ACTIVATED;
}
/* add timer to temporary list */
rt_list_insert_after(&list, &(t->row[RT_TIMER_SKIP_LIST_LEVEL - 1]));

RT_OBJECT_HOOK_CALL(rt_timer_enter_hook, (t));

/* call timeout function */
t->timeout_func(t->parameter);

Expand All @@ -651,20 +659,6 @@ void rt_timer_check(void)

RT_OBJECT_HOOK_CALL(rt_timer_exit_hook, (t));
RT_DEBUG_LOG(RT_DEBUG_TIMER, ("current tick: %d\n", current_tick));

/* Check whether the timer object is detached or started again */
if (rt_list_isempty(&list))
{
continue;
}
rt_list_remove(&(t->row[RT_TIMER_SKIP_LIST_LEVEL - 1]));
if ((t->parent.flag & RT_TIMER_FLAG_PERIODIC) &&
(t->parent.flag & RT_TIMER_FLAG_ACTIVATED))
{
/* start it */
t->parent.flag &= ~RT_TIMER_FLAG_ACTIVATED;
rt_timer_start(t);
}
}
else break;
}
Expand Down Expand Up @@ -695,12 +689,11 @@ void rt_soft_timer_check(void)
rt_tick_t current_tick;
struct rt_timer *t;
register rt_base_t level;
rt_list_t list;

rt_list_init(&list);

RT_DEBUG_LOG(RT_DEBUG_TIMER, ("software timer check enter\n"));

current_tick = rt_tick_get();

/* disable interrupt */
level = rt_hw_interrupt_disable();

Expand All @@ -709,29 +702,30 @@ void rt_soft_timer_check(void)
t = rt_list_entry(_soft_timer_list[RT_TIMER_SKIP_LIST_LEVEL - 1].next,
struct rt_timer, row[RT_TIMER_SKIP_LIST_LEVEL - 1]);

current_tick = rt_tick_get();

/*
* It supposes that the new tick shall less than the half duration of
* tick max.
*/
if ((current_tick - t->timeout_tick) < RT_TICK_MAX / 2)
{
RT_OBJECT_HOOK_CALL(rt_timer_enter_hook, (t));

/* remove timer from timer list firstly */
_timer_remove(t);
if (!(t->parent.flag & RT_TIMER_FLAG_PERIODIC))
if ((t->parent.flag & RT_TIMER_FLAG_PERIODIC) &&
(t->parent.flag & RT_TIMER_FLAG_ACTIVATED))
{
/* start it */
_timer_start(t, t->timeout_tick);
}
else
{
t->parent.flag &= ~RT_TIMER_FLAG_ACTIVATED;
}
/* add timer to temporary list */
rt_list_insert_after(&list, &(t->row[RT_TIMER_SKIP_LIST_LEVEL - 1]));

_soft_timer_status = RT_SOFT_TIMER_BUSY;
/* enable interrupt */
rt_hw_interrupt_enable(level);

RT_OBJECT_HOOK_CALL(rt_timer_enter_hook, (t));

/* call timeout function */
t->timeout_func(t->parameter);

Expand All @@ -740,21 +734,6 @@ void rt_soft_timer_check(void)

/* disable interrupt */
level = rt_hw_interrupt_disable();

_soft_timer_status = RT_SOFT_TIMER_IDLE;
/* Check whether the timer object is detached or started again */
if (rt_list_isempty(&list))
{
continue;
}
rt_list_remove(&(t->row[RT_TIMER_SKIP_LIST_LEVEL - 1]));
if ((t->parent.flag & RT_TIMER_FLAG_PERIODIC) &&
(t->parent.flag & RT_TIMER_FLAG_ACTIVATED))
{
/* start it */
t->parent.flag &= ~RT_TIMER_FLAG_ACTIVATED;
rt_timer_start(t);
}
}
else break; /* not check anymore */
}
Expand Down