<p>Stefan Sperling has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/10489">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fix time delta calculations in sched_clck.c<br><br>Use osmo_clock_gettime() to read the monotonic clock instead<br>of gettimeofday() which could drift backwards.<br>This requires switching the scheduler clock from struct timeval<br>to struct timespec. Expand some variables to 64 bits in order<br>to keep types used in calculations compatible.<br><br>The previous implementation unconditionally subtracted microsecond<br>values from different time measurements, causing overflow if the<br>current measurement was taken in less of a fraction of a second<br>than the past measurement. Use timespecsub() for the subtraction<br>instead which accounts for fractions of a second correctly.<br><br>Change-Id: Ic93f90685c6d6dc28dfc4ad48c998e0eac113cf8<br>Related: OS#3467<br>---<br>M src/host/trxcon/sched_clck.c<br>M src/host/trxcon/scheduler.h<br>2 files changed, 33 insertions(+), 35 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/89/10489/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/host/trxcon/sched_clck.c b/src/host/trxcon/sched_clck.c</span><br><span>index 59cffb2..0707232 100644</span><br><span>--- a/src/host/trxcon/sched_clck.c</span><br><span>+++ b/src/host/trxcon/sched_clck.c</span><br><span>@@ -27,12 +27,15 @@</span><br><span> #include <errno.h></span><br><span> #include <stdlib.h></span><br><span> #include <stdint.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <inttypes.h></span><br><span> #include <string.h></span><br><span> </span><br><span> #include <osmocom/core/talloc.h></span><br><span> #include <osmocom/core/msgb.h></span><br><span> #include <osmocom/core/bits.h></span><br><span> #include <osmocom/core/fsm.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/timer.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/timer_compat.h></span><br><span> #include <osmocom/gsm/a5.h></span><br><span> </span><br><span> #include "scheduler.h"</span><br><span>@@ -47,8 +50,9 @@</span><br><span> static void sched_clck_tick(void *data)</span><br><span> {</span><br><span> struct trx_sched *sched = (struct trx_sched *) data;</span><br><span style="color: hsl(0, 100%, 40%);">- struct timeval tv_now, *tv_clock;</span><br><span style="color: hsl(0, 100%, 40%);">- int32_t elapsed;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct timespec tv_now, *tv_clock, elapsed;</span><br><span style="color: hsl(120, 100%, 40%);">+ int64_t elapsed_us;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct timespec frame_duration = { .tv_sec = 0, .tv_nsec = FRAME_DURATION_uS * 1000 };</span><br><span> </span><br><span> /* Check if transceiver is still alive */</span><br><span> if (sched->fn_counter_lost++ == TRX_LOSS_FRAMES) {</span><br><span>@@ -59,16 +63,16 @@</span><br><span> }</span><br><span> </span><br><span> /* Get actual / previous frame time */</span><br><span style="color: hsl(0, 100%, 40%);">- gettimeofday(&tv_now, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_clock_gettime(CLOCK_MONOTONIC, &tv_now);</span><br><span> tv_clock = &sched->clock;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- elapsed = (tv_now.tv_sec - tv_clock->tv_sec) * 1000000</span><br><span style="color: hsl(0, 100%, 40%);">- + (tv_now.tv_usec - tv_clock->tv_usec);</span><br><span style="color: hsl(120, 100%, 40%);">+ timespecsub(&tv_now, tv_clock, &elapsed);</span><br><span style="color: hsl(120, 100%, 40%);">+ elapsed_us = (elapsed.tv_sec * 1000000) + (elapsed.tv_nsec / 1000);</span><br><span> </span><br><span> /* If someone played with clock, or if the process stalled */</span><br><span style="color: hsl(0, 100%, 40%);">- if (elapsed > FRAME_DURATION_uS * MAX_FN_SKEW || elapsed < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (elapsed_us > FRAME_DURATION_uS * MAX_FN_SKEW || elapsed_us < 0) {</span><br><span> LOGP(DSCH, LOGL_NOTICE, "PC clock skew: "</span><br><span style="color: hsl(0, 100%, 40%);">- "elapsed uS %d\n", elapsed);</span><br><span style="color: hsl(120, 100%, 40%);">+ "elapsed uS %" PRIu64 "\n", elapsed_us);</span><br><span> </span><br><span> sched->state = SCH_CLCK_STATE_WAIT;</span><br><span> </span><br><span>@@ -76,14 +80,11 @@</span><br><span> }</span><br><span> </span><br><span> /* Schedule next FN clock */</span><br><span style="color: hsl(0, 100%, 40%);">- while (elapsed > FRAME_DURATION_uS / 2) {</span><br><span style="color: hsl(0, 100%, 40%);">- tv_clock->tv_usec += FRAME_DURATION_uS;</span><br><span style="color: hsl(0, 100%, 40%);">- elapsed -= FRAME_DURATION_uS;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (tv_clock->tv_usec >= 1000000) {</span><br><span style="color: hsl(0, 100%, 40%);">- tv_clock->tv_sec++;</span><br><span style="color: hsl(0, 100%, 40%);">- tv_clock->tv_usec -= 1000000;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ while (elapsed_us > FRAME_DURATION_uS / 2) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct timespec next;</span><br><span style="color: hsl(120, 100%, 40%);">+ timespecadd(tv_clock, &frame_duration, &next);</span><br><span style="color: hsl(120, 100%, 40%);">+ *tv_clock = next;</span><br><span style="color: hsl(120, 100%, 40%);">+ elapsed_us -= FRAME_DURATION_uS;</span><br><span> </span><br><span> sched->fn_counter_proc = (sched->fn_counter_proc + 1)</span><br><span> % GSM_HYPERFRAME;</span><br><span>@@ -94,11 +95,11 @@</span><br><span> }</span><br><span> </span><br><span> osmo_timer_schedule(&sched->clock_timer, 0,</span><br><span style="color: hsl(0, 100%, 40%);">- FRAME_DURATION_uS - elapsed);</span><br><span style="color: hsl(120, 100%, 40%);">+ FRAME_DURATION_uS - elapsed_us);</span><br><span> }</span><br><span> </span><br><span> static void sched_clck_correct(struct trx_sched *sched,</span><br><span style="color: hsl(0, 100%, 40%);">- struct timeval *tv_now, uint32_t fn)</span><br><span style="color: hsl(120, 100%, 40%);">+ struct timespec *tv_now, uint32_t fn)</span><br><span> {</span><br><span> sched->fn_counter_proc = fn;</span><br><span> </span><br><span>@@ -107,7 +108,7 @@</span><br><span> sched->clock_cb(sched);</span><br><span> </span><br><span> /* Schedule first FN clock */</span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(&sched->clock, tv_now, sizeof(struct timeval));</span><br><span style="color: hsl(120, 100%, 40%);">+ sched->clock = *tv_now;</span><br><span> memset(&sched->clock_timer, 0, sizeof(sched->clock_timer));</span><br><span> </span><br><span> sched->clock_timer.cb = sched_clck_tick;</span><br><span>@@ -117,14 +118,14 @@</span><br><span> </span><br><span> int sched_clck_handle(struct trx_sched *sched, uint32_t fn)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct timeval tv_now, *tv_clock;</span><br><span style="color: hsl(0, 100%, 40%);">- int32_t elapsed, elapsed_fn;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct timespec tv_now, *tv_clock, elapsed;</span><br><span style="color: hsl(120, 100%, 40%);">+ int64_t elapsed_us, elapsed_fn;</span><br><span> </span><br><span> /* Reset lost counter */</span><br><span> sched->fn_counter_lost = 0;</span><br><span> </span><br><span> /* Get actual / previous frame time */</span><br><span style="color: hsl(0, 100%, 40%);">- gettimeofday(&tv_now, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_clock_gettime(CLOCK_MONOTONIC, &tv_now);</span><br><span> tv_clock = &sched->clock;</span><br><span> </span><br><span> /* If this is the first CLCK IND */</span><br><span>@@ -142,8 +143,8 @@</span><br><span> osmo_timer_del(&sched->clock_timer);</span><br><span> </span><br><span> /* Calculate elapsed time / frames since last processed fn */</span><br><span style="color: hsl(0, 100%, 40%);">- elapsed = (tv_now.tv_sec - tv_clock->tv_sec) * 1000000</span><br><span style="color: hsl(0, 100%, 40%);">- + (tv_now.tv_usec - tv_clock->tv_usec);</span><br><span style="color: hsl(120, 100%, 40%);">+ timespecsub(&tv_now, tv_clock, &elapsed);</span><br><span style="color: hsl(120, 100%, 40%);">+ elapsed_us = (elapsed.tv_sec * 1000000) + (elapsed.tv_nsec / 1000);</span><br><span> elapsed_fn = (fn + GSM_HYPERFRAME - sched->fn_counter_proc)</span><br><span> % GSM_HYPERFRAME;</span><br><span> </span><br><span>@@ -159,8 +160,8 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DSCH, LOGL_INFO, "GSM clock jitter: %d\n",</span><br><span style="color: hsl(0, 100%, 40%);">- elapsed_fn * FRAME_DURATION_uS - elapsed);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DSCH, LOGL_INFO, "GSM clock jitter: %" PRIu64 "\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ elapsed_fn * FRAME_DURATION_uS - elapsed_us);</span><br><span> </span><br><span> /* Too many frames have been processed already */</span><br><span> if (elapsed_fn < 0) {</span><br><span>@@ -168,14 +169,11 @@</span><br><span> * Set clock to the time or last FN should</span><br><span> * have been transmitted</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- tv_clock->tv_sec = tv_now.tv_sec;</span><br><span style="color: hsl(0, 100%, 40%);">- tv_clock->tv_usec = tv_now.tv_usec +</span><br><span style="color: hsl(0, 100%, 40%);">- (0 - elapsed_fn) * FRAME_DURATION_uS;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (tv_clock->tv_usec >= 1000000) {</span><br><span style="color: hsl(0, 100%, 40%);">- tv_clock->tv_sec++;</span><br><span style="color: hsl(0, 100%, 40%);">- tv_clock->tv_usec -= 1000000;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ struct timespec duration, next;</span><br><span style="color: hsl(120, 100%, 40%);">+ duration.tv_sec = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ duration.tv_nsec = (0 - elapsed_fn) * FRAME_DURATION_uS * 1000;</span><br><span style="color: hsl(120, 100%, 40%);">+ timespecadd(&tv_now, &duration, &next);</span><br><span style="color: hsl(120, 100%, 40%);">+ *tv_clock = next;</span><br><span> </span><br><span> /* Set time to the time our next FN has to be transmitted */</span><br><span> osmo_timer_schedule(&sched->clock_timer, 0,</span><br><span>@@ -195,7 +193,7 @@</span><br><span> }</span><br><span> </span><br><span> /* Schedule next FN to be transmitted */</span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(tv_clock, &tv_now, sizeof(struct timeval));</span><br><span style="color: hsl(120, 100%, 40%);">+ *tv_clock = tv_now;</span><br><span> osmo_timer_schedule(&sched->clock_timer, 0, FRAME_DURATION_uS);</span><br><span> </span><br><span> return 0;</span><br><span>diff --git a/src/host/trxcon/scheduler.h b/src/host/trxcon/scheduler.h</span><br><span>index f36c3b2..6c3a2f2 100644</span><br><span>--- a/src/host/trxcon/scheduler.h</span><br><span>+++ b/src/host/trxcon/scheduler.h</span><br><span>@@ -21,7 +21,7 @@</span><br><span> /*! \brief Clock state */</span><br><span> uint8_t state;</span><br><span> /*! \brief Local clock source */</span><br><span style="color: hsl(0, 100%, 40%);">- struct timeval clock;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct timespec clock;</span><br><span> /*! \brief Count of processed frames */</span><br><span> uint32_t fn_counter_proc;</span><br><span> /*! \brief Local frame counter advance */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/10489">change 10489</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/10489"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmocom-bb </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ic93f90685c6d6dc28dfc4ad48c998e0eac113cf8 </div>
<div style="display:none"> Gerrit-Change-Number: 10489 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Stefan Sperling <ssperling@sysmocom.de> </div>