Re: [PATCH v4 6/6] block-copy: atomic .cancelled and .finished fields in BlockCopyCallState

[Vladimir Sementsov-Ogievskiy]

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 ...


--
Best regards,
Vladimir