[dpdk-dev] rte_alarm: modify it to make it not to be affected by discontinuous jumps in the system time
Commit Message
Due to eal_alarm_callback() and rte_eal_alarm_set() use gettimeofday()
to get the current time, and gettimeofday() is affected by jumps.
For example, set up a rte_alarm which will be triggerd next second (
current time + 1 second) by rte_eal_alarm_set(). And the callback
function of this rte_alarm sets up another rte_alarm which will be
triggered next second (current time + 2 second).
Once we change the system time when the callback function is triggered,
it is possiblb that rte alarm functionalities work out of expectation.
Replace gettimeofday() with clock_gettime(CLOCK_MONOTONIC_RAW, &now)
could avoid this phenomenon.
Signed-off-by: Wen-Chi Yang <wolkayang@gmail.com>
---
lib/librte_eal/linuxapp/eal/eal_alarm.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
Comments
On Fri, 5 Jun 2015 10:46:36 +0800
Wen-Chi Yang <wolkayang@gmail.com> wrote:
> Due to eal_alarm_callback() and rte_eal_alarm_set() use gettimeofday()
> to get the current time, and gettimeofday() is affected by jumps.
>
> For example, set up a rte_alarm which will be triggerd next second (
> current time + 1 second) by rte_eal_alarm_set(). And the callback
> function of this rte_alarm sets up another rte_alarm which will be
> triggered next second (current time + 2 second).
> Once we change the system time when the callback function is triggered,
> it is possiblb that rte alarm functionalities work out of expectation.
>
> Replace gettimeofday() with clock_gettime(CLOCK_MONOTONIC_RAW, &now)
> could avoid this phenomenon.
>
> Signed-off-by: Wen-Chi Yang <wolkayang@gmail.com>
Agreed, this should be applied.
Does BSD version have same problem?
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Back when this was first submitted in June, I mentioned that
CLOCK_MONOTONIC_RAW was ~10x slower than CLOCK_MONOTONIC:
http://dpdk.org/ml/archives/dev/2015-June/018687.html
It's not completely free from NTP frequency adjustments, but it won't have
any discontinuities.
That's what we've been using in our tree since then...
Jay
On Tue, Oct 13, 2015 at 7:33 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:
> On Fri, 5 Jun 2015 10:46:36 +0800
> Wen-Chi Yang <wolkayang@gmail.com> wrote:
>
> > Due to eal_alarm_callback() and rte_eal_alarm_set() use gettimeofday()
> > to get the current time, and gettimeofday() is affected by jumps.
> >
> > For example, set up a rte_alarm which will be triggerd next second (
> > current time + 1 second) by rte_eal_alarm_set(). And the callback
> > function of this rte_alarm sets up another rte_alarm which will be
> > triggered next second (current time + 2 second).
> > Once we change the system time when the callback function is triggered,
> > it is possiblb that rte alarm functionalities work out of expectation.
> >
> > Replace gettimeofday() with clock_gettime(CLOCK_MONOTONIC_RAW, &now)
> > could avoid this phenomenon.
> >
> > Signed-off-by: Wen-Chi Yang <wolkayang@gmail.com>
>
> Agreed, this should be applied.
> Does BSD version have same problem?
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>
>
2015-10-13 17:33, Stephen Hemminger:
> Agreed, this should be applied.
> Does BSD version have same problem?
Implementation of rte_alarm for BSD is empty.
So I would say there is no such problem ;)
2015-10-14 07:09, Jay Rolette:
> Back when this was first submitted in June, I mentioned that
> CLOCK_MONOTONIC_RAW was ~10x slower than CLOCK_MONOTONIC:
>
> http://dpdk.org/ml/archives/dev/2015-June/018687.html
>
> It's not completely free from NTP frequency adjustments, but it won't have
> any discontinuities.
>
> That's what we've been using in our tree since then...
This patch will be applied with CLOCK_MONOTONIC_RAW.
Please send a patch to change it if relevant.
2015-10-13 17:33, Stephen Hemminger:
> On Fri, 5 Jun 2015 10:46:36 +0800
> Wen-Chi Yang <wolkayang@gmail.com> wrote:
>
> > Due to eal_alarm_callback() and rte_eal_alarm_set() use gettimeofday()
> > to get the current time, and gettimeofday() is affected by jumps.
> >
> > For example, set up a rte_alarm which will be triggerd next second (
> > current time + 1 second) by rte_eal_alarm_set(). And the callback
> > function of this rte_alarm sets up another rte_alarm which will be
> > triggered next second (current time + 2 second).
> > Once we change the system time when the callback function is triggered,
> > it is possiblb that rte alarm functionalities work out of expectation.
> >
> > Replace gettimeofday() with clock_gettime(CLOCK_MONOTONIC_RAW, &now)
> > could avoid this phenomenon.
> >
> > Signed-off-by: Wen-Chi Yang <wolkayang@gmail.com>
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Applied, thanks
@@ -99,14 +99,14 @@ static void
eal_alarm_callback(struct rte_intr_handle *hdl __rte_unused,
void *arg __rte_unused)
{
- struct timeval now;
+ struct timespec now;
struct alarm_entry *ap;
rte_spinlock_lock(&alarm_list_lk);
while ((ap = LIST_FIRST(&alarm_list)) !=NULL &&
- gettimeofday(&now, NULL) == 0 &&
+ clock_gettime(CLOCK_MONOTONIC_RAW, &now) == 0 &&
(ap->time.tv_sec < now.tv_sec || (ap->time.tv_sec == now.tv_sec &&
- ap->time.tv_usec <= now.tv_usec))){
+ (ap->time.tv_usec * NS_PER_US) <= now.tv_nsec))) {
ap->executing = 1;
ap->executing_id = pthread_self();
rte_spinlock_unlock(&alarm_list_lk);
@@ -126,11 +126,11 @@ eal_alarm_callback(struct rte_intr_handle *hdl __rte_unused,
atime.it_value.tv_sec = ap->time.tv_sec;
atime.it_value.tv_nsec = ap->time.tv_usec * NS_PER_US;
/* perform borrow for subtraction if necessary */
- if (now.tv_usec > ap->time.tv_usec)
+ if (now.tv_nsec > (ap->time.tv_usec * NS_PER_US))
atime.it_value.tv_sec--, atime.it_value.tv_nsec += US_PER_S * NS_PER_US;
atime.it_value.tv_sec -= now.tv_sec;
- atime.it_value.tv_nsec -= now.tv_usec * NS_PER_US;
+ atime.it_value.tv_nsec -= now.tv_nsec;
timerfd_settime(intr_handle.fd, 0, &atime, NULL);
}
rte_spinlock_unlock(&alarm_list_lk);
@@ -139,7 +139,7 @@ eal_alarm_callback(struct rte_intr_handle *hdl __rte_unused,
int
rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
{
- struct timeval now;
+ struct timespec now;
int ret = 0;
struct alarm_entry *ap, *new_alarm;
@@ -152,12 +152,12 @@ rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
return -ENOMEM;
/* use current time to calculate absolute time of alarm */
- gettimeofday(&now, NULL);
+ clock_gettime(CLOCK_MONOTONIC_RAW, &now);
new_alarm->cb_fn = cb_fn;
new_alarm->cb_arg = cb_arg;
- new_alarm->time.tv_usec = (now.tv_usec + us) % US_PER_S;
- new_alarm->time.tv_sec = now.tv_sec + ((now.tv_usec + us) / US_PER_S);
+ new_alarm->time.tv_usec = ((now.tv_nsec / NS_PER_US) + us) % US_PER_S;
+ new_alarm->time.tv_sec = now.tv_sec + (((now.tv_nsec / NS_PER_US) + us) / US_PER_S);
rte_spinlock_lock(&alarm_list_lk);
if (!handler_registered) {