lkml.org 
[lkml]   [2005]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch 1/3] mutex subsystem: trylock
    On Tue, 27 Dec 2005, Ingo Molnar wrote:

    >
    > * Arjan van de Ven <arjan@infradead.org> wrote:
    >
    > > > + * 1) if the exclusive store fails we fail, and
    > > > + *
    > > > + * 2) if the decremented value is not zero we don't even attempt the store.
    > >
    > >
    > > btw I really think that 1) is wrong. trylock should do everything it
    > > can to get the semaphore short of sleeping. Just because some
    > > cacheline got written to (which might even be shared!) in the middle
    > > of the atomic op is not a good enough reason to fail the trylock imho.
    > > Going into the slowpath.. fine. But here it's a quality of
    > > implementation issue; you COULD get the semaphore without sleeping (at
    > > least probably, you'd have to retry to know for sure) but because
    > > something wrote to the same cacheline as the lock... no. that's just
    > > not good enough.. sorry.
    >
    > point. I solved this in my tree by calling the generic trylock <fn> if
    > there's an __ex_flag failure in the ARMv6 case. Should be rare (and thus
    > the call is under unlikely()), and should thus still enable the fast
    > implementation.

    I'd solve it like this instead (on top of your latest patches):

    Index: linux-2.6/include/asm-arm/mutex.h
    ===================================================================
    --- linux-2.6.orig/include/asm-arm/mutex.h
    +++ linux-2.6/include/asm-arm/mutex.h
    @@ -110,12 +110,7 @@ do { \

    /*
    * For __mutex_fastpath_trylock we use another construct which could be
    - * described as an "incomplete atomic decrement" or a "single value cmpxchg"
    - * since it has two modes of failure:
    - *
    - * 1) if the exclusive store fails we fail, and
    - *
    - * 2) if the decremented value is not zero we don't even attempt the store.
    + * described as a "single value cmpxchg".
    *
    * This provides the needed trylock semantics like cmpxchg would, but it is
    * lighter and less generic than a true cmpxchg implementation.
    @@ -123,27 +118,22 @@ do { \
    static inline int
    __mutex_fastpath_trylock(atomic_t *count, int (*fn_name)(atomic_t *))
    {
    - int __ex_flag, __res;
    + int __ex_flag, __res, __orig;

    __asm__ (

    - "ldrex %0, [%2] \n"
    - "subs %0, %0, #1 \n"
    - "strexeq %1, %0, [%2] \n"
    + "1: ldrex %0, [%3] \n"
    + "subs %1, %0, #1 \n"
    + "strexeq %2, %1, [%3] \n"
    + "movlt %0, #0 \n"
    + "cmpeq %2, #0 \n"
    + "bgt 1b \n"

    - : "=&r" (__res), "=&r" (__ex_flag)
    + : "=&r" (__orig), "=&r" (__res), "=&r" (__ex_flag)
    : "r" (&count->counter)
    : "cc", "memory" );

    - /*
    - * We must not return a synthetic 'failure' if the conditional
    - * did not succeed - drop back into the generic slowpath if
    - * this happens (should be rare):
    - */
    - if (unlikely(__ex_flag))
    - return fn_name(count);
    -
    - return __res == 0;
    + return __orig;
    }

    #endif

    Nicolas
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-12-29 05:09    [W:3.735 / U:0.656 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site