xiaoxiang781216 commented on code in PR #15129:
URL: https://github.com/apache/nuttx/pull/15129#discussion_r1912449495
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -565,13 +580,15 @@ void nxsched_reassess_timer(void)
/* Convert this to the elapsed time and update clock tickbase */
- elapsed = ticks - g_timer_tick;
- g_timer_tick = ticks;
+ elapsed = ticks - get_g_time_tick();
+ set_g_time_tick(ticks);
Review Comment:
let's use atomic_xch to make get/set at the same time.
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -439,22 +462,18 @@ void nxsched_alarm_tick_expiration(clock_t ticks)
{
clock_t elapsed;
clock_t nexttime;
- irqstate_t flags;
/* Save the time that the alarm occurred */
- flags = enter_critical_section();
- elapsed = ticks - g_timer_tick;
- g_timer_tick = ticks;
- leave_critical_section(flags);
+ elapsed = ticks - get_g_time_tick();
Review Comment:
let's use atomic_xch to make get/set at the same time.
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -106,6 +106,24 @@ static unsigned int g_timer_interval;
* Private Functions
****************************************************************************/
+static inline_function void set_g_time_tick(clock_t tick)
+{
+#ifdef CONFIG_SYSTEM_TIME64
+ atomic64_set((FAR atomic64_t *)&g_timer_tick, tick);
+#else
+ atomic_set((FAR atomic_t *)&g_timer_tick, tick);
+#endif
+}
+
+static inline_function clock_t get_g_time_tick(void)
Review Comment:
```suggestion
static inline_function clock_t get_time_tick(void)
```
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -590,12 +607,10 @@ void nxsched_reassess_timer(void)
clock_t nxsched_get_next_expired(void)
{
- irqstate_t flags;
sclock_t ret;
- flags = enter_critical_section();
- ret = g_timer_tick + g_timer_interval - clock_systime_ticks();
- leave_critical_section(flags);
+ ret = get_g_time_tick() + atomic_read((FAR atomic_t *)&g_timer_interval) -
Review Comment:
change g_timer_interval to atomic_t and remove the cast
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -388,6 +391,8 @@ static clock_t nxsched_timer_start(clock_t ticks, clock_t
interval)
}
#endif
+ flags = enter_critical_section();
Review Comment:
but up_alarm_tick_start/up_timer_tick_start already hold spinlock internally
after the small lock patch apply to arch
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -106,6 +106,24 @@ static unsigned int g_timer_interval;
* Private Functions
****************************************************************************/
+static inline_function void set_g_time_tick(clock_t tick)
Review Comment:
let's return the old value:
```suggestion
static inline_function clock_t set_timer_tick(clock_t tick)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
