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

[Paolo Bonzini]

On 21/05/21 17:56, Vladimir Sementsov-Ogievskiy wrote:
> 21.05.2021 18:02, Paolo Bonzini wrote:
> > On 20/05/21 17:34, Vladimir Sementsov-Ogievskiy wrote:
> > > > By adding acquire/release pairs, we ensure that .ret and .error_is_read
> > > > fields are written by block_copy_dirty_clusters before .finished is 
> > > > true.
> > >
> > > As I already said, please, can we live with one mutex for now? 
> > > finished, ret, error_is_read, all these variables are changing 
> > > rarely. I doubt that performance is improved by these atomic 
> > > operations. But complexity of the architecture increases exponentially.
> >
> > The problem is that these are used outside coroutines. 
> > load-acquire/store-release is the simplest way to handle a "finished" 
> > flag really.
>
> Related, maybe we can support CoMutex outside of coroutine context?
>
> Create a coroutine, which will lock the mutex and unlock it for us... Or 
> something like this.. It will help the task of making everything 
> thread-safe a lot

No, it's not possible because the coroutine will yield to the caller if 
the mutex is contended, but the caller will not be able to use the data 
that is protected by the mutex.

There is no reason to have stuff like block_copy_call_status be a 
coroutine_fn.  Even if it's only called from a coroutine today I'd 
rather have the code more future proof.

Paolo