Date: Tue, 18 Sep 2012 01:13:08 +0100 From: Attilio Rao <attilio@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: Davide Italiano <davide@freebsd.org>, src-committers@freebsd.org, Jeff Roberson <jeff@freebsd.org>, Dimitry Andric <dim@freebsd.org>, svn-src-projects@freebsd.org, Konstantin Belousov <kostikbel@gmail.com> Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern Message-ID: <CAJ-FndCRg0UCThFkatp=tw7rUWWCvhsApLE=iztLpxpGBC1F9w@mail.gmail.com> In-Reply-To: <CAJ-FndD5EO12xsWOAe6u0EvX00q33wxO4OivnGjzj0=T2Oe8uA@mail.gmail.com> References: <201207301350.q6UDobCI099069@svn.freebsd.org> <CAJ-FndBj8tpC_BJXs_RH8sG2TBG8yA=Lxu3-GTVT9Ap_zOCuVQ@mail.gmail.com> <CAJ-FndDnO7wjnWPV0tTu%2BUGHjsxa3YDarMxmyei3ZmjLAFvRkQ@mail.gmail.com> <201207301732.33474.jhb@freebsd.org> <CAJ-FndD5EO12xsWOAe6u0EvX00q33wxO4OivnGjzj0=T2Oe8uA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Aug 2, 2012 at 9:56 PM, Attilio Rao <attilio@freebsd.org> wrote:
> On 7/30/12, John Baldwin <jhb@freebsd.org> wrote:
[ trimm ]
>> --- //depot/projects/smpng/sys/kern/kern_rmlock.c 2012-03-25
>> 18:45:29.000000000 0000
>> +++ //depot/user/jhb/lock/kern/kern_rmlock.c 2012-06-18 21:20:58.000000000
>> 0000
>> @@ -70,6 +70,9 @@
>> }
>>
>> static void assert_rm(const struct lock_object *lock, int what);
>> +#ifdef DDB
>> +static void db_show_rm(const struct lock_object *lock);
>> +#endif
>> static void lock_rm(struct lock_object *lock, int how);
>> #ifdef KDTRACE_HOOKS
>> static int owner_rm(const struct lock_object *lock, struct thread
>> **owner);
>
> While here, did you consider also:
> - Abstracting compiler_memory_barrier() into a MI, compiler dependent function?
So what do you think about this patch? (Please double-check the GIT log).
Thanks,
Attilio
Subject: [PATCH 11/11] Add an unified macro to deny ability from the compiler
to reorder instruction loads/stores at its will. The
macro is called __compiler_membar() and it is
currently supported for both gcc and clang (sharing
the same __GNUC__ tag).
Signed-off-by: Attilio Rao <attilio@pcbsd-2488.(none)>
---
sys/amd64/include/atomic.h | 4 ++--
sys/gnu/fs/xfs/FreeBSD/xfs_compat.h | 4 ----
sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h | 2 +-
sys/i386/include/atomic.h | 4 ++--
sys/kern/kern_rmlock.c | 8 ++------
sys/mips/include/cpufunc.h | 2 +-
sys/pc98/include/bus.h | 2 +-
sys/sparc64/include/atomic.h | 6 +++---
sys/sys/cdefs.h | 7 +++++++
sys/x86/include/bus.h | 2 +-
10 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/sys/amd64/include/atomic.h b/sys/amd64/include/atomic.h
index 99a94b7..91c33e6 100644
--- a/sys/amd64/include/atomic.h
+++ b/sys/amd64/include/atomic.h
@@ -226,7 +226,7 @@ atomic_fetchadd_long(volatile u_long *p, u_long v)
static __inline void \
atomic_store_rel_##TYPE(volatile u_##TYPE *p, u_##TYPE v)\
{ \
- __asm __volatile("" : : : "memory"); \
+ __compiler_membar(); \
*p = v; \
} \
struct __hack
@@ -240,7 +240,7 @@ atomic_load_acq_##TYPE(volatile u_##TYPE *p)
\
u_##TYPE tmp; \
\
tmp = *p; \
- __asm __volatile("" : : : "memory"); \
+ __compiler_membar(); \
return (tmp); \
} \
struct __hack
diff --git a/sys/gnu/fs/xfs/FreeBSD/xfs_compat.h
b/sys/gnu/fs/xfs/FreeBSD/xfs_compat.h
index 7229f27..55a03c9 100644
--- a/sys/gnu/fs/xfs/FreeBSD/xfs_compat.h
+++ b/sys/gnu/fs/xfs/FreeBSD/xfs_compat.h
@@ -129,10 +129,6 @@ typedef dev_t os_dev_t;
#define copy_from_user(dst, src, len) copyin((src), (dst), (len))
#endif
-#ifndef barrier
-#define barrier() __asm__ __volatile__("": : :"memory")
-#endif
-
/*
* Map simple global vairables to FreeBSD kernel equivalents
*/
diff --git a/sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h
b/sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h
index a47c413..cc1d8df 100644
--- a/sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h
+++ b/sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h
@@ -162,7 +162,7 @@
*/
#define EFSCORRUPTED 990 /* Filesystem is corrupted */
-#define SYNCHRONIZE() barrier()
+#define SYNCHRONIZE() __compiler_membar()
#define __return_address __builtin_return_address(0)
/*
diff --git a/sys/i386/include/atomic.h b/sys/i386/include/atomic.h
index 6ef5962..3b9d001 100644
--- a/sys/i386/include/atomic.h
+++ b/sys/i386/include/atomic.h
@@ -296,7 +296,7 @@ atomic_fetchadd_int(volatile u_int *p, u_int v)
static __inline void \
atomic_store_rel_##TYPE(volatile u_##TYPE *p, u_##TYPE v)\
{ \
- __asm __volatile("" : : : "memory"); \
+ __compiler_membar(); \
*p = v; \
} \
struct __hack
@@ -310,7 +310,7 @@ atomic_load_acq_##TYPE(volatile u_##TYPE *p)
\
u_##TYPE tmp; \
\
tmp = *p; \
- __asm __volatile("" : : : "memory"); \
+ __compiler_membar(); \
return (tmp); \
} \
struct __hack
diff --git a/sys/kern/kern_rmlock.c b/sys/kern/kern_rmlock.c
index ef1920b..30400b1 100644
--- a/sys/kern/kern_rmlock.c
+++ b/sys/kern/kern_rmlock.c
@@ -65,10 +65,6 @@ __FBSDID("$FreeBSD$");
* does not seem very useful
*/
-static __inline void compiler_memory_barrier(void) {
- __asm __volatile("":::"memory");
-}
-
static void assert_rm(const struct lock_object *lock, int what);
static void lock_rm(struct lock_object *lock, int how);
#ifdef KDTRACE_HOOKS
@@ -353,7 +349,7 @@ _rm_rlock(struct rmlock *rm, struct rm_priotracker
*tracker, int trylock)
td->td_critnest++; /* critical_enter(); */
- compiler_memory_barrier();
+ __compiler_membar();
pc = cpuid_to_pcpu[td->td_oncpu]; /* pcpu_find(td->td_oncpu); */
@@ -361,7 +357,7 @@ _rm_rlock(struct rmlock *rm, struct rm_priotracker
*tracker, int trylock)
sched_pin();
- compiler_memory_barrier();
+ __compiler_membar();
td->td_critnest--;
diff --git a/sys/mips/include/cpufunc.h b/sys/mips/include/cpufunc.h
index 513237f..3611d6e 100644
--- a/sys/mips/include/cpufunc.h
+++ b/sys/mips/include/cpufunc.h
@@ -70,7 +70,7 @@ static __inline void
mips_barrier(void)
{
#if defined(CPU_CNMIPS) || defined(CPU_RMI) || defined(CPU_NLM)
- __asm __volatile("" : : : "memory");
+ __compiler_membar();
#else
__asm __volatile (".set noreorder\n\t"
"nop\n\t"
diff --git a/sys/pc98/include/bus.h b/sys/pc98/include/bus.h
index 69e80f0..46d1a1b 100644
--- a/sys/pc98/include/bus.h
+++ b/sys/pc98/include/bus.h
@@ -593,7 +593,7 @@ bus_space_barrier(bus_space_tag_t tag,
bus_space_handle_t bsh,
if (flags & BUS_SPACE_BARRIER_READ)
__asm __volatile("lock; addl $0,0(%%esp)" : : : "memory");
else
- __asm __volatile("" : : : "memory");
+ __compiler_membar();
}
#ifdef BUS_SPACE_NO_LEGACY
diff --git a/sys/sparc64/include/atomic.h b/sys/sparc64/include/atomic.h
index 06a1984..447880a 100644
--- a/sys/sparc64/include/atomic.h
+++ b/sys/sparc64/include/atomic.h
@@ -97,7 +97,7 @@
#define atomic_cas_acq(p, e, s, sz) ({
\
itype(sz) v; \
v = atomic_cas((p), (e), (s), sz); \
- __asm __volatile("" : : : "memory"); \
+ __compiler_membar(); \
v; \
})
@@ -122,7 +122,7 @@
#define atomic_op_acq(p, op, v, sz) ({
\
itype(sz) t; \
t = atomic_op((p), op, (v), sz); \
- __asm __volatile("" : : : "memory"); \
+ __compiler_membar(); \
t; \
})
@@ -139,7 +139,7 @@
#define atomic_load_acq(p, sz) ({
\
itype(sz) v; \
v = atomic_load((p), sz); \
- __asm __volatile("" : : : "memory"); \
+ __compiler_membar(); \
v; \
})
diff --git a/sys/sys/cdefs.h b/sys/sys/cdefs.h
index 8224672..fc6a75f 100644
--- a/sys/sys/cdefs.h
+++ b/sys/sys/cdefs.h
@@ -114,6 +114,13 @@
#endif
/*
+ * Compiler memory barriers, specific to gcc and clang.
+ */
+#if defined(__GNUC__)
+#define __compiler_membar() __asm __volatile(" " : : : "memory")
+#endif
+
+/*
* The __CONCAT macro is used to concatenate parts of symbol names, e.g.
* with "#define OLD(foo) __CONCAT(old,foo)", OLD(foo) produces oldfoo.
* The __CONCAT macro is a bit tricky to use if it must work in non-ANSI
diff --git a/sys/x86/include/bus.h b/sys/x86/include/bus.h
index 3b4342a..fb5babf 100644
--- a/sys/x86/include/bus.h
+++ b/sys/x86/include/bus.h
@@ -1014,7 +1014,7 @@ bus_space_barrier(bus_space_tag_t tag __unused,
bus_space_handle_t bsh __unused,
__asm __volatile("lock; addl $0,0(%%esp)" : : : "memory");
#endif
- __asm __volatile("" : : : "memory");
+ __compiler_membar();
#endif
}
--
1.7.7.4
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndCRg0UCThFkatp=tw7rUWWCvhsApLE=iztLpxpGBC1F9w>