views:

154

answers:

2

I'm not quite sure if it's safe to spin on a volatile variable in user-mode threads, to implement a light-weight spin_lock, I looked at the tbb source code, tbb_machine.h:170,

//! Spin WHILE the value of the variable is equal to a given value
/** T and U should be comparable types. */
template<typename T, typename U>
void spin_wait_while_eq( const volatile T& location, U value ) {
    atomic_backoff backoff;
    while( location==value ) backoff.pause();
}

And there is no fences in atomic_backoff class as I can see. While from other user-mode spin_lock implementation, most of them use CAS (Compare and Swap).

+2  A: 

Tricky. I'd say that in theory, this code isn't safe. If there are no memory barriers then the data accesses you're guarding could be moved across the spinlock. However, this would only be done if the compiler inlined very aggressively, and could see a purpose in this reordering.

Perhaps Intel simply determined that this code works on all current compilers, and that even though a compiler could theoretically perform transformations that'd break this code, those transformations wouldn't speed up the program, and so compilers probably won't do it.

Another possibility is that this code is only used on compilers that implicitly implement memory barriers around volatile accesses. Microsoft's Visual C++ compiler (2005 and later) does this. Have you checked if this is wrapped in an #ifdef block or similar, applying this implementation only on compilers where volatile use memory barriers?

jalf
"Perhaps Intel simply determined that this code works on all current compilers". TBB is inherently not portable. Probably they determined that it works on all the compilers/platforms that they support, not necessarily all current compilers. When they say their code works on GCC, I suspect that doesn't necessarily mean it works on every platform GCC targets.
Steve Jessop
@Steve: Fair enough. I know it's not portable. What I meant was that they felt confident that it worked on 1) currently supported compilers and 2) compilers they're likely to support in the future (primarily newer versions of these compilers, like VC2010 and GCC4.5)
jalf
jalf, thank you for the answer :)
yongsun
A: 

Note that this is a spin wait and not a spin lock. There is no write operation involved. Nothing is acquired.

If you tried to add a write operation to complete the locking process, you would have an unsolvable race.

Potatoswatter
@Potatoswatter, if I need add a write operation, what should I do to complete this? to place a load_with_acquire in spin_wait_while_eq, and a store_with_release in the writer?
yongsun
@yongsun: Don't. Use some library implementation, such as `pthread_mutex_lock`. I'd imagine TBB should include something appropriate too. You could use atomic variables to roll your own, atomics being anything you can find that implements load_with_acquire and store_with_release. But that will likely be suboptimal compared to any library.
Potatoswatter