qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: [PATCH v2 1/2] lockable: add lock guards
Date: Mon, 16 Mar 2020 11:09:56 +0000	[thread overview]
Message-ID: <20200316110957.449700-2-stefanha@redhat.com> (raw)
In-Reply-To: <20200316110957.449700-1-stefanha@redhat.com>

This patch introduces two lock guard macros that automatically unlock a
lock object (QemuMutex and others):

  void f(void) {
      QEMU_LOCK_GUARD(&mutex);
      if (!may_fail()) {
          return; /* automatically unlocks mutex */
      }
      ...
  }

and:

  WITH_QEMU_LOCK_GUARD(&mutex) {
      if (!may_fail()) {
          return; /* automatically unlocks mutex */
      }
  }
  /* automatically unlocks mutex here */
  ...

Convert qemu-timer.c functions that benefit from these macros as an
example.  Manual qemu_mutex_lock/unlock() callers are left unmodified in
cases where clarity would not improve by switching to the macros.

Many other QemuMutex users remain in the codebase that might benefit
from lock guards.  Over time they can be converted, if that is
desirable.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/lockable.h | 65 +++++++++++++++++++++++++++++++++++++++++
 util/qemu-timer.c       | 23 +++++++--------
 2 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 84ea794bcf..2b52c7c1e5 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -93,4 +93,69 @@ static inline void qemu_lockable_unlock(QemuLockable *x)
     x->unlock(x->object);
 }
 
+static inline QemuLockable *qemu_lockable_auto_lock(QemuLockable *x)
+{
+    qemu_lockable_lock(x);
+    return x;
+}
+
+static inline void qemu_lockable_auto_unlock(QemuLockable *x)
+{
+    if (x) {
+        qemu_lockable_unlock(x);
+    }
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
+
+#define WITH_QEMU_LOCK_GUARD_(x, var) \
+    for (g_autoptr(QemuLockable) var = \
+                qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x))); \
+         var; \
+         qemu_lockable_auto_unlock(var), var = NULL)
+
+/**
+ * WITH_QEMU_LOCK_GUARD - Lock a lock object for scope
+ *
+ * @x: a lock object (currently one of QemuMutex, CoMutex, QemuSpin).
+ *
+ * This macro defines a lock scope such that entering the scope takes the lock
+ * and leaving the scope releases the lock.  Return statements are allowed
+ * within the scope and release the lock.  Break and continue statements leave
+ * the scope early and release the lock.
+ *
+ *   WITH_QEMU_LOCK_GUARD(&mutex) {
+ *       ...
+ *       if (error) {
+ *           return; <-- mutex is automatically unlocked
+ *       }
+ *
+ *       if (early_exit) {
+ *           break;  <-- leave this scope early
+ *       }
+ *       ...
+ *   }
+ */
+#define WITH_QEMU_LOCK_GUARD(x) \
+    WITH_QEMU_LOCK_GUARD_((x), qemu_lockable_auto##__COUNTER__)
+
+/**
+ * QEMU_LOCK_GUARD - Lock an object until the end of the scope
+ *
+ * @x: a lock object (currently one of QemuMutex, CoMutex, QemuSpin).
+ *
+ * This macro takes a lock until the end of the scope.  Return statements
+ * release the lock.
+ *
+ *   ... <-- mutex not locked
+ *   QEMU_LOCK_GUARD(&mutex); <-- mutex locked from here onwards
+ *   ...
+ *   if (error) {
+ *       return; <-- mutex is automatically unlocked
+ *   }
+ */
+#define QEMU_LOCK_GUARD(x) \
+    g_autoptr(QemuLockable) qemu_lockable_auto##__COUNTER__ = \
+            qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x)))
+
 #endif
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index ef52d28d37..d548d3c1ad 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 #include "qemu/timer.h"
+#include "qemu/lockable.h"
 #include "sysemu/replay.h"
 #include "sysemu/cpus.h"
 
@@ -186,13 +187,12 @@ bool timerlist_expired(QEMUTimerList *timer_list)
         return false;
     }
 
-    qemu_mutex_lock(&timer_list->active_timers_lock);
-    if (!timer_list->active_timers) {
-        qemu_mutex_unlock(&timer_list->active_timers_lock);
-        return false;
+    WITH_QEMU_LOCK_GUARD(&timer_list->active_timers_lock) {
+        if (!timer_list->active_timers) {
+            return false;
+        }
+        expire_time = timer_list->active_timers->expire_time;
     }
-    expire_time = timer_list->active_timers->expire_time;
-    qemu_mutex_unlock(&timer_list->active_timers_lock);
 
     return expire_time <= qemu_clock_get_ns(timer_list->clock->type);
 }
@@ -225,13 +225,12 @@ int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
      * value but ->notify_cb() is called when the deadline changes.  Therefore
      * the caller should notice the change and there is no race condition.
      */
-    qemu_mutex_lock(&timer_list->active_timers_lock);
-    if (!timer_list->active_timers) {
-        qemu_mutex_unlock(&timer_list->active_timers_lock);
-        return -1;
+    WITH_QEMU_LOCK_GUARD(&timer_list->active_timers_lock) {
+        if (!timer_list->active_timers) {
+            return -1;
+        }
+        expire_time = timer_list->active_timers->expire_time;
     }
-    expire_time = timer_list->active_timers->expire_time;
-    qemu_mutex_unlock(&timer_list->active_timers_lock);
 
     delta = expire_time - qemu_clock_get_ns(timer_list->clock->type);
 
-- 
2.24.1


  reply	other threads:[~2020-03-16 11:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-16 11:09 [PATCH v2 0/2] thread: add lock guard macros Stefan Hajnoczi
2020-03-16 11:09 ` Stefan Hajnoczi [this message]
2020-03-16 11:09 ` [PATCH v2 2/2] lockable: add QemuRecMutex support Stefan Hajnoczi
2020-03-16 11:29 ` [PATCH v2 0/2] thread: add lock guard macros Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200316110957.449700-2-stefanha@redhat.com \
    --to=stefanha@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).