lkml.org 
[lkml]   [2017]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/8] asm-generic: add atomic-instrumented.h
    Hi,

    On Tue, Mar 28, 2017 at 06:15:41PM +0200, Dmitry Vyukov wrote:
    > The new header allows to wrap per-arch atomic operations
    > and add common functionality to all of them.

    I had a quick look at what it would take to have arm64 use this, and I
    have a couple of thoughts.

    > +static __always_inline int atomic_xchg(atomic_t *v, int i)
    > +{
    > + return arch_atomic_xchg(v, i);
    > +}

    I generally agree that avoiding several layers of CPP aids readability
    here, and as-is I think this is fine.

    However, avoiding CPP entirely will mean that the file becomes painfully
    verbose when support for {relaxed,acquire,release}-order variants is
    added.

    Just considering atomic_xchg{,_relaxed,_acquire,_release}(), for
    example:

    ----
    static __always_inline int atomic_xchg(atomic_t *v, int i)
    {
    kasan_check_write(v, sizeof(*v));
    return arch_atomic_xchg(v, i);
    }

    #ifdef arch_atomic_xchg_relaxed
    static __always_inline int atomic_xchg(atomic_t *v, int i)
    {
    kasan_check_write(v, sizeof(*v));
    return arch_atomic_xchg_relaxed(v, i);
    }
    #define atomic_xchg_relaxed atomic_xchg_relaxed
    #endif

    #ifdef arch_atomic_xchg_acquire
    static __always_inline int atomic_xchg(atomic_t *v, int i)
    {
    kasan_check_write(v, sizeof(*v));
    return arch_atomic_xchg_acquire(v, i);
    }
    #define atomic_xchg_acquire atomic_xchg_acquire
    #endif

    #ifdef arch_atomic_xchg_release
    static __always_inline int atomic_xchg(atomic_t *v, int i)
    {
    kasan_check_write(v, sizeof(*v));
    return arch_atomic_xchg_release(v, i);
    }
    #define atomic_xchg_release atomic_xchg_release
    #endif
    ----


    With some minimal CPP, it can be a lot more manageable:

    ----
    #define INSTR_ATOMIC_XCHG(order) \
    static __always_inline int atomic_xchg##order(atomic_t *v, int i) \
    { \
    kasan_check_write(v, sizeof(*v)); \
    arch_atomic_xchg##order(v, i); \
    }

    #define INSTR_ATOMIC_XCHG()

    #ifdef arch_atomic_xchg_relaxed
    INSTR_ATOMIC_XCHG(_relaxed)
    #define atomic_xchg_relaxed atomic_xchg_relaxed
    #endif

    #ifdef arch_atomic_xchg_acquire
    INSTR_ATOMIC_XCHG(_acquire)
    #define atomic_xchg_acquire atomic_xchg_acquire
    #endif

    #ifdef arch_atomic_xchg_relaxed
    INSTR_ATOMIC_XCHG(_relaxed)
    #define atomic_xchg_relaxed atomic_xchg_relaxed
    #endif
    ----


    Is there any objection to some light CPP usage as above for adding the
    {relaxed,acquire,release} variants?

    Thanks,
    Mark.

    \
     
     \ /
      Last update: 2017-03-29 19:17    [W:5.250 / U:0.064 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site