[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