[PATCH] timecounters: Fix timehand generation read/write
Sebastian Huber
sebastian.huber at embedded-brains.de
Wed Jun 3 11:57:29 UTC 2015
The compiler is free to re-order load/store instructions to non-volatile
variables around a load/store of a volatile variable. So the volatile
generation counter is insufficent. In addition tests on a Freescale
T4240 platform with 24 PowerPC processors showed that real memory
barriers are required. Compiler memory barriers are not enough.
For the test the timehand count was reduced to one with 10000
tc_windup() calls per second. The timehand memory location was adjusted
so that the th_generation field was on its own cache line.
Close #2356.
---
cpukit/score/src/kern_tc.c | 115 +++++++++++++--------
testsuites/sptests/Makefile.am | 1 +
testsuites/sptests/configure.ac | 1 +
testsuites/sptests/sptimecounter03/Makefile.am | 19 ++++
testsuites/sptests/sptimecounter03/init.c | 110 ++++++++++++++++++++
.../sptests/sptimecounter03/sptimecounter03.doc | 11 ++
.../sptests/sptimecounter03/sptimecounter03.scn | 2 +
7 files changed, 216 insertions(+), 43 deletions(-)
create mode 100644 testsuites/sptests/sptimecounter03/Makefile.am
create mode 100644 testsuites/sptests/sptimecounter03/init.c
create mode 100644 testsuites/sptests/sptimecounter03/sptimecounter03.doc
create mode 100644 testsuites/sptests/sptimecounter03/sptimecounter03.scn
diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
index 5479927..6693fe8 100644
--- a/cpukit/score/src/kern_tc.c
+++ b/cpukit/score/src/kern_tc.c
@@ -66,7 +66,8 @@ __FBSDID("$FreeBSD r277406 2015-01-20T03:54:30Z$");
#include <sys/vdso.h>
#endif /* __rtems__ */
#ifdef __rtems__
-#include <rtems.h>
+#include <rtems/rtems/clock.h>
+#include <rtems/score/atomic.h>
ISR_LOCK_DEFINE(static, _Timecounter_Lock, "Timecounter");
#define hz rtems_clock_get_ticks_per_second()
#define printf(...)
@@ -76,6 +77,16 @@ fls(int x)
{
return x ? sizeof(x) * 8 - __builtin_clz(x) : 0;
}
+static void
+rmb(void)
+{
+ _Atomic_Fence(ATOMIC_ORDER_ACQUIRE);
+}
+static void
+wmb(void)
+{
+ _Atomic_Fence(ATOMIC_ORDER_RELEASE);
+}
/* FIXME: https://devel.rtems.org/ticket/2348 */
#define ntp_update_second(a, b) do { (void) a; (void) b; } while (0)
#endif /* __rtems__ */
@@ -120,7 +131,7 @@ struct timehands {
struct timeval th_microtime;
struct timespec th_nanotime;
/* Fields not to be copied in tc_windup start with th_generation. */
- volatile uint32_t th_generation;
+ uint32_t th_generation;
struct timehands *th_next;
};
@@ -252,6 +263,24 @@ tc_delta(struct timehands *th)
tc->tc_counter_mask);
}
+static uint32_t
+tc_getgen(struct timehands *th)
+{
+ uint32_t gen;
+
+ gen = th->th_generation;
+ rmb();
+ return (gen);
+}
+
+static void
+tc_setgen(struct timehands *th, uint32_t newgen)
+{
+
+ wmb();
+ th->th_generation = newgen;
+}
+
/*
* Functions for reading the time. We have to loop until we are sure that
* the timehands that we operated on was not updated under our feet. See
@@ -267,10 +296,10 @@ fbclock_binuptime(struct bintime *bt)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*bt = th->th_offset;
bintime_addx(bt, th->th_scale * tc_delta(th));
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -325,9 +354,9 @@ fbclock_getbinuptime(struct bintime *bt)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*bt = th->th_offset;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -338,9 +367,9 @@ fbclock_getnanouptime(struct timespec *tsp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
bintime2timespec(&th->th_offset, tsp);
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -351,9 +380,9 @@ fbclock_getmicrouptime(struct timeval *tvp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
bintime2timeval(&th->th_offset, tvp);
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -364,9 +393,9 @@ fbclock_getbintime(struct bintime *bt)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*bt = th->th_offset;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
bintime_add(bt, &boottimebin);
}
@@ -378,9 +407,9 @@ fbclock_getnanotime(struct timespec *tsp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*tsp = th->th_nanotime;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -391,9 +420,9 @@ fbclock_getmicrotime(struct timeval *tvp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*tvp = th->th_microtime;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
#else /* !FFCLOCK */
void
@@ -404,10 +433,10 @@ binuptime(struct bintime *bt)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*bt = th->th_offset;
bintime_addx(bt, th->th_scale * tc_delta(th));
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -462,9 +491,9 @@ getbinuptime(struct bintime *bt)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*bt = th->th_offset;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -475,9 +504,9 @@ getnanouptime(struct timespec *tsp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
bintime2timespec(&th->th_offset, tsp);
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -488,9 +517,9 @@ getmicrouptime(struct timeval *tvp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
bintime2timeval(&th->th_offset, tvp);
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -501,9 +530,9 @@ getbintime(struct bintime *bt)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*bt = th->th_offset;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
bintime_add(bt, &boottimebin);
}
@@ -515,9 +544,9 @@ getnanotime(struct timespec *tsp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*tsp = th->th_nanotime;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
void
@@ -528,9 +557,9 @@ getmicrotime(struct timeval *tvp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*tvp = th->th_microtime;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
#endif /* FFCLOCK */
@@ -943,11 +972,11 @@ ffclock_read_counter(ffcounter *ffcount)
*/
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
ffth = fftimehands;
delta = tc_delta(th);
*ffcount = ffth->tick_ffcount;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
*ffcount += delta;
}
@@ -1052,9 +1081,9 @@ dtrace_getnanotime(struct timespec *tsp)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
*tsp = th->th_nanotime;
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
}
#endif /* __rtems__ */
@@ -1096,7 +1125,7 @@ sysclock_getsnapshot(struct sysclock_snap *clock_snap, int fast)
do {
th = timehands;
- gen = th->th_generation;
+ gen = tc_getgen(th);
fbi->th_scale = th->th_scale;
fbi->tick_time = th->th_offset;
#ifdef FFCLOCK
@@ -1110,7 +1139,7 @@ sysclock_getsnapshot(struct sysclock_snap *clock_snap, int fast)
#endif
if (!fast)
delta = tc_delta(th);
- } while (gen == 0 || gen != th->th_generation);
+ } while (gen == 0 || gen != tc_getgen(th));
clock_snap->delta = delta;
#ifdef FFCLOCK
@@ -1358,7 +1387,7 @@ tc_windup(void)
tho = timehands;
th = tho->th_next;
ogen = th->th_generation;
- th->th_generation = 0;
+ tc_setgen(th, 0);
bcopy(tho, th, offsetof(struct timehands, th_generation));
/*
@@ -1479,7 +1508,7 @@ tc_windup(void)
*/
if (++ogen == 0)
ogen = 1;
- th->th_generation = ogen;
+ tc_setgen(th, ogen);
/* Go live with the new struct timehands. */
#ifdef FFCLOCK
@@ -1727,13 +1756,13 @@ pps_capture(struct pps_state *pps)
KASSERT(pps != NULL, ("NULL pps pointer in pps_capture"));
th = timehands;
- pps->capgen = th->th_generation;
+ pps->capgen = tc_getgen(th);
pps->capth = th;
#ifdef FFCLOCK
pps->capffth = fftimehands;
#endif
pps->capcount = th->th_counter->tc_get_timecount(th->th_counter);
- if (pps->capgen != th->th_generation)
+ if (pps->capgen != tc_getgen(th))
pps->capgen = 0;
}
@@ -1753,7 +1782,7 @@ pps_event(struct pps_state *pps, int event)
KASSERT(pps != NULL, ("NULL pps pointer in pps_event"));
/* If the timecounter was wound up underneath us, bail out. */
- if (pps->capgen == 0 || pps->capgen != pps->capth->th_generation)
+ if (pps->capgen == 0 || pps->capgen != tc_getgen(pps->capth))
return;
/* Things would be easier with arrays. */
@@ -1803,7 +1832,7 @@ pps_event(struct pps_state *pps, int event)
bintime2timespec(&bt, &ts);
/* If the timecounter was wound up underneath us, bail out. */
- if (pps->capgen != pps->capth->th_generation)
+ if (pps->capgen != tc_getgen(pps->capth))
return;
*pcount = pps->capcount;
@@ -1921,7 +1950,7 @@ _Timecounter_Tick_simple(uint32_t delta, uint32_t offset)
*/
if (++ogen == 0)
ogen = 1;
- th->th_generation = ogen;
+ tc_setgen(th, ogen);
/* Go live with the new struct timehands. */
time_second = th->th_microtime.tv_sec;
diff --git a/testsuites/sptests/Makefile.am b/testsuites/sptests/Makefile.am
index c3fc443..948a8f0 100644
--- a/testsuites/sptests/Makefile.am
+++ b/testsuites/sptests/Makefile.am
@@ -40,6 +40,7 @@ endif
_SUBDIRS += spintrcritical23
_SUBDIRS += sptimecounter01
_SUBDIRS += sptimecounter02
+_SUBDIRS += sptimecounter03
_SUBDIRS += spatomic01
_SUBDIRS += spintrcritical22
_SUBDIRS += spsem03
diff --git a/testsuites/sptests/configure.ac b/testsuites/sptests/configure.ac
index b8287a4..a16ab3e 100644
--- a/testsuites/sptests/configure.ac
+++ b/testsuites/sptests/configure.ac
@@ -43,6 +43,7 @@ AC_CONFIG_FILES([Makefile
spintrcritical23/Makefile
sptimecounter01/Makefile
sptimecounter02/Makefile
+sptimecounter03/Makefile
spatomic01/Makefile
spglobalcon01/Makefile
spintrcritical22/Makefile
diff --git a/testsuites/sptests/sptimecounter03/Makefile.am b/testsuites/sptests/sptimecounter03/Makefile.am
new file mode 100644
index 0000000..28209fa
--- /dev/null
+++ b/testsuites/sptests/sptimecounter03/Makefile.am
@@ -0,0 +1,19 @@
+rtems_tests_PROGRAMS = sptimecounter03
+sptimecounter03_SOURCES = init.c
+
+dist_rtems_tests_DATA = sptimecounter03.scn sptimecounter03.doc
+
+include $(RTEMS_ROOT)/make/custom/@RTEMS_BSP at .cfg
+include $(top_srcdir)/../automake/compile.am
+include $(top_srcdir)/../automake/leaf.am
+
+AM_CPPFLAGS += -I$(top_srcdir)/../support/include
+
+LINK_OBJS = $(sptimecounter03_OBJECTS)
+LINK_LIBS = $(sptimecounter03_LDLIBS)
+
+sptimecounter03$(EXEEXT): $(sptimecounter03_OBJECTS) $(sptimecounter03_DEPENDENCIES)
+ @rm -f sptimecounter03$(EXEEXT)
+ $(make-exe)
+
+include $(top_srcdir)/../automake/local.am
diff --git a/testsuites/sptests/sptimecounter03/init.c b/testsuites/sptests/sptimecounter03/init.c
new file mode 100644
index 0000000..2595c7d
--- /dev/null
+++ b/testsuites/sptests/sptimecounter03/init.c
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2015 embedded brains GmbH. All rights reserved.
+ *
+ * embedded brains GmbH
+ * Dornierstr. 4
+ * 82178 Puchheim
+ * Germany
+ * <rtems at embedded-brains.de>
+ *
+ * The license and distribution terms for this file may be
+ * found in the file LICENSE in this distribution or at
+ * http://www.rtems.org/license/LICENSE.
+ */
+
+#ifdef HAVE_CONFIG_H
+ #include "config.h"
+#endif
+
+#include <rtems/test.h>
+
+#include <rtems/bsd.h>
+
+#include <test_support.h>
+
+#include "tmacros.h"
+
+const char rtems_test_name[] = "SPTIMECOUNTER 3";
+
+#define CPU_COUNT 32
+
+static rtems_test_parallel_context ctx;;
+
+static rtems_interval test_binuptime_init(
+ rtems_test_parallel_context *ctx,
+ void *arg,
+ size_t active_workers
+)
+{
+ return 10 * rtems_clock_get_ticks_per_second();
+}
+
+static void test_binuptime_body(
+ rtems_test_parallel_context *ctx,
+ void *arg,
+ size_t active_workers,
+ size_t worker_index
+)
+{
+ struct bintime start;
+ struct bintime end;
+
+ rtems_bsd_binuptime(&start);
+
+ do {
+ rtems_bsd_binuptime(&end);
+ rtems_test_assert(
+ end.sec > start.sec
+ || (end.sec == start.sec && end.frac >= start.frac)
+ );
+ start = end;
+ } while (!rtems_test_parallel_stop_job(ctx));
+}
+
+static void test_binuptime_fini(
+ rtems_test_parallel_context *ctx,
+ void *arg,
+ size_t active_workers
+)
+{
+ /* Nothing to do */
+}
+
+static const rtems_test_parallel_job jobs[] = {
+ {
+ .init = test_binuptime_init,
+ .body = test_binuptime_body,
+ .fini = test_binuptime_fini,
+ .cascade = false
+ }
+};
+
+static void Init(rtems_task_argument arg)
+{
+ TEST_BEGIN();
+
+ rtems_test_parallel(&ctx, NULL, &jobs[0], RTEMS_ARRAY_SIZE(jobs));
+
+ TEST_END();
+ rtems_test_exit(0);
+}
+
+#define CONFIGURE_MICROSECONDS_PER_TICK 1000
+
+#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
+#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
+
+#define CONFIGURE_MAXIMUM_TASKS CPU_COUNT
+#define CONFIGURE_MAXIMUM_TIMERS 1
+
+#define CONFIGURE_SMP_APPLICATION
+
+#define CONFIGURE_SMP_MAXIMUM_PROCESSORS CPU_COUNT
+
+#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
+
+#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
+
+#define CONFIGURE_INIT
+
+#include <rtems/confdefs.h>
diff --git a/testsuites/sptests/sptimecounter03/sptimecounter03.doc b/testsuites/sptests/sptimecounter03/sptimecounter03.doc
new file mode 100644
index 0000000..f978e56
--- /dev/null
+++ b/testsuites/sptests/sptimecounter03/sptimecounter03.doc
@@ -0,0 +1,11 @@
+This file describes the directives and concepts tested by this test set.
+
+test set name: sptimecounter03
+
+directives:
+
+ - rtems_bsd_binuptime
+
+concepts:
+
+ - Ensure that the binuptime is monotonic within a certain time frame.
diff --git a/testsuites/sptests/sptimecounter03/sptimecounter03.scn b/testsuites/sptests/sptimecounter03/sptimecounter03.scn
new file mode 100644
index 0000000..fb249e5
--- /dev/null
+++ b/testsuites/sptests/sptimecounter03/sptimecounter03.scn
@@ -0,0 +1,2 @@
+*** BEGIN OF TEST SPTIMECOUNTER 3 ***
+*** END OF TEST SPTIMECOUNTER 3 ***
--
1.8.4.5
More information about the devel
mailing list