maria-developers team mailing list archive

[Michael Widenius <monty@xxxxxxxxxxxx>]

Hi!

>>>>> "Kristian" == Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx> 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