22.06.2021 11:15, Paolo Bonzini wrote:
On 19/06/21 22:06, Vladimir Sementsov-Ogievskiy wrote:

-    assert(call_state->finished);
+    assert(qatomic_load_acquire(&call_state->finished));

Hmm. Here qatomic_load_acquire protects nothing (assertion will crash if not 
yet finished anyway). So, caller is double sure that block-copy is finished.

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.

Otherwise I agree with your remarks.

Paolo

Also it's misleading: if we think that it do some protection, we are doing 
wrong thing: assertions may be simply compiled out, we can't rely on statements 
inside assert() to be executed.

So, let's use simple qatomic_read here too.

      if (error_is_read) {
          *error_is_read = call_state->error_is_read;
      }



--
Best regards,
Vladimir

Reply via email to