[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 07/10] qcow2: qcow2_co_preadv: improve lockin
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v5 07/10] qcow2: qcow2_co_preadv: improve locking |
Date: |
Mon, 29 Apr 2019 19:04:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 29.04.19 18:37, Max Reitz wrote:
> On 02.04.19 17:37, Vladimir Sementsov-Ogievskiy wrote:
>> Background: decryption will be done in threads, to take benefit of it,
>> we should move it out of the lock first.
>
> ...which is safe after your commit c972fa123c73501b4, I presume.
>
> (At first glance, the patched looked a bit weird to me because it
> doesn't give a reason why dropping the lock around
> qcrypto_block_decrypt() would be OK.)
On second thought, I guess the actual reason it's safe is because the
crypto code never yields.
Max
>> But let's go further: it turns out, that for locking around switch
>> cases we have only two variants: when we just do memset(0) not
>> releasing the lock (it is useless) and when we actually can handle the
>> whole case out of the lock. So, refactor the whole thing to reduce
>> locked code region and make it clean.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
>> block/qcow2.c | 46 ++++++++++++++++++++++------------------------
>> 1 file changed, 22 insertions(+), 24 deletions(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 46e8e39da5..fcf92a7eb6 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -1983,6 +1983,7 @@ static coroutine_fn int
>> qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
>>
>> ret = qcow2_get_cluster_offset(bs, offset, &cur_bytes,
>> &cluster_offset);
>
> Isn't this the only function in the loop that actually needs the lock?
> Wouldn't it make more sense to just take it around this call?
>
> Max
>
>> if (ret < 0) {
>> + qemu_co_mutex_unlock(&s->lock);
>> goto fail;
>> }
>>
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v5 00/10] qcow2: encryption threads, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-block] [PATCH v5 09/10] qcow2: bdrv_co_pwritev: move encryption code out of the lock, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-block] [PATCH v5 06/10] qcow2-threads: split out generic path, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-block] [PATCH v5 02/10] qcow2.h: add missing include, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-block] [PATCH v5 03/10] qcow2: add separate file for threaded data processing functions, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-block] [PATCH v5 01/10] tests/perf: Test qemu-img convert from raw to encrypted qcow2, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-block] [PATCH v5 10/10] qcow2: do encryption in threads, Vladimir Sementsov-Ogievskiy, 2019/04/02