|
From: | Paolo Bonzini |
Subject: | Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields |
Date: | Wed, 26 May 2021 16:48:44 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 25/05/21 13:00, Vladimir Sementsov-Ogievskiy wrote:
Hmm. OK, let me think:First look at block_copy_do_copy(). It's called only from block_copy_task_entry. block_copy_task_entry() has mutex-critical-section anyway around handling return value. That means that we can simply move s->method modification logic to this already existing critical section.Next, block_copy_chunk_size() is called only from block_copy_task_create(), where we should have critical section too.
block_copy_do_copy would have to release the mutex around the reads/writes (including the bdrv_co_pwrite_zeroes that has nothing to do with s->method).
There's also the "goto out" if the copy-range operation succeeds, which makes things a bit more complicated. The goto suggests using QEMU_WITH_LOCK_GUARD, but that doesn't work too well either, because there are two accesses (one before the bdrv_co_copy_range and one after).
So I understand why you want to avoid atomics and I agree that they are more complicated than the other solutions, on the other hand I think this patch is the simplest code.
Paolo
So, no reason for atomics, as we already have critical sections.
[Prev in Thread] | Current Thread | [Next in Thread] |