From owner-svn-src-head@FreeBSD.ORG Mon Jun 24 17:08:55 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 8687F671; Mon, 24 Jun 2013 17:08:55 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) by mx1.freebsd.org (Postfix) with ESMTP id D0EB212B8; Mon, 24 Jun 2013 17:08:54 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.7/8.14.7) with ESMTP id r5OH8onN040371; Mon, 24 Jun 2013 20:08:50 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.8.3 kib.kiev.ua r5OH8onN040371 Received: (from kostik@localhost) by tom.home (8.14.7/8.14.7/Submit) id r5OH8nSe040370; Mon, 24 Jun 2013 20:08:49 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 24 Jun 2013 20:08:49 +0300 From: Konstantin Belousov To: Bruce Evans Subject: Re: svn commit: r252032 - head/sys/amd64/include Message-ID: <20130624170849.GH91021@kib.kiev.ua> References: <20130621081116.E1151@besplex.bde.org> <20130621090207.F1318@besplex.bde.org> <20130621064901.GS1214@FreeBSD.org> <20130621184140.G848@besplex.bde.org> <20130621135427.GA1214@FreeBSD.org> <20130622110352.J2033@besplex.bde.org> <20130622124832.S2347@besplex.bde.org> <20130622174921.I3112@besplex.bde.org> <20130623073343.GY91021@kib.kiev.ua> <20130623181458.J2256@besplex.bde.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="D8796BilH3OdfdWD" Content-Disposition: inline In-Reply-To: <20130623181458.J2256@besplex.bde.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Gleb Smirnoff , src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Jun 2013 17:08:55 -0000 --D8796BilH3OdfdWD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jun 23, 2013 at 07:57:57PM +1000, Bruce Evans wrote: > The case that can't be fixed by rereading the counters is when fetching > code runs in between the stores. If the stores are on a another CPU > that is currently executing them, then we can keep checking that the > counters don't change for the maximum time that any pair of stores > take to complete. This time is quite long and difficult to determine > (but it can be bounded by hard-disabling interrupts in the storer, and > limited by prefetching). If the stores are on the same CPU, then we > have no good way to detect that another thread is in the middle of > them, or to switch back to that thread. So we currently prevent this > case using slow methods. We are already explicit about the fetched value potentially not reflecting any real moment value, but indeed, having the fetch result off by 2^32 is not acceptable. We need atomic 64bit loads to fix this, which is provided by the same cmpxchg8b instruction on the 8-byte aligned elements. Intel guarantees that 8-byte aligned loads and stores are atomic. The following is the prototype for the x86. The other 64bit architectures are handled exactly like amd64. For 32bit !x86 arches, i.e. arm, mips32 and powerpc32, some more investigation is needed. diff --git a/sys/amd64/include/counter.h b/sys/amd64/include/counter.h index b37a4b8..ba302a3 100644 --- a/sys/amd64/include/counter.h +++ b/sys/amd64/include/counter.h @@ -36,6 +36,28 @@ extern struct pcpu __pcpu[1]; #define counter_enter() do {} while (0) #define counter_exit() do {} while (0) =20 +#ifdef IN_SUBR_COUNTER_C +static inline uint64_t +counter_u64_fetch_inline(uint64_t *p) +{ + uint64_t r; + int i; + + r =3D 0; + for (i =3D 0; i < mp_ncpus; i++) + r +=3D counter_u64_read_one((uint64_t *)c, i); + + return (r); +} +#endif + +static inline uint64_t +counter_u64_read_one(uint64_t *p, int cpu) +{ + + return (*(uint64_t *)((char *)p + sizeof(struct pcpu) * cpu)); +} + #define counter_u64_add_protected(c, i) counter_u64_add(c, i) =20 static inline void diff --git a/sys/i386/include/counter.h b/sys/i386/include/counter.h index 3e93b36..94dbee3 100644 --- a/sys/i386/include/counter.h +++ b/sys/i386/include/counter.h @@ -67,6 +67,50 @@ counter_64_inc_8b(uint64_t *p, int64_t inc) : "memory", "cc", "eax", "edx", "ebx", "ecx"); } =20 +static inline uint64_t +counter_u64_read_one_8b(uint64_t *p, int cpu) +{ + uint32_t res_lo, res_high; + + __asm __volatile( + "movl %%eax,%%ebx\n\t" + "movl %%edx,%%ecx\n\t" + "cmpxchg8b (%%esi)" + : "=3Da" (res_lo), "=3Dd"(res_high) + : "S" ((char *)p + sizeof(struct pcpu) * cpu) + : "cc", "ebx", "ecx"); + return (res_lo + ((uint64_t)res_high << 32)); +} + +#ifdef IN_SUBR_COUNTER_C +static inline uint64_t +counter_u64_fetch_inline(uint64_t *p) +{ + uint64_t res; + int i; + + res =3D 0; + if ((cpu_feature & CPUID_CX8) =3D=3D 0) { + /* + * The machines without cmpxchg8b are not SMP. + * Disabling the preemption provides atomicity of the + * counter reading, since update is done in the + * critical section as well. + */ + critical_enter(); + for (i =3D 0; i < mp_ncpus; i++) { + res +=3D *(uint64_t *)((char *)p + + sizeof(struct pcpu) * i); + } + critical_exit(); + } else { + for (i =3D 0; i < mp_ncpus; i++) + res +=3D counter_u64_read_one_8b(p, i); + } + return (res); +} +#endif + #define counter_u64_add_protected(c, inc) do { \ if ((cpu_feature & CPUID_CX8) =3D=3D 0) { \ CRITICAL_ASSERT(curthread); \ diff --git a/sys/kern/subr_counter.c b/sys/kern/subr_counter.c index a98ed40..3222881 100644 --- a/sys/kern/subr_counter.c +++ b/sys/kern/subr_counter.c @@ -29,11 +29,13 @@ __FBSDID("$FreeBSD$"); =20 #include #include -#include #include #include #include #include + +#define IN_SUBR_COUNTER_C +#include =20 static uma_zone_t uint64_pcpu_zone; =20 @@ -49,14 +51,8 @@ counter_u64_zero(counter_u64_t c) uint64_t counter_u64_fetch(counter_u64_t c) { - uint64_t r; - int i; =20 - r =3D 0; - for (i =3D 0; i < mp_ncpus; i++) - r +=3D *(uint64_t *)((char *)c + sizeof(struct pcpu) * i); - - return (r); + return (counter_u64_fetch_inline(c)); } =20 counter_u64_t --D8796BilH3OdfdWD Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.20 (FreeBSD) iQIcBAEBAgAGBQJRyH0hAAoJEJDCuSvBvK1BNYkQAIifPenblSKxWLEydgDgipiQ 76J8WrBdt61/I0OJzFm3K/jJH1rDqQBsoQ0/fcPLjSd9Y0xAZgryVnrORi18bVDv DrtTnoQX3agR2F4X5zk2yk1gYCzFyidG2qAUEY6w3IfYnNLpIblzJPazuO57TxI0 urX5Shm6GnmJInyAcM2yXxvww6hBkXZtOIiluSBK6o2n1IwgrYLovg6xjne1TgFs XFgbOC33beVcjslyZBeG9sn6d/BTLo+3A9SXpoAiHnZA9805jKC8QeastptyG/L7 1XSFGXX9H4XhcD/1truADTvlgwGhgJdgyn4z7t2asZOpDkttS0Sw3PCkxP59Bo+f SVy0E3dgWydWiZ0Iicng+E+RVziwhUqQhxqRkhNV2H+27nhA8LoNNmZEaOTmPfWm Fvt/hz4ltHX+lO4NpcO+seTHlRpVym1AIpGzbQhtZ4Y4Q2KyQ0lqMjGcW2lkMYSc V9uMB1eH23MynS0jgaogdY0sYlkK3i3cFJjRZ1wDRV/KYPAom7TM7Cl6u52BM2pD G6FcTzDzt1WGj0Shkz8vWlnCTS5xGaHeFBJ2HF7MaMwkJXySaVnJY2RKNSv3bqCf DH3Ce4JSVsZTiS+F6KnMVTmQ00wGXfBvwuk4p3lfxUf0V1X4lQoOkx4ErOmyIUAF NiuSpgxC4iiENX7ghw91 =4JHg -----END PGP SIGNATURE----- --D8796BilH3OdfdWD--