From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48187 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ptd0p-00089z-Qz for qemu-devel@nongnu.org; Sun, 27 Feb 2011 04:34:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ptd0o-0003lr-Nq for qemu-devel@nongnu.org; Sun, 27 Feb 2011 04:33:59 -0500 Received: from fmmailgate01.web.de ([217.72.192.221]:43827) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ptd0o-0003l7-7H for qemu-devel@nongnu.org; Sun, 27 Feb 2011 04:33:58 -0500 Message-ID: <4D6A1A7E.6040501@web.de> Date: Sun, 27 Feb 2011 10:33:50 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1298734819-1960-1-git-send-email-pbonzini@redhat.com> <1298734819-1960-8-git-send-email-pbonzini@redhat.com> In-Reply-To: <1298734819-1960-8-git-send-email-pbonzini@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig7C7EB1A030C27D059BB23077" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH v2 upstream 07/22] add assertions on the owner of a QemuMutex List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, aurelien@aurel32.net This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig7C7EB1A030C27D059BB23077 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2011-02-26 16:40, Paolo Bonzini wrote: > These are already present in the Win32 implementation, add them to > the pthread wrappers as well. Use PTHREAD_MUTEX_ERRORCHECK for mutex > operations, and track the owner separately for cond_signal/broadcast. >=20 > Signed-off-by: Paolo Bonzini > --- > qemu-thread-posix.c | 23 +++++++++++++++++++++-- > qemu-thread-posix.h | 1 + > 2 files changed, 22 insertions(+), 2 deletions(-) >=20 > diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c > index e307773..a4c6e25 100644 > --- a/qemu-thread-posix.c > +++ b/qemu-thread-posix.c > @@ -16,9 +16,12 @@ > #include > #include > #include > +#include > #include > #include "qemu-thread.h" > =20 > +static pthread_t pthread_null; > + > static void error_exit(int err, const char *msg) > { > fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err)); > @@ -28,8 +31,13 @@ static void error_exit(int err, const char *msg) > void qemu_mutex_init(QemuMutex *mutex) > { > int err; > + pthread_mutexattr_t mutexattr; > =20 > - err =3D pthread_mutex_init(&mutex->lock, NULL); > + mutex->owner =3D pthread_null; > + pthread_mutexattr_init(&mutexattr); > + pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_ERRORCHECK); > + err =3D pthread_mutex_init(&mutex->lock, &mutexattr); > + pthread_mutexattr_destroy(&mutexattr); > if (err) > error_exit(err, __func__); > } > @@ -48,13 +56,20 @@ void qemu_mutex_lock(QemuMutex *mutex) > int err; > =20 > err =3D pthread_mutex_lock(&mutex->lock); > + mutex->owner =3D pthread_self(); > if (err) > error_exit(err, __func__); > } > =20 > int qemu_mutex_trylock(QemuMutex *mutex) > { > - return pthread_mutex_trylock(&mutex->lock); > + int err; > + err =3D pthread_mutex_trylock(&mutex->lock); > + if (err =3D=3D 0) { > + mutex->owner =3D pthread_self(); > + } > + > + return !!err; > } > =20 > static void timespec_add_ms(struct timespec *ts, uint64_t msecs) > @@ -85,6 +100,7 @@ void qemu_mutex_unlock(QemuMutex *mutex) > { > int err; > =20 > + mutex->owner =3D pthread_null; > err =3D pthread_mutex_unlock(&mutex->lock); > if (err) > error_exit(err, __func__); > @@ -130,7 +146,10 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mut= ex) > { > int err; > =20 > + assert(pthread_equal(mutex->owner, pthread_self())); > + mutex->owner =3D pthread_null; > err =3D pthread_cond_wait(&cond->cond, &mutex->lock); Though POSIX is not 100% explicit on this, every sane pthread_cond_wait implementation will apply the same error checking as on pthread_mutex_unlock when the given mutex is of PTHREAD_MUTEX_ERRORCHECK. So, this assert is actually redundant as well. Now that we are left without any assertions, I start wondering about one of the original missions: enforce qemu_cond_signal/broadcast to be called under a mutex. What about extending those services with a mutex argument and applying the assert there? Could become a static-inline wrapper so that the argument is optimized away if assert() is inactive. Jan --------------enig7C7EB1A030C27D059BB23077 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAk1qGoMACgkQitSsb3rl5xQjUQCfdfILK5Tk+zQSeyDdRpRonlER GhQAoNaSSNRxGhZfDVUrTxVWW1gGWf0K =Ofls -----END PGP SIGNATURE----- --------------enig7C7EB1A030C27D059BB23077--