Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 8 Jul 2015 18:42:09 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r285286 - head/sys/kern
Message-ID:  <201507081842.t68Ig9EO074114@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Wed Jul  8 18:42:08 2015
New Revision: 285286
URL: https://svnweb.freebsd.org/changeset/base/285286

Log:
  Reimplement the ordering requirements for the timehands updates, and
  for timehands consumers, by using fences.
  
  Ensure that the timehands->th_generation reset to zero is visible
  before the data update is visible [*].  tc_setget() allowed data update
  writes to become visible before generation (but not on TSO
  architectures).
  
  Remove tc_setgen(), tc_getgen() helpers, use atomics inline [**].
  
  Noted by:	alc [*]
  Requested by:	bde [**]
  Reviewed by:	alc, bde
  Sponsored by:	The FreeBSD Foundation
  MFC after:	3 weeks

Modified:
  head/sys/kern/kern_tc.c

Modified: head/sys/kern/kern_tc.c
==============================================================================
--- head/sys/kern/kern_tc.c	Wed Jul  8 18:37:08 2015	(r285285)
+++ head/sys/kern/kern_tc.c	Wed Jul  8 18:42:08 2015	(r285286)
@@ -189,33 +189,6 @@ tc_delta(struct timehands *th)
 	    tc->tc_counter_mask);
 }
 
-static inline u_int
-tc_getgen(struct timehands *th)
-{
-
-#ifdef SMP
-	return (atomic_load_acq_int(&th->th_generation));
-#else
-	u_int gen;
-
-	gen = th->th_generation;
-	__compiler_membar();
-	return (gen);
-#endif
-}
-
-static inline void
-tc_setgen(struct timehands *th, u_int newgen)
-{
-
-#ifdef SMP
-	atomic_store_rel_int(&th->th_generation, newgen);
-#else
-	__compiler_membar();
-	th->th_generation = newgen;
-#endif
-}
-
 /*
  * 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
@@ -231,10 +204,11 @@ fbclock_binuptime(struct bintime *bt)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*bt = th->th_offset;
 		bintime_addx(bt, th->th_scale * tc_delta(th));
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -289,9 +263,10 @@ fbclock_getbinuptime(struct bintime *bt)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*bt = th->th_offset;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -302,9 +277,10 @@ fbclock_getnanouptime(struct timespec *t
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		bintime2timespec(&th->th_offset, tsp);
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -315,9 +291,10 @@ fbclock_getmicrouptime(struct timeval *t
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		bintime2timeval(&th->th_offset, tvp);
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -328,9 +305,10 @@ fbclock_getbintime(struct bintime *bt)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*bt = th->th_offset;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 	bintime_add(bt, &boottimebin);
 }
 
@@ -342,9 +320,10 @@ fbclock_getnanotime(struct timespec *tsp
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*tsp = th->th_nanotime;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -355,9 +334,10 @@ fbclock_getmicrotime(struct timeval *tvp
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*tvp = th->th_microtime;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 #else /* !FFCLOCK */
 void
@@ -368,10 +348,11 @@ binuptime(struct bintime *bt)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*bt = th->th_offset;
 		bintime_addx(bt, th->th_scale * tc_delta(th));
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -426,9 +407,10 @@ getbinuptime(struct bintime *bt)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*bt = th->th_offset;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -439,9 +421,10 @@ getnanouptime(struct timespec *tsp)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		bintime2timespec(&th->th_offset, tsp);
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -452,9 +435,10 @@ getmicrouptime(struct timeval *tvp)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		bintime2timeval(&th->th_offset, tvp);
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -465,9 +449,10 @@ getbintime(struct bintime *bt)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*bt = th->th_offset;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 	bintime_add(bt, &boottimebin);
 }
 
@@ -479,9 +464,10 @@ getnanotime(struct timespec *tsp)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*tsp = th->th_nanotime;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 void
@@ -492,9 +478,10 @@ getmicrotime(struct timeval *tvp)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*tvp = th->th_microtime;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 #endif /* FFCLOCK */
 
@@ -907,11 +894,12 @@ ffclock_read_counter(ffcounter *ffcount)
 	 */
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		ffth = fftimehands;
 		delta = tc_delta(th);
 		*ffcount = ffth->tick_ffcount;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 
 	*ffcount += delta;
 }
@@ -1015,9 +1003,10 @@ dtrace_getnanotime(struct timespec *tsp)
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		*tsp = th->th_nanotime;
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 }
 
 /*
@@ -1055,7 +1044,7 @@ sysclock_getsnapshot(struct sysclock_sna
 
 	do {
 		th = timehands;
-		gen = tc_getgen(th);
+		gen = atomic_load_acq_int(&th->th_generation);
 		fbi->th_scale = th->th_scale;
 		fbi->tick_time = th->th_offset;
 #ifdef FFCLOCK
@@ -1069,7 +1058,8 @@ sysclock_getsnapshot(struct sysclock_sna
 #endif
 		if (!fast)
 			delta = tc_delta(th);
-	} while (gen == 0 || gen != tc_getgen(th));
+		atomic_thread_fence_acq();
+	} while (gen == 0 || gen != th->th_generation);
 
 	clock_snap->delta = delta;
 	clock_snap->sysclock_active = sysclock_active;
@@ -1280,14 +1270,19 @@ tc_windup(void)
 	time_t t;
 
 	/*
-	 * Make the next timehands a copy of the current one, but do not
-	 * overwrite the generation or next pointer.  While we update
-	 * the contents, the generation must be zero.
+	 * Make the next timehands a copy of the current one, but do
+	 * not overwrite the generation or next pointer.  While we
+	 * update the contents, the generation must be zero.  We need
+	 * to ensure that the zero generation is visible before the
+	 * data updates become visible, which requires release fence.
+	 * For similar reasons, re-reading of the generation after the
+	 * data is read should use acquire fence.
 	 */
 	tho = timehands;
 	th = tho->th_next;
 	ogen = th->th_generation;
-	tc_setgen(th, 0);
+	th->th_generation = 0;
+	atomic_thread_fence_rel();
 	bcopy(tho, th, offsetof(struct timehands, th_generation));
 
 	/*
@@ -1404,7 +1399,7 @@ tc_windup(void)
 	 */
 	if (++ogen == 0)
 		ogen = 1;
-	tc_setgen(th, ogen);
+	atomic_store_rel_int(&th->th_generation, ogen);
 
 	/* Go live with the new struct timehands. */
 #ifdef FFCLOCK
@@ -1678,13 +1673,14 @@ pps_capture(struct pps_state *pps)
 
 	KASSERT(pps != NULL, ("NULL pps pointer in pps_capture"));
 	th = timehands;
-	pps->capgen = tc_getgen(th);
+	pps->capgen = atomic_load_acq_int(&th->th_generation);
 	pps->capth = th;
 #ifdef FFCLOCK
 	pps->capffth = fftimehands;
 #endif
 	pps->capcount = th->th_counter->tc_get_timecount(th->th_counter);
-	if (pps->capgen != tc_getgen(th))
+	atomic_thread_fence_acq();
+	if (pps->capgen != th->th_generation)
 		pps->capgen = 0;
 }
 
@@ -1704,7 +1700,8 @@ pps_event(struct pps_state *pps, int eve
 
 	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 != tc_getgen(pps->capth))
+	if (pps->capgen == 0 || pps->capgen !=
+	    atomic_load_acq_int(&pps->capth->th_generation))
 		return;
 
 	/* Things would be easier with arrays. */
@@ -1754,7 +1751,8 @@ pps_event(struct pps_state *pps, int eve
 	bintime2timespec(&bt, &ts);
 
 	/* If the timecounter was wound up underneath us, bail out. */
-	if (pps->capgen != tc_getgen(pps->capth))
+	atomic_thread_fence_acq();
+	if (pps->capgen != pps->capth->th_generation)
 		return;
 
 	*pcount = pps->capcount;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201507081842.t68Ig9EO074114>