Date: Wed, 3 Jun 2015 13:46:06 +0200 From: Sebastian Huber <sebastian.huber@embedded-brains.de> To: freebsd-hackers@freebsd.org Cc: phk@phk.freebsd.dk, Sebastian Huber <sebastian.huber@embedded-brains.de> Subject: [PATCH] timecounters: Fix timehand generation read/write Message-ID: <1433331966-27548-1-git-send-email-sebastian.huber@embedded-brains.de>
next in thread | raw e-mail | index | archive | help
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.
---
sys/kern/kern_tc.c | 102 +++++++++++++++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 42 deletions(-)
diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c
index 9dca0e8..00768cf 100644
--- a/sys/kern/kern_tc.c
+++ b/sys/kern/kern_tc.c
@@ -71,7 +71,7 @@ struct timehands {
struct timeval th_microtime;
struct timespec th_nanotime;
/* Fields not to be copied in tc_windup start with th_generation. */
- volatile u_int th_generation;
+ u_int th_generation;
struct timehands *th_next;
};
@@ -189,6 +189,24 @@ tc_delta(struct timehands *th)
tc->tc_counter_mask);
}
+static u_int
+tc_getgen(struct timehands *th)
+{
+ u_int gen;
+
+ gen = th->th_generation;
+ rmb();
+ return (gen);
+}
+
+static void
+tc_setgen(struct timehands *th, u_int 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
@@ -204,10 +222,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
@@ -262,9 +280,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
@@ -275,9 +293,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
@@ -288,9 +306,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
@@ -301,9 +319,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);
}
@@ -315,9 +333,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
@@ -328,9 +346,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
@@ -341,10 +359,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
@@ -399,9 +417,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
@@ -412,9 +430,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
@@ -425,9 +443,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
@@ -438,9 +456,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);
}
@@ -452,9 +470,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
@@ -465,9 +483,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 */
@@ -880,11 +898,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;
}
@@ -988,9 +1006,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));
}
/*
@@ -1028,7 +1046,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
@@ -1042,7 +1060,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;
clock_snap->sysclock_active = sysclock_active;
@@ -1259,8 +1277,8 @@ tc_windup(void)
*/
tho = timehands;
th = tho->th_next;
- ogen = th->th_generation;
- th->th_generation = 0;
+ ogen = tc_getgen(th);
+ tc_setgen(th, 0);
bcopy(tho, th, offsetof(struct timehands, th_generation));
/*
@@ -1377,7 +1395,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
@@ -1651,13 +1669,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;
}
@@ -1677,7 +1695,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. */
@@ -1727,7 +1745,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;
--
1.8.4.5
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1433331966-27548-1-git-send-email-sebastian.huber>