* [PATCH 1/4] block/accounting.c: Use lock guard macros
2020-11-09 15:43 [PATCH 0/4] Use lock guard macros in block Gan Qixin
@ 2020-11-09 15:43 ` Gan Qixin
2020-11-09 15:43 ` [PATCH 2/4] block/curl.c: " Gan Qixin
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Gan Qixin @ 2020-11-09 15:43 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: kwolf, Gan Qixin, zhang.zhanghailiang, mreitz, dnbrdsky, stefanha,
kuhn.chenqun, pbonzini
Replace manual lock()/unlock() calls with lock guard macros
(QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/accounting.c.
Signed-off-by: Gan Qixin <ganqixin@huawei.com>
---
block/accounting.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/block/accounting.c b/block/accounting.c
index 8d41c8a83a..2030851d79 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -199,29 +199,27 @@ static void block_account_one_io(BlockAcctStats *stats, BlockAcctCookie *cookie,
return;
}
- qemu_mutex_lock(&stats->lock);
-
- if (failed) {
- stats->failed_ops[cookie->type]++;
- } else {
- stats->nr_bytes[cookie->type] += cookie->bytes;
- stats->nr_ops[cookie->type]++;
- }
+ WITH_QEMU_LOCK_GUARD(&stats->lock) {
+ if (failed) {
+ stats->failed_ops[cookie->type]++;
+ } else {
+ stats->nr_bytes[cookie->type] += cookie->bytes;
+ stats->nr_ops[cookie->type]++;
+ }
- block_latency_histogram_account(&stats->latency_histogram[cookie->type],
- latency_ns);
+ block_latency_histogram_account(&stats->latency_histogram[cookie->type],
+ latency_ns);
- if (!failed || stats->account_failed) {
- stats->total_time_ns[cookie->type] += latency_ns;
- stats->last_access_time_ns = time_ns;
+ if (!failed || stats->account_failed) {
+ stats->total_time_ns[cookie->type] += latency_ns;
+ stats->last_access_time_ns = time_ns;
- QSLIST_FOREACH(s, &stats->intervals, entries) {
- timed_average_account(&s->latency[cookie->type], latency_ns);
+ QSLIST_FOREACH(s, &stats->intervals, entries) {
+ timed_average_account(&s->latency[cookie->type], latency_ns);
+ }
}
}
- qemu_mutex_unlock(&stats->lock);
-
cookie->type = BLOCK_ACCT_NONE;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] block/curl.c: Use lock guard macros
2020-11-09 15:43 [PATCH 0/4] Use lock guard macros in block Gan Qixin
2020-11-09 15:43 ` [PATCH 1/4] block/accounting.c: Use lock guard macros Gan Qixin
@ 2020-11-09 15:43 ` Gan Qixin
2020-11-09 15:43 ` [PATCH 3/4] block/throttle-groups.c: " Gan Qixin
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Gan Qixin @ 2020-11-09 15:43 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: kwolf, Gan Qixin, zhang.zhanghailiang, mreitz, dnbrdsky, stefanha,
kuhn.chenqun, pbonzini
Replace manual lock()/unlock() calls with lock guard macros
(QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/curl.c.
Signed-off-by: Gan Qixin <ganqixin@huawei.com>
---
block/curl.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 4f907c47be..d24a4c5897 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -564,23 +564,23 @@ static void curl_detach_aio_context(BlockDriverState *bs)
BDRVCURLState *s = bs->opaque;
int i;
- qemu_mutex_lock(&s->mutex);
- for (i = 0; i < CURL_NUM_STATES; i++) {
- if (s->states[i].in_use) {
- curl_clean_state(&s->states[i]);
+ WITH_QEMU_LOCK_GUARD(&s->mutex) {
+ for (i = 0; i < CURL_NUM_STATES; i++) {
+ if (s->states[i].in_use) {
+ curl_clean_state(&s->states[i]);
+ }
+ if (s->states[i].curl) {
+ curl_easy_cleanup(s->states[i].curl);
+ s->states[i].curl = NULL;
+ }
+ g_free(s->states[i].orig_buf);
+ s->states[i].orig_buf = NULL;
}
- if (s->states[i].curl) {
- curl_easy_cleanup(s->states[i].curl);
- s->states[i].curl = NULL;
+ if (s->multi) {
+ curl_multi_cleanup(s->multi);
+ s->multi = NULL;
}
- g_free(s->states[i].orig_buf);
- s->states[i].orig_buf = NULL;
- }
- if (s->multi) {
- curl_multi_cleanup(s->multi);
- s->multi = NULL;
}
- qemu_mutex_unlock(&s->mutex);
timer_del(&s->timer);
}
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/4] block/throttle-groups.c: Use lock guard macros
2020-11-09 15:43 [PATCH 0/4] Use lock guard macros in block Gan Qixin
2020-11-09 15:43 ` [PATCH 1/4] block/accounting.c: Use lock guard macros Gan Qixin
2020-11-09 15:43 ` [PATCH 2/4] block/curl.c: " Gan Qixin
@ 2020-11-09 15:43 ` Gan Qixin
2020-12-02 11:12 ` Kevin Wolf
2020-11-09 15:43 ` [PATCH 4/4] block/iscsi.c: " Gan Qixin
2020-12-02 9:22 ` [PATCH 0/4] Use lock guard macros in block Markus Armbruster
4 siblings, 1 reply; 10+ messages in thread
From: Gan Qixin @ 2020-11-09 15:43 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: kwolf, Gan Qixin, zhang.zhanghailiang, mreitz, dnbrdsky, stefanha,
kuhn.chenqun, pbonzini
Replace manual lock()/unlock() calls with lock guard macros
(QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/throttle-groups.c.
Signed-off-by: Gan Qixin <ganqixin@huawei.com>
---
block/throttle-groups.c | 48 ++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index e2f2813c0f..badb93a3be 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -546,7 +546,7 @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
tgm->aio_context = ctx;
qatomic_set(&tgm->restart_pending, 0);
- qemu_mutex_lock(&tg->lock);
+ QEMU_LOCK_GUARD(&tg->lock);
/* If the ThrottleGroup is new set this ThrottleGroupMember as the token */
for (i = 0; i < 2; i++) {
if (!tg->tokens[i]) {
@@ -565,8 +565,6 @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
qemu_co_mutex_init(&tgm->throttled_reqs_lock);
qemu_co_queue_init(&tgm->throttled_reqs[0]);
qemu_co_queue_init(&tgm->throttled_reqs[1]);
-
- qemu_mutex_unlock(&tg->lock);
}
/* Unregister a ThrottleGroupMember from its group, removing it from the list,
@@ -594,25 +592,25 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
/* Wait for throttle_group_restart_queue_entry() coroutines to finish */
AIO_WAIT_WHILE(tgm->aio_context, qatomic_read(&tgm->restart_pending) > 0);
- qemu_mutex_lock(&tg->lock);
- for (i = 0; i < 2; i++) {
- assert(tgm->pending_reqs[i] == 0);
- assert(qemu_co_queue_empty(&tgm->throttled_reqs[i]));
- assert(!timer_pending(tgm->throttle_timers.timers[i]));
- if (tg->tokens[i] == tgm) {
- token = throttle_group_next_tgm(tgm);
- /* Take care of the case where this is the last tgm in the group */
- if (token == tgm) {
- token = NULL;
+ WITH_QEMU_LOCK_GUARD(&tg->lock) {
+ for (i = 0; i < 2; i++) {
+ assert(tgm->pending_reqs[i] == 0);
+ assert(qemu_co_queue_empty(&tgm->throttled_reqs[i]));
+ assert(!timer_pending(tgm->throttle_timers.timers[i]));
+ if (tg->tokens[i] == tgm) {
+ token = throttle_group_next_tgm(tgm);
+ /* Take care of the case where this is the last tgm in the group */
+ if (token == tgm) {
+ token = NULL;
+ }
+ tg->tokens[i] = token;
}
- tg->tokens[i] = token;
}
- }
- /* remove the current tgm from the list */
- QLIST_REMOVE(tgm, round_robin);
- throttle_timers_destroy(&tgm->throttle_timers);
- qemu_mutex_unlock(&tg->lock);
+ /* remove the current tgm from the list */
+ QLIST_REMOVE(tgm, round_robin);
+ throttle_timers_destroy(&tgm->throttle_timers);
+ }
throttle_group_unref(&tg->ts);
tgm->throttle_state = NULL;
@@ -638,14 +636,14 @@ void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));
/* Kick off next ThrottleGroupMember, if necessary */
- qemu_mutex_lock(&tg->lock);
- for (i = 0; i < 2; i++) {
- if (timer_pending(tt->timers[i])) {
- tg->any_timer_armed[i] = false;
- schedule_next_request(tgm, i);
+ WITH_QEMU_LOCK_GUARD(&tg->lock) {
+ for (i = 0; i < 2; i++) {
+ if (timer_pending(tt->timers[i])) {
+ tg->any_timer_armed[i] = false;
+ schedule_next_request(tgm, i);
+ }
}
}
- qemu_mutex_unlock(&tg->lock);
throttle_timers_detach_aio_context(tt);
tgm->aio_context = NULL;
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] block/throttle-groups.c: Use lock guard macros
2020-11-09 15:43 ` [PATCH 3/4] block/throttle-groups.c: " Gan Qixin
@ 2020-12-02 11:12 ` Kevin Wolf
0 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2020-12-02 11:12 UTC (permalink / raw)
To: Gan Qixin
Cc: zhang.zhanghailiang, qemu-trivial, qemu-devel, mreitz, dnbrdsky,
stefanha, kuhn.chenqun, pbonzini
Am 09.11.2020 um 16:43 hat Gan Qixin geschrieben:
> Replace manual lock()/unlock() calls with lock guard macros
> (QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/throttle-groups.c.
>
> Signed-off-by: Gan Qixin <ganqixin@huawei.com>
> @@ -638,14 +636,14 @@ void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
> assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));
>
> /* Kick off next ThrottleGroupMember, if necessary */
> - qemu_mutex_lock(&tg->lock);
> - for (i = 0; i < 2; i++) {
> - if (timer_pending(tt->timers[i])) {
> - tg->any_timer_armed[i] = false;
> - schedule_next_request(tgm, i);
> + WITH_QEMU_LOCK_GUARD(&tg->lock) {
Indentation is off.
> + for (i = 0; i < 2; i++) {
> + if (timer_pending(tt->timers[i])) {
> + tg->any_timer_armed[i] = false;
> + schedule_next_request(tgm, i);
> + }
> }
> }
> - qemu_mutex_unlock(&tg->lock);
>
> throttle_timers_detach_aio_context(tt);
> tgm->aio_context = NULL;
Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] block/iscsi.c: Use lock guard macros
2020-11-09 15:43 [PATCH 0/4] Use lock guard macros in block Gan Qixin
` (2 preceding siblings ...)
2020-11-09 15:43 ` [PATCH 3/4] block/throttle-groups.c: " Gan Qixin
@ 2020-11-09 15:43 ` Gan Qixin
2020-12-02 11:11 ` Kevin Wolf
2020-12-02 9:22 ` [PATCH 0/4] Use lock guard macros in block Markus Armbruster
4 siblings, 1 reply; 10+ messages in thread
From: Gan Qixin @ 2020-11-09 15:43 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: kwolf, Gan Qixin, zhang.zhanghailiang, mreitz, dnbrdsky, stefanha,
kuhn.chenqun, pbonzini
Replace manual lock()/unlock() calls with lock guard macros
(QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/iscsi.c.
Signed-off-by: Gan Qixin <ganqixin@huawei.com>
---
block/iscsi.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index e30a7e3606..f5f657b582 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -322,7 +322,7 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
IscsiLun *iscsilun = acb->iscsilun;
- qemu_mutex_lock(&iscsilun->mutex);
+ QEMU_LOCK_GUARD(&iscsilun->mutex);
/* If it was cancelled or completed already, our work is done here */
if (acb->cancelled || acb->status != -EINPROGRESS) {
@@ -339,8 +339,6 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
iscsi_abort_task_cb, acb) < 0) {
qemu_aio_unref(acb); /* since iscsi_abort_task_cb() won't be called */
}
-
- qemu_mutex_unlock(&iscsilun->mutex);
}
static const AIOCBInfo iscsi_aiocb_info = {
@@ -375,22 +373,22 @@ static void iscsi_timed_check_events(void *opaque)
{
IscsiLun *iscsilun = opaque;
- qemu_mutex_lock(&iscsilun->mutex);
+ WITH_QEMU_LOCK_GUARD(&iscsilun->mutex) {
+ /* check for timed out requests */
+ iscsi_service(iscsilun->iscsi, 0);
- /* check for timed out requests */
- iscsi_service(iscsilun->iscsi, 0);
+ if (iscsilun->request_timed_out) {
+ iscsilun->request_timed_out = false;
+ iscsi_reconnect(iscsilun->iscsi);
+ }
- if (iscsilun->request_timed_out) {
- iscsilun->request_timed_out = false;
- iscsi_reconnect(iscsilun->iscsi);
+ /*
+ * newer versions of libiscsi may return zero events. Ensure we are
+ * able to return to service once this situation changes.
+ */
+ iscsi_set_events(iscsilun);
}
- /* newer versions of libiscsi may return zero events. Ensure we are able
- * to return to service once this situation changes. */
- iscsi_set_events(iscsilun);
-
- qemu_mutex_unlock(&iscsilun->mutex);
-
timer_mod(iscsilun->event_timer,
qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + EVENT_INTERVAL);
}
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] block/iscsi.c: Use lock guard macros
2020-11-09 15:43 ` [PATCH 4/4] block/iscsi.c: " Gan Qixin
@ 2020-12-02 11:11 ` Kevin Wolf
2020-12-03 7:04 ` ganqixin
0 siblings, 1 reply; 10+ messages in thread
From: Kevin Wolf @ 2020-12-02 11:11 UTC (permalink / raw)
To: Gan Qixin
Cc: zhang.zhanghailiang, qemu-trivial, qemu-devel, mreitz, dnbrdsky,
stefanha, kuhn.chenqun, pbonzini
Am 09.11.2020 um 16:43 hat Gan Qixin geschrieben:
> Replace manual lock()/unlock() calls with lock guard macros
> (QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/iscsi.c.
>
> Signed-off-by: Gan Qixin <ganqixin@huawei.com>
> ---
> block/iscsi.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index e30a7e3606..f5f657b582 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -322,7 +322,7 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
> IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
> IscsiLun *iscsilun = acb->iscsilun;
>
> - qemu_mutex_lock(&iscsilun->mutex);
> + QEMU_LOCK_GUARD(&iscsilun->mutex);
>
> /* If it was cancelled or completed already, our work is done here */
> if (acb->cancelled || acb->status != -EINPROGRESS) {
qemu_mutex_unlock(&iscsilun->mutex);
return;
}
I don't think this qemu_mutex_unlock() is right any more now.
Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH 4/4] block/iscsi.c: Use lock guard macros
2020-12-02 11:11 ` Kevin Wolf
@ 2020-12-03 7:04 ` ganqixin
0 siblings, 0 replies; 10+ messages in thread
From: ganqixin @ 2020-12-03 7:04 UTC (permalink / raw)
To: Kevin Wolf
Cc: Zhanghailiang, qemu-trivial@nongnu.org, qemu-devel@nongnu.org,
mreitz@redhat.com, dnbrdsky@gmail.com, stefanha@redhat.com,
Chenqun (kuhn), pbonzini@redhat.com
> -----Original Message-----
> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Sent: Wednesday, December 2, 2020 7:12 PM
> To: ganqixin <ganqixin@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-trivial@nongnu.org;
> pbonzini@redhat.com; mreitz@redhat.com; stefanha@redhat.com;
> dnbrdsky@gmail.com; Zhanghailiang <zhang.zhanghailiang@huawei.com>;
> Chenqun (kuhn) <kuhn.chenqun@huawei.com>
> Subject: Re: [PATCH 4/4] block/iscsi.c: Use lock guard macros
>
> Am 09.11.2020 um 16:43 hat Gan Qixin geschrieben:
> > Replace manual lock()/unlock() calls with lock guard macros
> > (QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/iscsi.c.
> >
> > Signed-off-by: Gan Qixin <ganqixin@huawei.com>
> > ---
> > block/iscsi.c | 28 +++++++++++++---------------
> > 1 file changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/block/iscsi.c b/block/iscsi.c index
> > e30a7e3606..f5f657b582 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
> > @@ -322,7 +322,7 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
> > IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
> > IscsiLun *iscsilun = acb->iscsilun;
> >
> > - qemu_mutex_lock(&iscsilun->mutex);
> > + QEMU_LOCK_GUARD(&iscsilun->mutex);
> >
> > /* If it was cancelled or completed already, our work is done here */
> > if (acb->cancelled || acb->status != -EINPROGRESS) {
> qemu_mutex_unlock(&iscsilun->mutex);
> return;
> }
>
> I don't think this qemu_mutex_unlock() is right any more now.
You are right, I ignored this qemu_mutex_unlock(). I will correct it and resubmit. :)
Thanks,
Gan Qixin
>
> Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] Use lock guard macros in block
2020-11-09 15:43 [PATCH 0/4] Use lock guard macros in block Gan Qixin
` (3 preceding siblings ...)
2020-11-09 15:43 ` [PATCH 4/4] block/iscsi.c: " Gan Qixin
@ 2020-12-02 9:22 ` Markus Armbruster
2020-12-02 9:33 ` Paolo Bonzini
4 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2020-12-02 9:22 UTC (permalink / raw)
To: Gan Qixin
Cc: kwolf, zhang.zhanghailiang, qemu-trivial, qemu-devel, mreitz,
dnbrdsky, stefanha, pbonzini, kuhn.chenqun
Did this fall through the cracks?
Gan Qixin <ganqixin@huawei.com> writes:
> Hi all,
> I saw some tasks to replace manual lock()/unlock() calls with lock guard macros in BiteSizedTasks.
> I am very interested in this and modified some of the files under block. Could someone help me check the code?
>
> Thanks,
> Gan Qixin
>
> Gan Qixin (4):
> block/accounting.c: Use lock guard macros
> block/curl.c: Use lock guard macros
> block/throttle-groups.c: Use lock guard macros
> block/iscsi.c: Use lock guard macros
>
> block/accounting.c | 32 +++++++++++++--------------
> block/curl.c | 28 ++++++++++++------------
> block/iscsi.c | 28 +++++++++++-------------
> block/throttle-groups.c | 48 ++++++++++++++++++++---------------------
> 4 files changed, 65 insertions(+), 71 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/4] Use lock guard macros in block
2020-12-02 9:22 ` [PATCH 0/4] Use lock guard macros in block Markus Armbruster
@ 2020-12-02 9:33 ` Paolo Bonzini
0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2020-12-02 9:33 UTC (permalink / raw)
To: Markus Armbruster, Gan Qixin
Cc: kwolf, zhang.zhanghailiang, qemu-trivial, qemu-devel, mreitz,
dnbrdsky, stefanha, kuhn.chenqun
On 02/12/20 10:22, Markus Armbruster wrote:
> Did this fall through the cracks?
>
> Gan Qixin <ganqixin@huawei.com> writes:
>
>> Hi all,
>> I saw some tasks to replace manual lock()/unlock() calls with lock guard macros in BiteSizedTasks.
>> I am very interested in this and modified some of the files under block. Could someone help me check the code?
>>
>> Thanks,
>> Gan Qixin
>>
>> Gan Qixin (4):
>> block/accounting.c: Use lock guard macros
>> block/curl.c: Use lock guard macros
>> block/throttle-groups.c: Use lock guard macros
>> block/iscsi.c: Use lock guard macros
>>
>> block/accounting.c | 32 +++++++++++++--------------
>> block/curl.c | 28 ++++++++++++------------
>> block/iscsi.c | 28 +++++++++++-------------
>> block/throttle-groups.c | 48 ++++++++++++++++++++---------------------
>> 4 files changed, 65 insertions(+), 71 deletions(-)
>
Yes, it did.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread