Hi!
"Kristian" == Kristian Nielsen <knielsen@knielsen-hq.org> writes:
<cut>
So there is a barrier between we set it and potentially clear it. As the 'clear' may now happen 'any time' (from other threads point of view) I don't see why it needs to be protected.
Kristian> Right. Kristian> I looked into this again. So we do need a memory barrier, I will try to Kristian> explain this more below. However, it turns out we already have this memory Kristian> barrier. Because in all relevant cases we call wait_for_commit::wakeup() Kristian> before we set the wakeup_subsequent_commits_running flag back to false. And Kristian> wakeup() takes a mutex, which implies a full memory barrier. Kristian> So I now removed the extra redundant locking (after adding comments explaining Kristian> why this is ok), and pushed that to 10.0-knielsen. Kristian> Let me see if I can explain why the memory barrier is needed. The potential Kristian> race (or one of them at least) is as follows: Kristian> Thread A is doing wakeup_subsequent_commits2(). Thread B is doing Kristian> unregister_wait_for_prior_commit2(). We assume B is on A's list of waiters. Thanks for spending time explaning this! This one is clear. What was not clear how the code that only goes and resets wakeup_subsequent_commits_running for all threads could cause a problem. However, things are good enough so we can leave the code according to your latest version. <cut> Kristian> Hope this helps, yes, it helped a lot. Thanks! Regards, Monty