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

sys: xtimer: fix some race conditions #4903

Merged
merged 1 commit into from
Mar 11, 2016
Merged
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
5 changes: 1 addition & 4 deletions sys/include/xtimer.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,8 @@ void xtimer_set(xtimer_t *timer, uint32_t offset);
* @note this function runs in O(n) with n being the number of active timers
*
* @param[in] timer ptr to timer structure that will be removed
*
* @return 1 on success
* @return 0 when timer was not active
*/
int xtimer_remove(xtimer_t *timer);
void xtimer_remove(xtimer_t *timer);

/**
* @brief receive a message blocking but with timeout
Expand Down
41 changes: 23 additions & 18 deletions sys/xtimer/xtimer_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ static xtimer_t *long_list_head = NULL;
static void _add_timer_to_list(xtimer_t **list_head, xtimer_t *timer);
static void _add_timer_to_long_list(xtimer_t **list_head, xtimer_t *timer);
static void _shoot(xtimer_t *timer);
static void _remove(xtimer_t *timer);
static inline void _lltimer_set(uint32_t target);
static uint32_t _time_left(uint32_t target, uint32_t reference);

Expand Down Expand Up @@ -94,7 +95,10 @@ void _xtimer_set64(xtimer_t *timer, uint32_t offset, uint32_t long_offset)
xtimer_set(timer, (uint32_t) offset);
}
else {
xtimer_remove(timer);
int state = disableIRQ();
if (_is_set(timer)) {
_remove(timer);
}

_xtimer_now64(&timer->target, &timer->long_target);
timer->target += offset;
Expand All @@ -103,7 +107,6 @@ void _xtimer_set64(xtimer_t *timer, uint32_t offset, uint32_t long_offset)
timer->long_target++;
}

int state = disableIRQ();
_add_timer_to_long_list(&long_list_head, timer);
restoreIRQ(state);
DEBUG("xtimer_set64(): added longterm timer (long_target=%" PRIu32 " target=%" PRIu32 ")\n",
Expand Down Expand Up @@ -173,13 +176,17 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target)
return 0;
}

unsigned state = disableIRQ();
if (_is_set(timer)) {
_remove(timer);
}

timer->target = target;
timer->long_target = _long_cnt;
if (target < now) {
timer->long_target++;
}

unsigned state = disableIRQ();
if ( (timer->long_target > _long_cnt) || !_this_high_period(target) ) {
DEBUG("xtimer_set_absolute(): the timer doesn't fit into the low-level timer's mask.\n");
_add_timer_to_long_list(&long_list_head, timer);
Expand Down Expand Up @@ -240,14 +247,8 @@ static int _remove_timer_from_list(xtimer_t **list_head, xtimer_t *timer)
return 0;
}

int xtimer_remove(xtimer_t *timer)
static void _remove(xtimer_t *timer)
{
if (!_is_set(timer)) {
return 0;
}

unsigned state = disableIRQ();
int res = 0;
if (timer_list_head == timer) {
uint32_t next;
timer_list_head = timer->next;
Expand All @@ -261,17 +262,21 @@ int xtimer_remove(xtimer_t *timer)
_lltimer_set(next);
}
else {
res = _remove_timer_from_list(&timer_list_head, timer) ||
_remove_timer_from_list(&overflow_list_head, timer) ||
_remove_timer_from_list(&long_list_head, timer);
if (!_remove_timer_from_list(&timer_list_head, timer)) {
if (!_remove_timer_from_list(&overflow_list_head, timer)) {
_remove_timer_from_list(&long_list_head, timer);
}
}
}
}

timer->target = 0;
timer->long_target = 0;

void xtimer_remove(xtimer_t *timer)
{
int state = disableIRQ();
if (_is_set(timer)) {
_remove(timer);
}
restoreIRQ(state);

return res;
}

static uint32_t _time_left(uint32_t target, uint32_t reference)
Expand Down