[PATCH v3 0/5] block-copy: make helper APIs thread safe

Emanuele Giuseppe Esposito posted 5 patches 2 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210614081130.22134-1-eesposit@redhat.com
Test checkpatch passed
Maintainers: John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>, Max Reitz <mreitz@redhat.com>
block/block-copy.c                | 28 ++++++--------
block/meson.build                 |  1 +
block/progress_meter.c            | 64 +++++++++++++++++++++++++++++++
blockjob.c                        | 46 +++++++++++++---------
include/qemu/co-shared-resource.h |  4 +-
include/qemu/progress_meter.h     | 34 ++++++++--------
include/qemu/ratelimit.h          | 12 +++++-
job-qmp.c                         |  8 +++-
job.c                             |  3 ++
qemu-img.c                        |  9 +++--
util/qemu-co-shared-resource.c    | 24 +++++++++---
11 files changed, 168 insertions(+), 65 deletions(-)
create mode 100644 block/progress_meter.c
[PATCH v3 0/5] block-copy: make helper APIs thread safe
Posted by Emanuele Giuseppe Esposito 2 years, 10 months ago
This serie of patches bring thread safety to the smaller APIs used by
block-copy, namely ratelimit, progressmeter, co-shared-resource
and aiotask.
The end goal is to reduce the usage of AioContexlock in block-copy,
by introducing smaller granularity locks thus on making the block layer
thread safe. 

What's missing for block-copy to be fully thread-safe is fixing
the CoSleep API to allow cross-thread sleep and wakeup.
Paolo is working on it and will post the patches once his new
CoSleep API is accepted.

Patches 1-3 work on ratelimit, 4 covers progressmeter and
5 co-shared-resources.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
v3:
* Rebase on current master (had conflicts in block-copy), remove based-on in
  cover letter

Emanuele Giuseppe Esposito (2):
  progressmeter: protect with a mutex
  co-shared-resource: protect with a mutex

Paolo Bonzini (3):
  ratelimit: treat zero speed as unlimited
  block-copy: let ratelimit handle a speed of 0
  blockjob: let ratelimit handle a speed of 0

 block/block-copy.c                | 28 ++++++--------
 block/meson.build                 |  1 +
 block/progress_meter.c            | 64 +++++++++++++++++++++++++++++++
 blockjob.c                        | 46 +++++++++++++---------
 include/qemu/co-shared-resource.h |  4 +-
 include/qemu/progress_meter.h     | 34 ++++++++--------
 include/qemu/ratelimit.h          | 12 +++++-
 job-qmp.c                         |  8 +++-
 job.c                             |  3 ++
 qemu-img.c                        |  9 +++--
 util/qemu-co-shared-resource.c    | 24 +++++++++---
 11 files changed, 168 insertions(+), 65 deletions(-)
 create mode 100644 block/progress_meter.c

-- 
2.31.1


Re: [PATCH v3 0/5] block-copy: make helper APIs thread safe
Posted by Emanuele Giuseppe Esposito 2 years, 10 months ago

On 14/06/2021 10:11, Emanuele Giuseppe Esposito wrote:
> This serie of patches bring thread safety to the smaller APIs used by
> block-copy, namely ratelimit, progressmeter, co-shared-resource
> and aiotask.
> The end goal is to reduce the usage of AioContexlock in block-copy,
> by introducing smaller granularity locks thus on making the block layer
> thread safe.
> 
> What's missing for block-copy to be fully thread-safe is fixing
> the CoSleep API to allow cross-thread sleep and wakeup.
> Paolo is working on it and will post the patches once his new
> CoSleep API is accepted.
> 
> Patches 1-3 work on ratelimit, 4 covers progressmeter and
> 5 co-shared-resources.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> v3:
> * Rebase on current master (had conflicts in block-copy), remove based-on in
>    cover letter

Hi Kevin & Max,

I think this series ha been reviewed and I just rebased it to current 
master. Can you give it a look and let me know if it can be merged?

Thank you,
Emanuele

> 
> Emanuele Giuseppe Esposito (2):
>    progressmeter: protect with a mutex
>    co-shared-resource: protect with a mutex
> 
> Paolo Bonzini (3):
>    ratelimit: treat zero speed as unlimited
>    block-copy: let ratelimit handle a speed of 0
>    blockjob: let ratelimit handle a speed of 0
> 
>   block/block-copy.c                | 28 ++++++--------
>   block/meson.build                 |  1 +
>   block/progress_meter.c            | 64 +++++++++++++++++++++++++++++++
>   blockjob.c                        | 46 +++++++++++++---------
>   include/qemu/co-shared-resource.h |  4 +-
>   include/qemu/progress_meter.h     | 34 ++++++++--------
>   include/qemu/ratelimit.h          | 12 +++++-
>   job-qmp.c                         |  8 +++-
>   job.c                             |  3 ++
>   qemu-img.c                        |  9 +++--
>   util/qemu-co-shared-resource.c    | 24 +++++++++---
>   11 files changed, 168 insertions(+), 65 deletions(-)
>   create mode 100644 block/progress_meter.c
> 


Re: [PATCH v3 0/5] block-copy: make helper APIs thread safe
Posted by Vladimir Sementsov-Ogievskiy 2 years, 10 months ago
14.06.2021 11:17, Emanuele Giuseppe Esposito wrote:
> 
> 
> On 14/06/2021 10:11, Emanuele Giuseppe Esposito wrote:
>> This serie of patches bring thread safety to the smaller APIs used by
>> block-copy, namely ratelimit, progressmeter, co-shared-resource
>> and aiotask.
>> The end goal is to reduce the usage of AioContexlock in block-copy,
>> by introducing smaller granularity locks thus on making the block layer
>> thread safe.
>>
>> What's missing for block-copy to be fully thread-safe is fixing
>> the CoSleep API to allow cross-thread sleep and wakeup.
>> Paolo is working on it and will post the patches once his new
>> CoSleep API is accepted.
>>
>> Patches 1-3 work on ratelimit, 4 covers progressmeter and
>> 5 co-shared-resources.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>> v3:
>> * Rebase on current master (had conflicts in block-copy), remove based-on in
>>    cover letter
> 
> Hi Kevin & Max,
> 
> I think this series ha been reviewed and I just rebased it to current master. Can you give it a look and let me know if it can be merged?
> 
> Thank you,
> Emanuele


I think, I can queue it myself as a block-job series. ratelimit and progressmeter are not mentioned in Block Jobs sections of MAINTAINERS, but actually these APIs used only by block-jobs.

I remember, Stefan had a complain against patch 5 and against general design of adding mutex to every structure.. Stefan, what do you think now? Paolo, is this v3 OK for you?

If everybody silent, I don't see a reason to slow down these series anymore and will make a pull request on Tuesday.

-- 
Best regards,
Vladimir

Re: [PATCH v3 0/5] block-copy: make helper APIs thread safe
Posted by Vladimir Sementsov-Ogievskiy 2 years, 10 months ago
19.06.2021 15:21, Vladimir Sementsov-Ogievskiy wrote:
> 14.06.2021 11:17, Emanuele Giuseppe Esposito wrote:
>>
>>
>> On 14/06/2021 10:11, Emanuele Giuseppe Esposito wrote:
>>> This serie of patches bring thread safety to the smaller APIs used by
>>> block-copy, namely ratelimit, progressmeter, co-shared-resource
>>> and aiotask.
>>> The end goal is to reduce the usage of AioContexlock in block-copy,
>>> by introducing smaller granularity locks thus on making the block layer
>>> thread safe.
>>>
>>> What's missing for block-copy to be fully thread-safe is fixing
>>> the CoSleep API to allow cross-thread sleep and wakeup.
>>> Paolo is working on it and will post the patches once his new
>>> CoSleep API is accepted.
>>>
>>> Patches 1-3 work on ratelimit, 4 covers progressmeter and
>>> 5 co-shared-resources.
>>>
>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>> ---
>>> v3:
>>> * Rebase on current master (had conflicts in block-copy), remove based-on in
>>>    cover letter
>>
>> Hi Kevin & Max,
>>
>> I think this series ha been reviewed and I just rebased it to current master. Can you give it a look and let me know if it can be merged?
>>
>> Thank you,
>> Emanuele
> 
> 
> I think, I can queue it myself as a block-job series. ratelimit and progressmeter are not mentioned in Block Jobs sections of MAINTAINERS, but actually these APIs used only by block-jobs.
> 
> I remember, Stefan had a complain against patch 5 and against general design of adding mutex to every structure.. Stefan, what do you think now? Paolo, is this v3 OK for you?
> 
> If everybody silent, I don't see a reason to slow down these series anymore and will make a pull request on Tuesday.
> 

Hmm, actually, I'll wait for final version of "[PATCH v4 0/6] block-copy: protect block-copy internal structures" which seems to be close, to pull them together.


-- 
Best regards,
Vladimir

Re: [PATCH v3 0/5] block-copy: make helper APIs thread safe
Posted by Vladimir Sementsov-Ogievskiy 2 years, 10 months ago
14.06.2021 11:11, Emanuele Giuseppe Esposito wrote:
> This serie of patches bring thread safety to the smaller APIs used by
> block-copy, namely ratelimit, progressmeter, co-shared-resource
> and aiotask.
> The end goal is to reduce the usage of AioContexlock in block-copy,
> by introducing smaller granularity locks thus on making the block layer
> thread safe.
> 
> What's missing for block-copy to be fully thread-safe is fixing
> the CoSleep API to allow cross-thread sleep and wakeup.
> Paolo is working on it and will post the patches once his new
> CoSleep API is accepted.
> 
> Patches 1-3 work on ratelimit, 4 covers progressmeter and
> 5 co-shared-resources.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Thanks, applied to the jobs branch

-- 
Best regards,
Vladimir