[PATCH v2] timecounters: Fix timehand generation read/write
Konstantin Belousov
kostikbel at gmail.com
Wed Jun 3 14:56:19 UTC 2015
On Wed, Jun 03, 2015 at 01:52:48PM +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.
>
> 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.
> ---
>
> v2: Don't use tc_getgen() in tc_windup() since in the only writer there is no
> need for a read memory barrier.
>
> sys/kern/kern_tc.c | 100 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 59 insertions(+), 41 deletions(-)
>
> diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c
> index 9dca0e8..bb9614a 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);
Why not
return (atomic_load_acq_int(&th->th_generation);
?
> +}
> +
> +static void
> +tc_setgen(struct timehands *th, u_int newgen)
> +{
> +
> + wmb();
> + th->th_generation = newgen;
And there,
atomic_store_rel_int(&th->th_generation, newgen);
?
More information about the freebsd-hackers
mailing list