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

Sebastian Huber sebastian.huber at embedded-brains.de
Wed Jun 3 21:19:51 UTC 2015


----- Konstantin Belousov <kostikbel at gmail.com> schrieb:
> 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);
> ?

Ok, I will send a second version of the patch next Tuesday.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.


More information about the freebsd-hackers mailing list