Is volatile required here

Question

I'm implementing a 'sequence lock' class to allow locked write and lock-free reads of a data structure.

The struct that will contain the data contains the sequence value, which will be incremented twice while the write takes place. Once before the writing starts, and once after the writing is completed. The writer is on other threads than the reader(s).

This is what the struct that holds a copy of the data, and the sequence value looks like:

template<typename T>
struct seq_data_t
{
    seq_data_t() : seq(0) {};
    int seq;                     <- should this be 'volatile int seq;'?
    T data;
};

The whole sequence lock class holds N copies of this structure in a circular buffer. Writer threads always write over the oldest copy of the data in the circular buffer, then mark it as the current copy. The writing is mutex locked.

The read function does not lock. It attempts to read the 'current' copy of the data. It stores the 'seq' value before reading. Then it reads data. Then it reads the seq value again, and compares it to the value it read the first time. If the seq value has not changed, the read is deemed to be good.

Since the writer thread could change the value of 'seq' while a read is occurring, I'm thinking that the seq variable should be marked volatile, so that the read function will explicitly read the value after it reads the data.

The read function looks like this: It will be on threads other than the writer, and perhaps several threads.

    void read(std::function<void(T*)>read_function)
    {
        for (;;)
        {
            seq_data_type<T>* d = _data.current; // get current copy
            int seq1 = d->seq;      // store starting seq no
            if (seq1 % 2)           // if odd, being modified...
                continue;           //     loop back

            read_function(&d->data);  // call the passed in read function
                                      // passing it our data.


//??????? could this read be optimized out if seq is not volatile?
            int seq2 = d->seq;      // <-- does this require that seq be volatile?
//???????

            if (seq1 == seq2)       // if still the same, good.
                return;             // if not the same, we will stay in this
                                    // loop until this condition is met.
        }
    }

Questions:

1) must seq be volatile in this context?

2) in the context of a struct with multiple members, are only the volatile qualified variable volatile, and not the other members? i.e. is only 'seq' volatile if I only mark it volatile within the struct?

Answer 1

1) must seq be volatile in this context?

Sure, most probably the read from seq will be optimized out with -O3. So yes, you should hint the compiler that seq might be changed elsewhere (i.e. in other thread) with volatile keyword.

For x86 architecture it would be enough, because x86 memory model is (almost) sequential as described on Wikipedia.

For portability, you better use atomic primitives.

2) in the context of a struct with multiple members, are only the volatile qualified variable volatile, and not the other members? i.e. is only 'seq' volatile if I only mark it volatile within the struct?

No, the data should be marked as volatile as well (or you should use atomic primitives as well). Basically, the loop:

for (;;) {
    seq1 = d->seq;
    read_data(d->data);
    seq2 = d->seq;
    if (seq1 == seq2)
        return;
}

is equivalent to:

read_data(d->data);
return;

Because the only observable effect in the code is the read_data() call.

Please note, that most likely with -O3 compiler will reorder your code quite extensively. So even for x86 architecture you will need a compiler barriers between first seq read, data read and second seq read, i.e.:

for (;;)
    {
        seq_data_type<T>* d = _data.current;
        int seq1 = d->seq;
        COMPILER_BARRIER();
        if (seq1 % 2)
            continue;

        read_function(&d->data);
        COMPILER_BARRIER();
        int seq2 = d->seq;
        if (seq1 == seq2)
            return;
    }
}

The most lightweight compiler barrier is:

 #define COMPILER_BARRIER asm volatile("" ::: "memory")

For C++11 you can use atomic_signal_fence() instead.

Overall, it is safer to use std::atomic<>: it is more portable and not that tricky as juggling with volatiles and compiler barriers...

Please also have a look at Herb Sutter's presentation called "atomic<> Weapons", which explains compiler and other memory barriers as well as atomics: https://channel9.msdn.com/Shows/Going+Deep/Cpp-and-Beyond-2012-Herb-Sutter-atomic-Weapons-1-of-2

Answer 2

Don't use volatile, use std::atomic<>. volatile is designed and meant to be used for interacting with memory mapped hardware, std::atomic<> is designed and meant to be used for thread synchronization. Use the right tool for the job.

Features of good std::atomic<> implementations:

• They are lockless for standard integer types (everything up to long long, usually).

• They work with any data type, but will use a transparent mutex for complex data types.

• If std::atomic<> is lockless, it inserts the correct memory barriers/fences to achieve correct semantics.

• Manipulations of std::atomic<> cannot be optimized away, they are designed for inter-thread communication after all.

Answer 3

The answer it: it depends. Do you have a reason to suspect that your compiler is not aware that code executed from within callback functions can be executed at any time? This is usually not the case on hosted system compilers (Windows/Linux etc), but may very well be the case in embedded systems, particularly bare metal or RTOS.

This topic is kind of a beaten dead horse, for example here:

What volatile does:

• Guarantees an up-to-date value in the variable, if the variable is modified from an external source (a hardware register, an interrupt, a different thread, a callback function etc).
• Blocks all optimizations of read/write access to the variable.
• Prevent dangerous optimization bugs that can happen to variables shared between several threads/interrupts/callback functions, when the compiler does not realize that the thread/interrupt/callback is called by the program. (This is particularly common among various questionable embedded system compilers, and when you get this bug it is very hard to track down.)

What volatile does not:

• It does not guarantee atomic access or any form of thread-safety.
• It cannot be used instead of a mutex/semaphore/guard/critical section. It cannot be used for thread synchronization.

What volatile may or may not do:

• It may or may not be implemented by the compiler to provide a memory barrier, to protect against instruction cache/instruction pipe/instruction re-ordering issues in a multi-core environment. You should never assume that volatile does this for you, unless the compiler documentation explicitly states that it does.

Answer 4

As said Is volatile required here - You sholud not use volatile for inter-thread synchronization. Here is why (from C++ standard):

[..] volatile is a hint to the implementation to avoid aggressive optimization involving the object because the value of the object might be changed by means undetectable by an implementation.[...]

What volatile doesn't do is ensure that the sequence of the operations (and especially memory reads and writes) in one thread is visible in the same order in other threads (due to superscalar architecture of modern CPUs) . For this you need memory barriers or memory fences (different names for same thing). Here is some more reading that you may find useful:

• What is a memory fence?
• memory orderes for memory barriers - http://en.cppreference.com/w/cpp/atomic/memory_order

Answer 5

The actual problem is that reading from some memory (data in this case) while it's being written to is described as a data race, and therefore the behavior of the program is undefined. Even if you make seq atomic, reading from data will still cause a data race. One possible correct approach is to lock on read as well.

Answering your question of whether volatile solves the read from seq being optimized out: the compiler won't remove both reads from seq, but that doesn't solve anything, because seq is still prone to data races, resulting in undefined behavior as well. That's not what volatile is meant for, so don't abuse it.

Answer 6

If code is to be portable, volatile is never appropriate unless dealing with memory-mapped hardware. I repeat, never appropriate. Microsoft Visual C++, (x86 or x86/64), using the default compiler flags, adds some memory-order guarantees that are not in the standard. So using that compiler, with the non-standard behavior turned on, volatile might work for some multi-threading operations.

Use the standard multi-threading support, such as std::atomic, std::mutex, std::condition_variable, etc.