Date: Wed, 3 Jun 2015 23:19:49 +0200 (CEST) From: Sebastian Huber <sebastian.huber@embedded-brains.de> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-hackers@freebsd.org, phk@phk.freebsd.dk Subject: Re: [PATCH v2] timecounters: Fix timehand generation read/write Message-ID: <2127254638.13127.1433366389045.JavaMail.zimbra@embedded-brains.de> In-Reply-To: <20150603145606.GW2499@kib.kiev.ua> References: <1433331966-27548-1-git-send-email-sebastian.huber@embedded-brains.de> <1433332368-27645-1-git-send-email-sebastian.huber@embedded-brains.de> <20150603145606.GW2499@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
----- Konstantin Belousov <kostikbel@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-volatil=
e
> > 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.
> >=20
> > For the test the timehand count was reduced to one with 10000
> > tc_windup() calls per second. The timehand memory location was adjuste=
d
> > so that the th_generation field was on its own cache line.
> > ---
> >=20
> > v2: Don't use tc_getgen() in tc_windup() since in the only writer there=
is no
> > need for a read memory barrier.
> >=20
> > sys/kern/kern_tc.c | 100 +++++++++++++++++++++++++++++++--------------=
--------
> > 1 file changed, 59 insertions(+), 41 deletions(-)
> >=20
> > 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 {
> > =09struct timeval=09=09th_microtime;
> > =09struct timespec=09=09th_nanotime;
> > =09/* Fields not to be copied in tc_windup start with th_generation. *=
/
> > -=09volatile u_int=09=09th_generation;
> > +=09u_int=09=09=09th_generation;
> > =09struct timehands=09*th_next;
> > };
> > =20
> > @@ -189,6 +189,24 @@ tc_delta(struct timehands *th)
> > =09 tc->tc_counter_mask);
> > }
> > =20
> > +static u_int
> > +tc_getgen(struct timehands *th)
> > +{
> > +=09u_int gen;
> > +
> > +=09gen =3D th->th_generation;
> > +=09rmb();
> > +=09return (gen);
> Why not
> =09return (atomic_load_acq_int(&th->th_generation);
> ?
>=20
> > +}
> > +
> > +static void
> > +tc_setgen(struct timehands *th, u_int newgen)
> > +{
> > +
> > +=09wmb();
> > +=09th->th_generation =3D newgen;
> And there,
> =09atomic_store_rel_int(&th->th_generation, newgen);
> ?
Ok, I will send a second version of the patch next Tuesday.
--=20
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=C3=A4ftliche Mitteilung im Sinne des EHUG.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2127254638.13127.1433366389045.JavaMail.zimbra>