-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sys: xtimer concurrency/robustness improvement #9530
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
Changes from all commits
3873447
ca1ddeb
35b6ed4
a18f931
6f5154b
3289acc
8661205
1ee8801
3e4a86d
6eed5b9
992afd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,6 @@ extern "C" { | |
| * @{ | ||
| */ | ||
| #define XTIMER_WIDTH (16U) | ||
| #define XTIMER_OVERHEAD (6U) | ||
| /** @} */ | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| From 22f5984e32bec116e3449ebad1b29b2a18f4d93e Mon Sep 17 00:00:00 2001 | ||
| From: Michel Rottleuthner <michel.rottleuthner@haw-hamburg.de> | ||
| Date: Thu, 9 Jan 2020 11:16:38 +0100 | ||
| Subject: [PATCH] update xtimer_t member names | ||
|
|
||
| --- | ||
| app.c | 2 +- | ||
| l2.c | 2 +- | ||
| pit.c | 2 +- | ||
| 3 files changed, 3 insertions(+), 3 deletions(-) | ||
|
|
||
| diff --git a/app.c b/app.c | ||
| index 8d37bd9..da9fc3c 100644 | ||
| --- a/app.c | ||
| +++ b/app.c | ||
| @@ -380,7 +380,7 @@ int ndn_app_schedule(ndn_app_t* handle, ndn_app_sched_cb_t cb, void* context, | ||
| if (entry == NULL) return -1; | ||
|
|
||
| // initialize the timer | ||
| - entry->timer.target = entry->timer.long_target = 0; | ||
| + entry->timer.offset = entry->timer.long_offset = 0; | ||
|
|
||
| // initialize the msg struct | ||
| entry->timer_msg.type = MSG_XTIMER; | ||
| diff --git a/l2.c b/l2.c | ||
| index 77d6be4..b64b9df 100644 | ||
| --- a/l2.c | ||
| +++ b/l2.c | ||
| @@ -155,7 +155,7 @@ ndn_shared_block_t* ndn_l2_frag_receive(kernel_pid_t iface, | ||
| entry->id = id; | ||
|
|
||
| // initialize timer | ||
| - entry->timer.target = entry->timer.long_target = 0; | ||
| + entry->timer.offset = entry->timer.long_offset = 0; | ||
| entry->timer_msg.type = NDN_L2_FRAG_MSG_TYPE_TIMEOUT; | ||
| entry->timer_msg.content.ptr = (char*)(&entry->timer_msg); | ||
|
|
||
| diff --git a/pit.c b/pit.c | ||
| index 644cf08..944a07c 100644 | ||
| --- a/pit.c | ||
| +++ b/pit.c | ||
| @@ -155,7 +155,7 @@ int ndn_pit_add(kernel_pid_t face_id, int face_type, ndn_shared_block_t* si, | ||
| *pit_entry = entry; | ||
|
|
||
| /* initialize the timer */ | ||
| - entry->timer.target = entry->timer.long_target = 0; | ||
| + entry->timer.offset = entry->timer.long_offset = 0; | ||
|
|
||
| /* initialize the msg struct */ | ||
| entry->timer_msg.type = NDN_PIT_MSG_TYPE_TIMEOUT; | ||
| -- | ||
| 2.24.1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| From 6b1223df6884bcfd00f4ecc8918b790baf041021 Mon Sep 17 00:00:00 2001 | ||
| From: Michel Rottleuthner <michel.rottleuthner@haw-hamburg.de> | ||
| Date: Wed, 8 Jan 2020 19:50:48 +0100 | ||
| Subject: [PATCH] update xtimer_t member names | ||
|
|
||
| --- | ||
| porting/npl/riot/include/nimble/nimble_npl_os.h | 2 +- | ||
| 1 file changed, 1 insertion(+), 1 deletion(-) | ||
|
|
||
| diff --git a/porting/npl/riot/include/nimble/nimble_npl_os.h b/porting/npl/riot/include/nimble/nimble_npl_os.h | ||
| index 67eb74e1..61c1ea8a 100644 | ||
| --- a/porting/npl/riot/include/nimble/nimble_npl_os.h | ||
| +++ b/porting/npl/riot/include/nimble/nimble_npl_os.h | ||
| @@ -209,7 +209,7 @@ ble_npl_callout_stop(struct ble_npl_callout *co) | ||
| static inline bool | ||
| ble_npl_callout_is_active(struct ble_npl_callout *c) | ||
| { | ||
| - return (c->timer.target || c->timer.long_target); | ||
| + return (c->timer.offset || c->timer.long_offset); | ||
| } | ||
|
|
||
| static inline ble_npl_time_t | ||
| -- | ||
| 2.24.1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,8 +73,10 @@ typedef void (*xtimer_callback_t)(void*); | |
| */ | ||
| typedef struct xtimer { | ||
| struct xtimer *next; /**< reference to next timer in timer lists */ | ||
| uint32_t target; /**< lower 32bit absolute target time */ | ||
| uint32_t long_target; /**< upper 32bit absolute target time */ | ||
| uint32_t offset; /**< lower 32bit offset time */ | ||
| uint32_t long_offset; /**< upper 32bit offset time */ | ||
| uint32_t start_time; /**< lower 32bit absolute start time */ | ||
| uint32_t long_start_time; /**< upper 32bit absolute start time */ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hah! I initially started ztimer to reduce the size of xtimer_t. :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this is not optimal, but I think we can live with it for now.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yup. 😄
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MichelRottleuthner good suggestion. Might be able to consider as a next PR :) Adding long_start_time is kinda quick, after-thought to merge this PR for this release.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good, I'm totally fine with leaving that for a future PR |
||
| xtimer_callback_t callback; /**< callback function to call when timer | ||
| expires */ | ||
| void *arg; /**< argument to pass to callback function */ | ||
|
|
@@ -218,8 +220,6 @@ static inline void xtimer_periodic_wakeup(xtimer_ticks32_t *last_wakeup, uint32_ | |
| * expired. | ||
| * | ||
| * @param[in] timer timer struct to work with. | ||
| * Its xtimer_t::target and xtimer_t::long_target | ||
| * fields need to be initialized with 0 on first use | ||
MichelRottleuthner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * @param[in] offset microseconds from now | ||
| * @param[in] pid pid of the thread that will be woken up | ||
| */ | ||
|
|
@@ -232,8 +232,6 @@ static inline void xtimer_set_wakeup(xtimer_t *timer, uint32_t offset, kernel_pi | |
| * expired. | ||
| * | ||
| * @param[in] timer timer struct to work with. | ||
| * Its xtimer_t::target and xtimer_t::long_target | ||
| * fields need to be initialized with 0 on first use | ||
| * @param[in] offset microseconds from now | ||
| * @param[in] pid pid of the thread that will be woken up | ||
| */ | ||
|
|
@@ -252,8 +250,6 @@ static inline void xtimer_set_wakeup64(xtimer_t *timer, uint64_t offset, kernel_ | |
| * know *exactly* what that means. | ||
| * | ||
| * @param[in] timer the timer structure to use. | ||
| * Its xtimer_t::target and xtimer_t::long_target | ||
| * fields need to be initialized with 0 on first use | ||
| * @param[in] offset time in microseconds from now specifying that timer's | ||
| * callback's execution time | ||
| */ | ||
|
|
@@ -273,8 +269,6 @@ static inline void xtimer_set(xtimer_t *timer, uint32_t offset); | |
| * know *exactly* what that means. | ||
| * | ||
| * @param[in] timer the timer structure to use. | ||
| * Its xtimer_t::target and xtimer_t::long_target | ||
| * fields need to be initialized with 0 on first use | ||
| * @param[in] offset_us time in microseconds from now specifying that timer's | ||
| * callback's execution time | ||
| */ | ||
|
|
@@ -426,8 +420,6 @@ void xtimer_set_timeout_flag(xtimer_t *t, uint32_t timeout); | |
| * needs to point to valid memory until the message has been delivered. | ||
| * | ||
| * @param[in] timer timer struct to work with. | ||
| * Its xtimer_t::target and xtimer_t::long_target | ||
| * fields need to be initialized with 0 on first use. | ||
| * @param[in] offset microseconds from now | ||
| * @param[in] msg ptr to msg that will be sent | ||
| * @param[in] target_pid pid the message will be sent to | ||
|
|
@@ -444,8 +436,6 @@ static inline void xtimer_set_msg(xtimer_t *timer, uint32_t offset, msg_t *msg, | |
| * needs to point to valid memory until the message has been delivered. | ||
| * | ||
| * @param[in] timer timer struct to work with. | ||
| * Its xtimer_t::target and xtimer_t::long_target | ||
| * fields need to be initialized with 0 on first use. | ||
| * @param[in] offset microseconds from now | ||
| * @param[in] msg ptr to msg that will be sent | ||
| * @param[in] target_pid pid the message will be sent to | ||
|
|
@@ -487,29 +477,6 @@ static inline int xtimer_msg_receive_timeout64(msg_t *msg, uint64_t timeout); | |
| #define XTIMER_BACKOFF 30 | ||
| #endif | ||
|
|
||
| /** | ||
| * @brief xtimer overhead value, in hardware ticks | ||
| * | ||
| * This value specifies the time a timer will be late if uncorrected, e.g., | ||
| * the system-specific xtimer execution time from timer ISR to executing | ||
| * a timer's callback's first instruction. | ||
| * | ||
| * E.g., with XTIMER_OVERHEAD == 0 | ||
| * start=xtimer_now(); | ||
| * xtimer_set(&timer, X); | ||
| * (in callback:) | ||
| * overhead=xtimer_now()-start-X; | ||
| * | ||
| * xtimer automatically subtracts XTIMER_OVERHEAD from a timer's target time, | ||
| * but when the timer triggers, xtimer will spin-lock until a timer's target | ||
| * time is reached, so timers will never trigger early. | ||
| * | ||
| * This is supposed to be defined per-device in e.g., periph_conf.h. | ||
| */ | ||
| #ifndef XTIMER_OVERHEAD | ||
| #define XTIMER_OVERHEAD 20 | ||
| #endif | ||
|
|
||
| #ifndef XTIMER_ISR_BACKOFF | ||
| /** | ||
| * @brief xtimer IRQ backoff time, in hardware ticks | ||
|
|
@@ -522,29 +489,6 @@ static inline int xtimer_msg_receive_timeout64(msg_t *msg, uint64_t timeout); | |
| #define XTIMER_ISR_BACKOFF 20 | ||
| #endif | ||
|
|
||
| #ifndef XTIMER_PERIODIC_SPIN | ||
| /** | ||
| * @brief xtimer_periodic_wakeup spin cutoff | ||
| * | ||
| * If the difference between target time and now is less than this value, then | ||
| * xtimer_periodic_wakeup will use xtimer_spin instead of setting a timer. | ||
| */ | ||
| #define XTIMER_PERIODIC_SPIN (XTIMER_BACKOFF * 2) | ||
| #endif | ||
|
|
||
| #ifndef XTIMER_PERIODIC_RELATIVE | ||
| /** | ||
| * @brief xtimer_periodic_wakeup relative target cutoff | ||
| * | ||
| * If the difference between target time and now is less than this value, then | ||
| * xtimer_periodic_wakeup will set a relative target time in the future instead | ||
| * of the true target. | ||
| * | ||
| * This is done to prevent target time underflows. | ||
| */ | ||
| #define XTIMER_PERIODIC_RELATIVE (512) | ||
| #endif | ||
|
|
||
| /* | ||
| * Default xtimer configuration | ||
| */ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.