WT-3127 bug: CPU yield calls don't necessarily imply memory barriers

[keithbostic]

bug: CPU yield calls don't necessarily imply memory barriers
Add a full-barrier as part of the yield call.

[agorrod]

@keithbostic which uses of our yield function assume that it acts as a barrier? I don't mind the change, but I'd like to understand the motivation for making it.

[keithbostic]

@agorrod

which uses of our yield function assume that it acts as a barrier?

We have loops like this one from __log_slot_close:

/*
 * A thread setting the unbuffered flag sets the unbuffered size after
 * setting the flag.  There could be a delay between a thread setting
 * the flag, a thread closing the slot, and the original thread setting
 * that value.  If the state is unbuffered, wait for the unbuffered
 * size to be set.
 */
while (WT_LOG_SLOT_UNBUFFERED_ISSET(old_state) &&
    slot->slot_unbuffered == 0)
        __wt_yield();

Technically, I don't think there's anything that prevents caching slot->slot_unbuffered in a register instead of re-reading it from memory in each iteration.

[agorrod]

I don't think there's anything that prevents caching slot->slot_unbuffered in a register instead of re-reading it from memory in each iteration

Does a barrier in a function call change that? I thought the way to ensure the compiler doesn't cache a local copy was to mark the field volatile?

I like this change - I'm just trying to make sure I understand..

[keithbostic]

Does a barrier in a function call change that? I thought the way to ensure the compiler doesn't cache a local copy was to mark the field volatile?

Yes, the compiler can't cache a value across a memory barrier. Declaring volatile would have the same effect with respect to variable caching, but would require all callers of __wt_yield to add volatile declarations, so this is a simpler change. (To be fair, a barrier is a blunter change as no values can be cached across the call, whereas only a single value must be discarded with volatile. Since we're yielding the CPU, I don't think it matters here.)

I don't like using volatile much (here's the Linux kernel commentary, which covers the discussion pretty well), but there are smarter people than me who strongly disagree. :)

[agorrod]

Thanks for the explanations @keithbostic lgtm