Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Sep 2012 20:00:48 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Davide Italiano <davide@freebsd.org>, src-committers@freebsd.org, John Baldwin <jhb@freebsd.org>, Jeff Roberson <jeff@freebsd.org>, Dimitry Andric <dim@freebsd.org>, svn-src-projects@freebsd.org
Subject:   Re: svn commit: r238907 - projects/calloutng/sys/kern
Message-ID:  <CAJ-FndCmg-GdTf9FWQGZPYB-iGuEt-JH8HrCL%2BwaOqJ%2B8ZUbHQ@mail.gmail.com>
In-Reply-To: <CAJ-FndAXw6zjXr=zB3gAVQDKUV_K4=SF39iYQQOV23NkfJ=MPw@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> <CAJ-FndCRg0UCThFkatp=tw7rUWWCvhsApLE=iztLpxpGBC1F9w@mail.gmail.com> <20120918083324.GX37286@deviant.kiev.zoral.com.ua> <CAJ-FndAXw6zjXr=zB3gAVQDKUV_K4=SF39iYQQOV23NkfJ=MPw@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Sep 18, 2012 at 4:30 PM, Attilio Rao <attilio@freebsd.org> wrote:
> On 9/18/12, Konstantin Belousov <kostikbel@gmail.com> wrote:
>> On Tue, Sep 18, 2012 at 01:13:08AM +0100, Attilio Rao wrote:
>>> 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
>>> +
>>> +/*
>>
>> Traditionally, we do provide the fallback for non-GNUC compilers, by
>> defining extern function with the compatible signature. In this case,
>> the empty function just works for the purpose, although with higher
>> overhead than the GNUC case.
>
> I agree, we need a fallback here. Unfortunately I'm buried with job
> stuff but I will provide an errata patch ASAP.

Here is the patch. I didn't use a real extern function body for it,
but just went with an empty macro.

Attilio


Index: sys/sparc64/include/atomic.h
===================================================================
--- sys/sparc64/include/atomic.h        (revisione 240672)
+++ sys/sparc64/include/atomic.h        (copia locale)
@@ -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;                                                              \
 })

Index: sys/pc98/include/bus.h
===================================================================
--- sys/pc98/include/bus.h      (revisione 240672)
+++ sys/pc98/include/bus.h      (copia locale)
@@ -593,7 +593,7 @@ bus_space_barrier(bus_space_tag_t tag, bus_space_h
        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
Index: sys/i386/include/atomic.h
===================================================================
--- sys/i386/include/atomic.h   (revisione 240672)
+++ sys/i386/include/atomic.h   (copia locale)
@@ -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
Index: sys/amd64/include/atomic.h
===================================================================
--- sys/amd64/include/atomic.h  (revisione 240672)
+++ sys/amd64/include/atomic.h  (copia locale)
@@ -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
Index: sys/sys/cdefs.h
===================================================================
--- sys/sys/cdefs.h     (revisione 240672)
+++ sys/sys/cdefs.h     (copia locale)
@@ -487,6 +487,15 @@
 #define        __GLOBL1(sym)   __asm__(".globl " #sym)
 #define        __GLOBL(sym)    __GLOBL1(sym)

+/*
+ * Compiler memory barriers, specific to gcc and clang.
+ */
+#if defined(__GNUC__)
+#define        __compiler_membar()     __asm __volatile(" " : : : "memory")
+#else
+#define        __compiler_membar()     struct __hack
+#endif
+
 #if defined(__GNUC__) || defined(__INTEL_COMPILER)
 #define        __IDSTRING(name,string) __asm__(".ident\t\"" string "\"")
 #else
Index: sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h
===================================================================
--- sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h        (revisione 240672)
+++ sys/gnu/fs/xfs/FreeBSD/xfs_freebsd.h        (copia locale)
@@ -162,7 +162,7 @@
  */
 #define EFSCORRUPTED    990            /* Filesystem is corrupted */

-#define SYNCHRONIZE()  barrier()
+#define SYNCHRONIZE()  __compiler_membar()
 #define __return_address __builtin_return_address(0)

 /*
Index: sys/gnu/fs/xfs/FreeBSD/xfs_compat.h
===================================================================
--- sys/gnu/fs/xfs/FreeBSD/xfs_compat.h (revisione 240672)
+++ sys/gnu/fs/xfs/FreeBSD/xfs_compat.h (copia locale)
@@ -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
  */
Index: sys/kern/kern_rmlock.c
===================================================================
--- sys/kern/kern_rmlock.c      (revisione 240672)
+++ sys/kern/kern_rmlock.c      (copia locale)
@@ -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

        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

        sched_pin();

-       compiler_memory_barrier();
+       __compiler_membar();

        td->td_critnest--;

Index: sys/mips/include/cpufunc.h
===================================================================
--- sys/mips/include/cpufunc.h  (revisione 240672)
+++ sys/mips/include/cpufunc.h  (copia locale)
@@ -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"
Index: sys/x86/include/bus.h
===================================================================
--- sys/x86/include/bus.h       (revisione 240672)
+++ sys/x86/include/bus.h       (copia locale)
@@ -1014,7 +1014,7 @@ bus_space_barrier(bus_space_tag_t tag __unused, bu
                __asm __volatile("lock; addl $0,0(%%esp)" : : : "memory");
 #endif
        else
-               __asm __volatile("" : : : "memory");
+               __compiler_membar();
 #endif
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndCmg-GdTf9FWQGZPYB-iGuEt-JH8HrCL%2BwaOqJ%2B8ZUbHQ>