[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 12/17] block: protect tracked_requests and flush
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 12/17] block: protect tracked_requests and flush_queue with reqs_lock |
Date: |
Thu, 4 May 2017 10:35:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 04/05/2017 09:30, Fam Zheng wrote:
>> @@ -2302,11 +2308,13 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>> current_gen = atomic_read(&bs->write_gen);
>>
>> /* Wait until any previous flushes are completed */
>> + qemu_co_mutex_lock(&bs->reqs_lock);
>> while (bs->active_flush_req) {
>> - qemu_co_queue_wait(&bs->flush_queue, NULL);
>> + qemu_co_queue_wait(&bs->flush_queue, &bs->reqs_lock);
>> }
>>
>> bs->active_flush_req = true;
>> + qemu_co_mutex_unlock(&bs->reqs_lock);
>>
>> /* Write back all layers by calling one driver function */
>> if (bs->drv->bdrv_co_flush) {
>> @@ -2328,10 +2336,14 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>> goto flush_parent;
>> }
>>
>> - /* Check if we really need to flush anything */
>> + /* Check if we really need to flush anything
>> + * TODO: use int and atomic access */
>> + qemu_co_mutex_lock(&bs->reqs_lock);
>> if (bs->flushed_gen == current_gen) {
>
> Should the atomic reading of current_gen be moved down here, to avoid TOCTOU?
No, but another change is needed; current_gen needs to be read under the
lock, to ensure that flushes are processed in increasing generation order.
In addition, this access to flushed_gen does not need the lock;
bs->active_flush_req itself acts as a "lock" for bs->flushed_gen, only
the coroutine that set it to true will read it or write it. I'll adjust
this patch accordingly.
Paolo