[openib-general] Re: Re: ipoib_mcast_send.patch

[Michael S. Tsirkin]

Quoting r. Roland Dreier <rdreier at cisco.com>:
> Subject: Re: Re: ipoib_mcast_send.patch
> 
>     Michael> Right, but I thought atomic test_and_set_bit implied
>     Michael> smp_wmb already?
> 
> So did I but then I looked in the kernel source and now I think that
> set_bit operations are only ordered against other bitops that touch
> the same word.  For example ia64 just uses cmpxchg to implement the
> bitops, and powerpc just uses locked loads and stores.
> 
>  - R.
> 


Hmm. Roland, which kernel version is that? 

On 2.6.15 I see in include/asm-powerpc/bitops.h

static __inline__ int test_and_set_bit(unsigned long nr,
                                       volatile unsigned long *addr)
{
        unsigned long old, t;
        unsigned long mask = BITOP_MASK(nr);
        unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);

        __asm__ __volatile__(
        EIEIO_ON_SMP
"1:"    PPC_LLARX "%0,0,%3              # test_and_set_bit\n"
        "or     %1,%0,%2 \n"
        PPC405_ERR77(0,%3)
        PPC_STLCX "%1,0,%3 \n"
        "bne-   1b"
        ISYNC_ON_SMP
        : "=&r" (old), "=&r" (t)
        : "r" (mask), "r" (p)
        : "cc", "memory");

        return (old & mask) != 0;
}

EIEIO_ON_SMP is a write barrier on smp, isnt it?

I see this in 2.6.11: include/asm-ppc64/bitops.h

static __inline__ int test_and_set_bit(unsigned long nr, volatile unsigned long
*addr)
{
        unsigned long old, t;
        unsigned long mask = 1UL << (nr & 0x3f);
        unsigned long *p = ((unsigned long *)addr) + (nr >> 6);

        __asm__ __volatile__(
        EIEIO_ON_SMP
"1:     ldarx   %0,0,%3         # test_and_set_bit\n\
        or      %1,%0,%2 \n\
        stdcx.  %1,0,%3 \n\
        bne-    1b"
        ISYNC_ON_SMP
        : "=&r" (old), "=&r" (t)
        : "r" (mask), "r" (p)
        : "cc", "memory");

        return (old & mask) != 0;
}

EIEIO_ON_SMP is exactly what is needed, no?

/*
 * The test_and_*_bit operations are taken to imply a memory barrier
 * on SMP systems.
 */


...

/*
 * test_and_*_bit do imply a memory barrier (?)
 */
static __inline__ int test_and_set_bit(int nr, volatile unsigned long *addr)
{
        unsigned int old, t;
        unsigned int mask = 1 << (nr & 0x1f);
        volatile unsigned int *p = ((volatile unsigned int *)addr) + (nr >> 5);

        __asm__ __volatile__(SMP_WMB "\n\
1:      lwarx   %0,0,%4 \n\
        or      %1,%0,%3 \n"
        PPC405_ERR77(0,%4)
"       stwcx.  %1,0,%4 \n\
        bne     1b"
        SMP_MB
        : "=&r" (old), "=&r" (t), "=m" (*p)
        : "r" (mask), "r" (p), "m" (*p)
        : "cc", "memory");

        return (old & mask) != 0;
}

Ahem. It does look to me like atomics imply smp_wmb.

-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies