Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

[Emanuele Giuseppe Esposito]

On 21/05/2021 19:10, Vladimir Sementsov-Ogievskiy wrote:
> 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote:
> > With tasks and calls lock protecting all State fields,
> > .method is the last BlockCopyState field left unprotected.
> > Set it as atomic.
> >
> > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>
> OK, in 06 some things are out of coroutine. Here could we just reuse mutex?
>
> I believe, that we don't need any kind of protection for .method inside 
> block_copy_state_new(), as it's just a creation and initialization of 
> new structure.

I agree here, will remove the atomic_set in block_copy_state_new.
> And other things are called from coroutines. So, seems no reasons for 
> additional atomic access logic?

But... why should I use a mutex? I think the .method usage is pretty
straightforward, adding a lock (which one, tasks_lock? does not seem 
appropriate) would just cover also functions that do not need it, since 
the field is modified in if-else statements (see block_copy_do_copy).
It looks to me that an atomic here won't hurt, and it's pretty 
straightforward to understand.

Thank you,
Emanuele