|
From: | Emanuele Giuseppe Esposito |
Subject: | Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields |
Date: | Tue, 25 May 2021 12:18:06 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
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 prettystraightforward, 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
[Prev in Thread] | Current Thread | [Next in Thread] |