On 22/06/2021 12:39, Vladimir Sementsov-Ogievskiy wrote:
22.06.2021 13:20, Paolo Bonzini wrote:
On 22/06/21 11:36, Vladimir Sementsov-Ogievskiy wrote:
It does. If it returns true, you still want the load of finished to
happen before the reads that follow.
Hmm.. The worst case if we use just qatomic_read is that assertion
will not crash when it actually should. That doesn't break the logic.
But that's not good anyway.
OK, I agree, let's keep it.
You can also have a finished job, but get a stale value for
error_is_read or ret. The issue is not in getting the stale value per
se, but in block_copy_call_status's caller not expecting it.
(I understand you agree, but I guess it can be interesting to learn
about this too).
Hmm. So, do you mean that we can read ret and error_is_read ONLY after
explicitly doing load_acquire(finished) and checking that it's true?
That means that we must do it not in assertion (to not be compiled out):
bool finished = load_acquire()
assert(finished);
... read reat and error_is_read ...
If I understand correctly, this was what I was trying to say before:
maybe it's better that we make sure that @finished is set before reading
@ret and @error_is_read. And because assert can be disabled, we can do
like you wrote above.
Anyways, let's wait Paolo's answer for this. Once this is ready, I will
send v5.
Emanuele
