qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: peterx@redhat.com, Paolo Bonzini <pbonzini@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Yury Kotov <yury-kotov@yandex-team.ru>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: [PATCH RFC 2/2] qemu-thread: Fail hard for suspecious mutex unlocks
Date: Tue, 11 Oct 2022 18:41:54 -0400	[thread overview]
Message-ID: <20221011224154.644379-3-peterx@redhat.com> (raw)
In-Reply-To: <20221011224154.644379-1-peterx@redhat.com>

Add a field for QemuMutex to remember the locked status, then assert
properly when CONFIG_DEBUG_MUTEX enabled on illegal unlocks.

The pthread library is by default quite loose on this by allowing the
unlock to quietly succeed.  But that could cause the follow up things very
unpredictable so if there's a bug it'll be harder to track than failing
early at the illegal unlock.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/qemu/thread-posix.h |  1 +
 util/qemu-thread-common.h   | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index 5f2f3d1386..e13bd5492c 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -9,6 +9,7 @@ struct QemuMutex {
 #ifdef CONFIG_DEBUG_MUTEX
     const char *file;
     int line;
+    bool locked;
 #endif
     bool initialized;
 };
diff --git a/util/qemu-thread-common.h b/util/qemu-thread-common.h
index 2af6b12085..ed74bdb0d1 100644
--- a/util/qemu-thread-common.h
+++ b/util/qemu-thread-common.h
@@ -21,6 +21,7 @@ static inline void qemu_mutex_post_init(QemuMutex *mutex)
 #ifdef CONFIG_DEBUG_MUTEX
     mutex->file = NULL;
     mutex->line = 0;
+    mutex->locked = false;
 #endif
     mutex->initialized = true;
 }
@@ -37,6 +38,7 @@ static inline void qemu_mutex_post_lock(QemuMutex *mutex,
 #ifdef CONFIG_DEBUG_MUTEX
     mutex->file = file;
     mutex->line = line;
+    mutex->locked = true;
 #endif
     trace_qemu_mutex_locked(mutex, file, line);
 }
@@ -47,6 +49,14 @@ static inline void qemu_mutex_pre_unlock(QemuMutex *mutex,
 #ifdef CONFIG_DEBUG_MUTEX
     mutex->file = NULL;
     mutex->line = 0;
+    /*
+     * pthread_mutex_unlock() by default silently ignore unlocking a mutex
+     * even if it's not locked.  Make it strict with QEMU when DEBUG_MUTEX
+     * is enabled, so that we can capture it at the exact wrong unlock.
+     * It'll be easier to track this than having misterious deadlock later.
+     */
+    assert(mutex->locked);
+    mutex->locked = false;
 #endif
     trace_qemu_mutex_unlock(mutex, file, line);
 }
-- 
2.37.3



  parent reply	other threads:[~2022-10-11 22:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-11 22:41 [PATCH RFC 0/2] qemu-thread: Strict unlock check Peter Xu
2022-10-11 22:41 ` [PATCH RFC 1/2] qemu-thread: Enable the new timedwait to use DEBUG_MUTEX too Peter Xu
2022-10-11 22:41 ` Peter Xu [this message]
2022-10-12 18:16 ` [PATCH RFC 0/2] qemu-thread: Strict unlock check Peter Xu
2022-10-13 10:31   ` Peter Maydell
2022-10-13 15:05     ` Peter Xu

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=20221011224154.644379-3-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=yury-kotov@yandex-team.ru \
    /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).