[PATCH v3] timecounters: Fix timehand generation read/write

[Konstantin Belousov]

On Tue, Jun 09, 2015 at 10:31:55AM +0200, Sebastian Huber wrote:
> 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.
This looks fine.  The only detail I changed is that I do not see a reason
to use atomics or barriers on UP machines.  See the fragment at the end
of the message for the exact diff.

> 
> 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.
You mean, that the 'th_generation for its own cache line' was done for
testing only ?

> ---
> 
> v2: Don't use tc_getgen() in tc_windup() since in the only writer there is no
> need for a read memory barrier.
> 
> v3: Use atomic load/store instead of explicit memory barriers.
> 
If you do not have any comments, I will commit this version.

diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c
index 9dca0e8..fabfe03 100644
--- a/sys/kern/kern_tc.c
+++ b/sys/kern/kern_tc.c
@@ -189,6 +190,28 @@ tc_delta(struct timehands *th)
 	    tc->tc_counter_mask);
 }
 
+static u_int
+tc_getgen(struct timehands *th)
+{
+
+#ifdef SMP
+	return (atomic_load_acq_int(&th->th_generation));
+#else
+	return (th->th_generation);
+#endif
+}
+
+static void
+tc_setgen(struct timehands *th, u_int newgen)
+{
+
+#ifdef SMP
+	atomic_store_rel_int(&th->th_generation, newgen);
+#else
+	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