[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 07/10] qcow2: qcow2_co_preadv: improve lockin
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-devel] [PATCH v5 07/10] qcow2: qcow2_co_preadv: improve locking |
Date: |
Tue, 30 Apr 2019 09:44:35 +0000 |
30.04.2019 11:38, Vladimir Sementsov-Ogievskiy wrote:
> 29.04.2019 19: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.)
>>
>>> 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?
>>
>
> Hmm, looks correct, I'll resend.
>
>
Or not, actually, we may have several qcow2_get_data_offset calls under one
lock,
if clusters are different kinds of ZERO. So, I think better to keep it as is
for now.
--
Best regards,
Vladimir
- [Qemu-devel] [PATCH v5 03/10] qcow2: add separate file for threaded data processing functions, (continued)
- [Qemu-devel] [PATCH v5 03/10] qcow2: add separate file for threaded data processing functions, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-devel] [PATCH v5 05/10] qcow2-threads: qcow2_co_do_compress: protect queuing by mutex, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-devel] [PATCH v5 04/10] qcow2-threads: use thread_pool_submit_co, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-devel] [PATCH v5 09/10] qcow2: bdrv_co_pwritev: move encryption code out of the lock, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-devel] [PATCH v5 06/10] qcow2-threads: split out generic path, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-devel] [PATCH v5 02/10] qcow2.h: add missing include, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-devel] [PATCH v5 07/10] qcow2: qcow2_co_preadv: improve locking, Vladimir Sementsov-Ogievskiy, 2019/04/02
- [Qemu-devel] [PATCH v5 10/10] qcow2: do encryption in threads, Vladimir Sementsov-Ogievskiy, 2019/04/02
- Re: [Qemu-devel] [PATCH v5 00/10] qcow2: encryption threads, Vladimir Sementsov-Ogievskiy, 2019/04/16
- Re: [Qemu-devel] [PATCH v5 00/10] qcow2: encryption threads, Max Reitz, 2019/04/28
- Re: [Qemu-devel] [PATCH v5 00/10] qcow2: encryption threads, Max Reitz, 2019/04/29