Message ID | 20210518094058.25952-6-eesposit@redhat.com |
---|---|
State | New |
Headers | show |
Series | block-copy: make helper APIs thread safe | expand |
18.05.2021 12:40, Emanuele Giuseppe Esposito wrote: > co-shared-resource is currently not thread-safe, as also reported > in co-shared-resource.h. Add a QemuMutex because co_try_get_from_shres > can also be invoked from non-coroutine context. > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > --- > include/qemu/co-shared-resource.h | 4 +--- > util/qemu-co-shared-resource.c | 27 ++++++++++++++++++++++----- > 2 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/include/qemu/co-shared-resource.h b/include/qemu/co-shared-resource.h > index 4e4503004c..78ca5850f8 100644 > --- a/include/qemu/co-shared-resource.h > +++ b/include/qemu/co-shared-resource.h > @@ -26,15 +26,13 @@ > #ifndef QEMU_CO_SHARED_RESOURCE_H > #define QEMU_CO_SHARED_RESOURCE_H > > - > +/* Accesses to co-shared-resource API are thread-safe */ > typedef struct SharedResource SharedResource; > > /* > * Create SharedResource structure > * > * @total: total amount of some resource to be shared between clients > - * > - * Note: this API is not thread-safe. > */ > SharedResource *shres_create(uint64_t total); > > diff --git a/util/qemu-co-shared-resource.c b/util/qemu-co-shared-resource.c > index 1c83cd9d29..bb875a86be 100644 > --- a/util/qemu-co-shared-resource.c > +++ b/util/qemu-co-shared-resource.c > @@ -28,10 +28,13 @@ > #include "qemu/co-shared-resource.h" > > struct SharedResource { > - uint64_t total; > - uint64_t available; > + uint64_t total; /* Set in shres_create() and not changed anymore */ > > + /* State fields protected by lock */ > + uint64_t available; > CoQueue queue; > + > + QemuMutex lock; > }; > > SharedResource *shres_create(uint64_t total) > @@ -40,6 +43,7 @@ SharedResource *shres_create(uint64_t total) > > s->total = s->available = total; > qemu_co_queue_init(&s->queue); > + qemu_mutex_init(&s->lock); > > return s; > } > @@ -47,10 +51,12 @@ SharedResource *shres_create(uint64_t total) > void shres_destroy(SharedResource *s) > { > assert(s->available == s->total); > + qemu_mutex_destroy(&s->lock); > g_free(s); > } > > -bool co_try_get_from_shres(SharedResource *s, uint64_t n) > +/* Called with lock held. */ > +static bool co_try_get_from_shres_locked(SharedResource *s, uint64_t n) > { > if (s->available >= n) { > s->available -= n; > @@ -60,16 +66,27 @@ bool co_try_get_from_shres(SharedResource *s, uint64_t n) > return false; > } > > +bool co_try_get_from_shres(SharedResource *s, uint64_t n) > +{ > + bool res; > + QEMU_LOCK_GUARD(&s->lock); > + res = co_try_get_from_shres_locked(s, n); > + > + return res; > +} it can be two lines: QEMU_LOCK_GUARD(&s->lock); return co_try_get_from_shres_locked(s, n); > + > void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n) > { > assert(n <= s->total); > - while (!co_try_get_from_shres(s, n)) { > - qemu_co_queue_wait(&s->queue, NULL); > + QEMU_LOCK_GUARD(&s->lock); > + while (!co_try_get_from_shres_locked(s, n)) { > + qemu_co_queue_wait(&s->queue, &s->lock); > } > } > > void coroutine_fn co_put_to_shres(SharedResource *s, uint64_t n) > { > + QEMU_LOCK_GUARD(&s->lock); > assert(s->total - s->available >= n); > s->available += n; > qemu_co_queue_restart_all(&s->queue); > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
diff --git a/include/qemu/co-shared-resource.h b/include/qemu/co-shared-resource.h index 4e4503004c..78ca5850f8 100644 --- a/include/qemu/co-shared-resource.h +++ b/include/qemu/co-shared-resource.h @@ -26,15 +26,13 @@ #ifndef QEMU_CO_SHARED_RESOURCE_H #define QEMU_CO_SHARED_RESOURCE_H - +/* Accesses to co-shared-resource API are thread-safe */ typedef struct SharedResource SharedResource; /* * Create SharedResource structure * * @total: total amount of some resource to be shared between clients - * - * Note: this API is not thread-safe. */ SharedResource *shres_create(uint64_t total); diff --git a/util/qemu-co-shared-resource.c b/util/qemu-co-shared-resource.c index 1c83cd9d29..bb875a86be 100644 --- a/util/qemu-co-shared-resource.c +++ b/util/qemu-co-shared-resource.c @@ -28,10 +28,13 @@ #include "qemu/co-shared-resource.h" struct SharedResource { - uint64_t total; - uint64_t available; + uint64_t total; /* Set in shres_create() and not changed anymore */ + /* State fields protected by lock */ + uint64_t available; CoQueue queue; + + QemuMutex lock; }; SharedResource *shres_create(uint64_t total) @@ -40,6 +43,7 @@ SharedResource *shres_create(uint64_t total) s->total = s->available = total; qemu_co_queue_init(&s->queue); + qemu_mutex_init(&s->lock); return s; } @@ -47,10 +51,12 @@ SharedResource *shres_create(uint64_t total) void shres_destroy(SharedResource *s) { assert(s->available == s->total); + qemu_mutex_destroy(&s->lock); g_free(s); } -bool co_try_get_from_shres(SharedResource *s, uint64_t n) +/* Called with lock held. */ +static bool co_try_get_from_shres_locked(SharedResource *s, uint64_t n) { if (s->available >= n) { s->available -= n; @@ -60,16 +66,27 @@ bool co_try_get_from_shres(SharedResource *s, uint64_t n) return false; } +bool co_try_get_from_shres(SharedResource *s, uint64_t n) +{ + bool res; + QEMU_LOCK_GUARD(&s->lock); + res = co_try_get_from_shres_locked(s, n); + + return res; +} + void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n) { assert(n <= s->total); - while (!co_try_get_from_shres(s, n)) { - qemu_co_queue_wait(&s->queue, NULL); + QEMU_LOCK_GUARD(&s->lock); + while (!co_try_get_from_shres_locked(s, n)) { + qemu_co_queue_wait(&s->queue, &s->lock); } } void coroutine_fn co_put_to_shres(SharedResource *s, uint64_t n) { + QEMU_LOCK_GUARD(&s->lock); assert(s->total - s->available >= n); s->available += n; qemu_co_queue_restart_all(&s->queue);
co-shared-resource is currently not thread-safe, as also reported in co-shared-resource.h. Add a QemuMutex because co_try_get_from_shres can also be invoked from non-coroutine context. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- include/qemu/co-shared-resource.h | 4 +--- util/qemu-co-shared-resource.c | 27 ++++++++++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-)