[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 6/9] qcow2: qcow2_co_preadv: improve locking
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-block] [PATCH v3 6/9] qcow2: qcow2_co_preadv: improve locking |
Date: |
Wed, 16 Jan 2019 14:53:13 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Tue 08 Jan 2019 06:06:52 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> @@ -1891,39 +1892,38 @@ static coroutine_fn int
> qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
> qemu_iovec_reset(&hd_qiov);
> qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
>
> + if (ret == QCOW2_CLUSTER_ZERO_PLAIN ||
> + ret == QCOW2_CLUSTER_ZERO_ALLOC ||
> + (ret == QCOW2_CLUSTER_UNALLOCATED && !bs->backing))
> + {
> + /* No sense in releasing the lock */
> +
> + qemu_iovec_memset(&hd_qiov, 0, 0, cur_bytes);
> +
> + bytes -= cur_bytes;
> + offset += cur_bytes;
> + bytes_done += cur_bytes;
> + continue;
> + }
Maybe that was just me, but I got confused by the "No sense in releasing
the lock" thinking that it referred to the memset() call.
What you're trying to say is that it doesn't make sense to release the
lock while you only get zero / unallocated clusters only to lock it
again at the next iteration of the loop.
I think that comment may make sense when you think about the code
changes that you are doing, but when you read the actual resulting code
it's fairly obvious that you don't need to touch the lock there.
So I'd rather remove that comment.
Else the patch looks good.
Reviewed-by: Alberto Garcia <address@hidden>
Berto
- [Qemu-block] [PATCH v3 0/9] qcow2: encryption threads, Vladimir Sementsov-Ogievskiy, 2019/01/08
- [Qemu-block] [PATCH v3 3/9] qcow2-threads: use thread_pool_submit_co, Vladimir Sementsov-Ogievskiy, 2019/01/08
- [Qemu-block] [PATCH v3 6/9] qcow2: qcow2_co_preadv: improve locking, Vladimir Sementsov-Ogievskiy, 2019/01/08
- Re: [Qemu-block] [PATCH v3 6/9] qcow2: qcow2_co_preadv: improve locking,
Alberto Garcia <=
- [Qemu-block] [PATCH v3 7/9] qcow2: qcow2_co_preadv: skip using hd_qiov when possible, Vladimir Sementsov-Ogievskiy, 2019/01/08
- [Qemu-block] [PATCH v3 5/9] qcow2-threads: split out generic path, Vladimir Sementsov-Ogievskiy, 2019/01/08
- [Qemu-block] [PATCH v3 4/9] qcow2-threads: qcow2_co_do_compress: protect queuing by mutex, Vladimir Sementsov-Ogievskiy, 2019/01/08
- [Qemu-block] [PATCH v3 1/9] qcow2.h: add missing include, Vladimir Sementsov-Ogievskiy, 2019/01/08