Date: Mon, 24 Jun 2013 20:08:49 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Gleb Smirnoff <glebius@freebsd.org>, src-committers@freebsd.org Subject: Re: svn commit: r252032 - head/sys/amd64/include Message-ID: <20130624170849.GH91021@kib.kiev.ua> In-Reply-To: <20130623181458.J2256@besplex.bde.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
--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 <sys/param.h>
#include <sys/systm.h>
-#include <sys/counter.h>
#include <sys/kernel.h>
#include <sys/smp.h>
#include <sys/sysctl.h>
#include <vm/uma.h>
+
+#define IN_SUBR_COUNTER_C
+#include <sys/counter.h>
=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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130624170849.GH91021>