By the time I see multiple atomics operations stacked like this, I start to wonder: isn't there a way to guarantee this by simply using locking APIs? We already call VT_LOCK()/VT_UNLOCK(), it seems. Maybe this is just missing in one of the other places?

In D9828#202716, @ed wrote:

By the time I see multiple atomics operations stacked like this, I start to wonder: isn't there a way to guarantee this by simply using locking APIs? We already call VT_LOCK()/VT_UNLOCK(), it seems. Maybe this is just missing in one of the other places?

I just opened bug 217408 for this. I have more analysis there.

In short, the other end of the race may hold a spin lock when calling vt_resume_flush_timer(), so it can't acquire the vd lock (a regular mutex) at that point.

We don't actually need atomics here. But, we do care about order of operations, so I used them for the memory barriers they provide. We need to ensure that vd_timer_armed is 1 before we set VDF_ASYNC. We may be able to do away with the second atomic. I guess it isn't really critical VDF_ASYNC is set before we call vt_timer(). (And, even though it might be somewhat unusual, vt_timer() may well fire before callout_reset() returns in the case the thread gets pre-empted. But, as I said, it may actually not be critical that VDF_ASYNC is set before vt_timer() runs. So, maybe that was an overreaction on my part.)

I had a long discussion with @ed in the bug. You can see that for context. I think the summary is that I still think we should commit this code, even if @ed thinks a more robust fix requires a larger change. I would like to do that, but do also support @ed making a further bug fix in the future.