[ofa-general] clueless noob and build probs with 1.2rc2

Roland Dreier rdreier at cisco.com
Wed Apr 25 08:46:04 PDT 2007


 > The built-in atomics in gcc had some ia64 issues until gcc 4.1.1 so we
 > had no choice but to get down and do some bogus things. :^)

Actually I think you've underestimated the depth of the bogosity.
Many of the uses of atomics in dapl that a quick grep turns up look to
be cargo cult uses where the atomic operations are used in a way that
doesn't protect against races.  eg dapl_cookie.c:

    new_head = (dapl_os_atomic_read (&buffer->head) + 1) % buffer->pool_size;

    if ( new_head == dapl_os_atomic_read (&buffer->tail) )
    {   
        dat_status = DAT_INSUFFICIENT_RESOURCES;
        goto bail;
    }
    else
    {   
        dapl_os_atomic_set (&buffer->head, new_head);

if there's no other locking on buffer->head, then there's a race
between the dapl_os_atomic_read() and the dapl_os_atomic_set().  And
if there is other locking, then there's no point in making
buffer->head be an atomic variable.

Since you only have atomic_inc and atomic_dec and not anything like
atomic_dec_and_test, then there's not really any race-free way to use
the atomic variables.  I guess the uses like evd_ref_count are OK,
since that's really only a hint about whether something is free, but
there's not much point in taking the portability hassles of trying to
use atomics -- I think using a pthread mutex would be pretty much
equivalent in performance in the common uncontended case, and anyway
wherever you're doing the reference counting is not a hot path.

And don't even make me get started on the places where
dapl_os_atomic_assign() (really cmpxchg) is used, like dapls_rbuf_add()

So the best thing to do (assuming you're stuck with udapl) would
probably be to get rid of all the dapl_os_atomic_xxx junk and just use
portable pthreads locking.



More information about the general mailing list