mbox series

[0/6] block-copy: make helper APIs thread safe

Message ID 20210510085941.22769-1-eesposit@redhat.com (mailing list archive)
Headers show
Series block-copy: make helper APIs thread safe | expand

Message

Emanuele Giuseppe Esposito May 10, 2021, 8:59 a.m. UTC
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 the global
AioContexlock in block-copy, by introducing smaller granularity
locks thus on making the block layer thread safe. 

This serie depends on Paolo's coroutine_sleep API.

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 (they are also based on the first
ratelimit patch sent by Paolo), 4 covers progressmeter,
5 co-shared-resources and 6 aiopool.

Based-on: <20210503112550.478521-1-pbonzini@redhat.com> [coroutine_sleep]
Based-on: <20210413125533.217440-1-pbonzini@redhat.com> [ratelimit]
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
v1 -> v2:
* More field categorized as IN/State/OUT in the various struct
* Fix a couple of places where I missed locks [Vladimir, Paolo]

Emanuele Giuseppe Esposito (3):
  progressmeter: protect with a mutex
  co-shared-resource: protect with a mutex
  aiopool: 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/aio_task.c               | 63 ++++++++++++++++++++--------------
 block/block-copy.c             | 28 ++++++---------
 blockjob.c                     | 45 +++++++++++++++---------
 include/block/aio_task.h       |  2 +-
 include/qemu/progress_meter.h  | 31 +++++++++++++++++
 include/qemu/ratelimit.h       | 12 +++++--
 job-qmp.c                      |  8 +++--
 job.c                          |  3 ++
 qemu-img.c                     |  9 +++--
 util/qemu-co-shared-resource.c | 26 +++++++++++---
 10 files changed, 155 insertions(+), 72 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy May 10, 2021, 10:55 a.m. UTC | #1
10.05.2021 11:59, 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 the global
> AioContexlock in block-copy, by introducing smaller granularity
> locks thus on making the block layer thread safe.
> 
> This serie depends on Paolo's coroutine_sleep API.
> 
> 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 (they are also based on the first
> ratelimit patch sent by Paolo), 4 covers progressmeter,
> 5 co-shared-resources and 6 aiopool.
> 
> Based-on: <20210503112550.478521-1-pbonzini@redhat.com> [coroutine_sleep]
> Based-on: <20210413125533.217440-1-pbonzini@redhat.com> [ratelimit]

Seems, patchew failed to parse your "Based-on" tags: https://patchew.org/QEMU/20210510085941.22769-1-eesposit@redhat.com/
(and tries to apply series on master)
I think, it's because you placed "[...]" comments on the same lines.

Interesting, will patchew see, if I just duplicate tags here:

Based-on: <20210503112550.478521-1-pbonzini@redhat.com>
Based-on: <20210413125533.217440-1-pbonzini@redhat.com>


> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> v1 -> v2:
> * More field categorized as IN/State/OUT in the various struct
> * Fix a couple of places where I missed locks [Vladimir, Paolo]
> 
> Emanuele Giuseppe Esposito (3):
>    progressmeter: protect with a mutex
>    co-shared-resource: protect with a mutex
>    aiopool: 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/aio_task.c               | 63 ++++++++++++++++++++--------------
>   block/block-copy.c             | 28 ++++++---------
>   blockjob.c                     | 45 +++++++++++++++---------
>   include/block/aio_task.h       |  2 +-
>   include/qemu/progress_meter.h  | 31 +++++++++++++++++
>   include/qemu/ratelimit.h       | 12 +++++--
>   job-qmp.c                      |  8 +++--
>   job.c                          |  3 ++
>   qemu-img.c                     |  9 +++--
>   util/qemu-co-shared-resource.c | 26 +++++++++++---
>   10 files changed, 155 insertions(+), 72 deletions(-)
>
Stefan Hajnoczi May 12, 2021, 2:24 p.m. UTC | #2
On Mon, May 10, 2021 at 10:59:35AM +0200, 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 the global
> AioContexlock in block-copy, by introducing smaller granularity
> locks thus on making the block layer thread safe. 

I'm not sure "global" is accurate. Block jobs can deal with arbitrary
AioContexts, not just a global AioContext. The lock is per-AioContext
rather than global. Or did I miss something?