* Mahmoud Mandour (ma.mando...@gmail.com) wrote:
> Removed various qemu_mutex_lock and their respective qemu_mutex_unlock
> calls and used lock guard macros (QEMU_LOCK_GUARD and
> WITH_QEMU_LOCK_GUARD). This simplifies the code by
> eliminating qemu_mutex_unlock calls.
>
> Signed-off-by: Mahmoud Mandour <ma.mando...@gmail.com>
> ---
> monitor/monitor.c | 8 ++------
> monitor/qmp.c | 51 ++++++++++++++++++++++-------------------------
> 2 files changed, 26 insertions(+), 33 deletions(-)
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index e94f532cf5..640496e562 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -349,7 +349,7 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event,
> QDict *qdict)
> evconf = &monitor_qapi_event_conf[event];
> trace_monitor_protocol_event_queue(event, qdict, evconf->rate);
>
> - qemu_mutex_lock(&monitor_lock);
> + QEMU_LOCK_GUARD(&monitor_lock);
>
> if (!evconf->rate) {
> /* Unthrottled event */
> @@ -391,8 +391,6 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event,
> QDict *qdict)
> timer_mod_ns(evstate->timer, now + evconf->rate);
> }
> }
> -
> - qemu_mutex_unlock(&monitor_lock);
> }
>
> void qapi_event_emit(QAPIEvent event, QDict *qdict)
> @@ -447,7 +445,7 @@ static void monitor_qapi_event_handler(void *opaque)
> MonitorQAPIEventConf *evconf = &monitor_qapi_event_conf[evstate->event];
>
> trace_monitor_protocol_event_handler(evstate->event, evstate->qdict);
> - qemu_mutex_lock(&monitor_lock);
> + QEMU_LOCK_GUARD(&monitor_lock);
>
> if (evstate->qdict) {
> int64_t now = qemu_clock_get_ns(monitor_get_event_clock());
> @@ -462,8 +460,6 @@ static void monitor_qapi_event_handler(void *opaque)
> timer_free(evstate->timer);
> g_free(evstate);
> }
> -
> - qemu_mutex_unlock(&monitor_lock);
> }
>
> static unsigned int qapi_event_throttle_hash(const void *key)
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 2326bd7f9b..2b0308f933 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -76,7 +76,7 @@ static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP
> *mon)
>
> static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
> {
> - qemu_mutex_lock(&mon->qmp_queue_lock);
> + QEMU_LOCK_GUARD(&mon->qmp_queue_lock);
>
> /*
> * Same condition as in monitor_qmp_dispatcher_co(), but before
> @@ -103,7 +103,6 @@ static void
> monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
> monitor_resume(&mon->common);
> }
>
> - qemu_mutex_unlock(&mon->qmp_queue_lock);
> }
>
> void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
> @@ -179,7 +178,7 @@ static QMPRequest
> *monitor_qmp_requests_pop_any_with_lock(void)
> Monitor *mon;
> MonitorQMP *qmp_mon;
>
> - qemu_mutex_lock(&monitor_lock);
> + QEMU_LOCK_GUARD(&monitor_lock);
>
> QTAILQ_FOREACH(mon, &mon_list, entry) {
> if (!monitor_is_qmp(mon)) {
> @@ -205,8 +204,6 @@ static QMPRequest
> *monitor_qmp_requests_pop_any_with_lock(void)
> QTAILQ_INSERT_TAIL(&mon_list, mon, entry);
> }
>
> - qemu_mutex_unlock(&monitor_lock);
> -
> return req_obj;
You could have used it for the qmp_queue_lock in this function as well;
that can go in a later patch sometime.
> }
>
> @@ -376,30 +373,30 @@ static void handle_qmp_command(void *opaque, QObject
> *req, Error *err)
> req_obj->err = err;
>
> /* Protect qmp_requests and fetching its length. */
> - qemu_mutex_lock(&mon->qmp_queue_lock);
> + WITH_QEMU_LOCK_GUARD(&mon->qmp_queue_lock) {
>
> - /*
> - * Suspend the monitor when we can't queue more requests after
> - * this one. Dequeuing in monitor_qmp_dispatcher_co() or
> - * monitor_qmp_cleanup_queue_and_resume() will resume it.
> - * Note that when OOB is disabled, we queue at most one command,
> - * for backward compatibility.
> - */
> - if (!qmp_oob_enabled(mon) ||
> - mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> - monitor_suspend(&mon->common);
> - }
> + /*
> + * Suspend the monitor when we can't queue more requests after
> + * this one. Dequeuing in monitor_qmp_dispatcher_co() or
> + * monitor_qmp_cleanup_queue_and_resume() will resume it.
> + * Note that when OOB is disabled, we queue at most one command,
> + * for backward compatibility.
> + */
> + if (!qmp_oob_enabled(mon) ||
> + mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> + monitor_suspend(&mon->common);
> + }
>
> - /*
> - * Put the request to the end of queue so that requests will be
> - * handled in time order. Ownership for req_obj, req,
> - * etc. will be delivered to the handler side.
> - */
> - trace_monitor_qmp_in_band_enqueue(req_obj, mon,
> - mon->qmp_requests->length);
> - assert(mon->qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX);
> - g_queue_push_tail(mon->qmp_requests, req_obj);
> - qemu_mutex_unlock(&mon->qmp_queue_lock);
> + /*
> + * Put the request to the end of queue so that requests will be
> + * handled in time order. Ownership for req_obj, req,
> + * etc. will be delivered to the handler side.
> + */
> + trace_monitor_qmp_in_band_enqueue(req_obj, mon,
> + mon->qmp_requests->length);
> + assert(mon->qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX);
> + g_queue_push_tail(mon->qmp_requests, req_obj);
> + }
>
> /* Kick the dispatcher routine */
> if (!qatomic_xchg(&qmp_dispatcher_co_busy, true)) {
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> --
> 2.25.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
