diff mbox series

[5/9] monitor: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD

Message ID 20210311031538.5325-6-ma.mandourr@gmail.com (mailing list archive)
State New, archived
Headers show
Series Changing qemu_mutex_locks to lock guard macros | expand

Commit Message

Mahmoud Abumandour March 11, 2021, 3:15 a.m. UTC 
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.mandourr@gmail.com>
---
 monitor/monitor.c |  8 ++------
 monitor/qmp.c     | 51 ++++++++++++++++++++++-------------------------
 2 files changed, 26 insertions(+), 33 deletions(-)

Comments

Dr. David Alan Gilbert March 11, 2021, 9:50 a.m. UTC | #1 
* Mahmoud Mandour (ma.mandourr@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.mandourr@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 <dgilbert@redhat.com>

> -- 
> 2.25.1
>
diff mbox series

Patch

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;
 }
 
@@ -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)) {