Are the memory barriers correct for this lock?

Question

Is this correct? And am I correct in assuming that applying the memory ordering on the std::atomic_flag does NOT provide synchronization for general-purpose locks?

#include <atomic>

class Spinlock
{
public:
    Spinlock(): f(ATOMIC_FLAG_INIT) {}

    void lock()
    {
        while(f.test_and_set(std::memory_order_relaxed));
        std::atomic_thread_fence(std::memory_order_acquire);
    }
    void unlock()
    {
        std::atomic_thread_fence(std::memory_order_release);
        f.clear(std::memory_order_relaxed);
    }

private:
    std::atomic_flag f;
};

I'm sorry if this is a stupid question, but I feel like an std::atmoic_thread_fence IS necessary for a general-purpose lock, and that applying memory_order_acquire on the test_and_set and memory_order_release on the clear is not enough, but I'm also not sure.

Answer 1

Overall it is correct. Since you used 'std::memory_order_relaxed' in the test_and_set function then without the 'atomic_thread_fence' call there is nothing that would prevent reordering of operations done before and after the Mutex::lock. Since it is expected that 'Mutex::lock' acts as a memory barrier then the call to 'atomic_thread_fence' becomes necessary. But I believe the same effect could be achieved by using 'std::memory_order_acquire' in the 'test_and_set function'. See this: http://en.cppreference.com/w/cpp/atomic/atomic_flag