lkml.org 
[lkml]   [2015]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH tip/core/rcu 01/14] rcu: Protect rcu_boost() lockless accesses with ACCESS_ONCE()
    Am 12.01.2015 um 23:12 schrieb Paul E. McKenney:
    > On Mon, Jan 12, 2015 at 09:59:57AM +0100, Peter Zijlstra wrote:
    >> On Fri, Jan 09, 2015 at 10:58:50PM +0100, Christian Borntraeger wrote:
    >>> Am 09.01.2015 um 14:56 schrieb Peter Zijlstra:
    >>>> On Fri, Jan 09, 2015 at 05:49:54AM -0800, Paul E. McKenney wrote:
    >>>>>> That reminds me, I think the new conversion for stores will most likely
    >>>>>> introduce silly arg bugs:
    >>>>>>
    >>>>>> - ACCESS_ONCE(a) = b;
    >>>>>> + ASSIGN_ONCE(b, a);
    >>>>>
    >>>>> I was planning to do mine by hand for this sort of reason.
    >>>>>
    >>>>> Or are you thinking of something more subtle than the case where
    >>>>> "b" is an unparenthesized comma-separated expression?
    >>>>
    >>>> I think he's revering to the wrong way around-ness of the thing.
    >>>>
    >>>> Its a bit of a mixed bag on assignments, but for instance
    >>>> rcu_assign_pointer() takes them the right way around, as does
    >>>> atomic_set().
    >>>>
    >>>> So yes, I think the ASSIGN_ONCE() thing got the arguments the wrong way
    >>>> around.
    >>>>
    >>>> We could maybe still change it, before its in too long ?
    >>>
    >>> Linus initial proposal was inspired by put_user model which is (val,
    >>> ptr) and I took that.
    >>
    >> Yeah, like I said, its a bit of a mixed bag. We've got plenty examples
    >> of the wrong way around.
    >>
    >>> As my focus was on avoiding the volatile bug,
    >>> all my current conversions are READ_ONCE as no potential ASSIGN_ONCE
    >>> user was done on a non-scalar type, so I have no first hand
    >>> experience.
    >>
    >> So the implication there is that we'd preserve ACCESS_ONCE() for use on
    >> scalar types. I don't think we should do that, I think we should just
    >> en-mass convert to {READ,WRITE}/{LOAD,STORE}_ONCE() and kill off
    >> ACCESS_ONCE().
    >
    > Yep. For one thing, the proposed replacements work much better with
    > C11 than does ACCESS_ONCE().

    As we agreed there is no perfect interface regarding val,x vs. x,val.
    But it seems that there is some consensus that I should push something like the following (still whitespace damaged) to Linus for 3.19?
    Peter, Davidlohr, Paul (maybe Linus) can you ACK/NACK?


    Subject: Change ASSIGN_ONCE(val, x) to WRITE_ONCE(x, val)

    Feedback has shown that WRITE_ONCE(x, val) is easier to use than ASSIGN_ONCE(val,x).
    There are no in-tree users yet, so lets change it.

    Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>


    diff --git a/include/linux/compiler.h b/include/linux/compiler.h
    index 84734a7..38865c7 100644
    --- a/include/linux/compiler.h
    +++ b/include/linux/compiler.h
    @@ -215,7 +215,7 @@ static __always_inline void __read_once_size(volatile void *p, void *res, int si
    }
    }

    -static __always_inline void __assign_once_size(volatile void *p, void *res, int size)
    +static __always_inline void __write_once_size(volatile void *p, void *res, int size)
    {
    switch (size) {
    case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
    @@ -235,15 +235,15 @@ static __always_inline void __assign_once_size(volatile void *p, void *res, int
    /*
    * Prevent the compiler from merging or refetching reads or writes. The
    * compiler is also forbidden from reordering successive instances of
    - * READ_ONCE, ASSIGN_ONCE and ACCESS_ONCE (see below), but only when the
    + * READ_ONCE, WRITE_ONCE and ACCESS_ONCE (see below), but only when the
    * compiler is aware of some particular ordering. One way to make the
    * compiler aware of ordering is to put the two invocations of READ_ONCE,
    - * ASSIGN_ONCE or ACCESS_ONCE() in different C statements.
    + * WRITE_ONCE or ACCESS_ONCE() in different C statements.
    *
    * In contrast to ACCESS_ONCE these two macros will also work on aggregate
    * data types like structs or unions. If the size of the accessed data
    * type exceeds the word size of the machine (e.g., 32 bits or 64 bits)
    - * READ_ONCE() and ASSIGN_ONCE() will fall back to memcpy and print a
    + * READ_ONCE() and WRITE_ONCE() will fall back to memcpy and print a
    * compile-time warning.
    *
    * Their two major use cases are: (1) Mediating communication between
    @@ -257,8 +257,8 @@ static __always_inline void __assign_once_size(volatile void *p, void *res, int
    #define READ_ONCE(x) \
    ({ typeof(x) __val; __read_once_size(&x, &__val, sizeof(__val)); __val; })

    -#define ASSIGN_ONCE(val, x) \
    - ({ typeof(x) __val; __val = val; __assign_once_size(&x, &__val, sizeof(__val)); __val; })
    +#define WRITE_ONCE(x, val) \
    + ({ typeof(x) __val; __val = val; __write_once_size(&x, &__val, sizeof(__val)); __val; })

    #endif /* __KERNEL__ */

    @@ -458,7 +458,7 @@ static __always_inline void __assign_once_size(volatile void *p, void *res, int
    * with an explicit memory barrier or atomic instruction that provides the
    * required ordering.
    *
    - * If possible use READ_ONCE/ASSIGN_ONCE instead.
    + * If possible use READ_ONCE/WRITE_ONCE instead.
    */
    #define __ACCESS_ONCE(x) ({ \
    __maybe_unused typeof(x) __var = (typeof(x)) 0;




    \
     
     \ /
      Last update: 2015-01-13 09:41    [W:3.324 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site