Skip site navigation (1)Skip section navigation (2)
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>