On Wed, 04/18 17:06, Peter Xu wrote:
> We have had some tracing tools for mutex but it's not easy to use them
> for e.g. dead locks. Let's provide "--enable-debug-mutex" parameter
> when configure to allow QemuMutex to store the last owner that took
> specific lock. It might be extremely easy to use this tool to debug
> deadlocks since we can directly know who took the lock then as long as
> we can have a gdb attached to the process.
>
> CC: Paolo Bonzini <pbonz...@redhat.com>
> CC: Stefan Hajnoczi <stefa...@redhat.com>
> CC: Fam Zheng <f...@redhat.com>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> v2:
> - fix "--disable-debug-mutex" not working..
> ---
> configure | 10 ++++++++++
> include/qemu/thread-posix.h | 4 ++++
> util/qemu-thread-posix.c | 4 ++++
> 3 files changed, 18 insertions(+)
>
> diff --git a/configure b/configure
> index 0a19b033bc..a80af735b2 100755
> --- a/configure
> +++ b/configure
> @@ -451,6 +451,7 @@ jemalloc="no"
> replication="yes"
> vxhs=""
> libxml2=""
> +debug_mutex="no"
>
> supported_cpu="no"
> supported_os="no"
> @@ -1374,6 +1375,10 @@ for opt do
> ;;
> --disable-git-update) git_update=no
> ;;
> + --enable-debug-mutex) debug_mutex=yes
> + ;;
> + --disable-debug-mutex) debug_mutex=no
> + ;;
> *)
> echo "ERROR: unknown option $opt"
> echo "Try '$0 --help' for more information"
> @@ -1631,6 +1636,7 @@ disabled with --disable-FEATURE, default is enabled if
> available:
> crypto-afalg Linux AF_ALG crypto backend driver
> vhost-user vhost-user support
> capstone capstone disassembler support
> + debug-mutex mutex debugging support
>
> NOTE: The object files are built at the place where configure is launched
> EOF
> @@ -5874,6 +5880,7 @@ echo "avx2 optimization $avx2_opt"
> echo "replication support $replication"
> echo "VxHS block device $vxhs"
> echo "capstone $capstone"
> +echo "mutex debugging $debug_mutex"
>
> if test "$sdl_too_old" = "yes"; then
> echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -6602,6 +6609,9 @@ fi
> if test "$capstone" != "no" ; then
> echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> fi
> +if test "$debug_mutex" = "yes" ; then
> + echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> +fi
>
> # Hold two types of flag:
> # CONFIG_THREAD_SETNAME_BYTHREAD - we've got a way of setting the name on
> diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
> index f3f47e426f..fd27b34128 100644
> --- a/include/qemu/thread-posix.h
> +++ b/include/qemu/thread-posix.h
> @@ -12,6 +12,10 @@ typedef QemuMutex QemuRecMutex;
>
> struct QemuMutex {
> pthread_mutex_t lock;
> +#ifdef CONFIG_DEBUG_MUTEX
> + const char *file;
> + int line;
> +#endif
These look quite cheap, why we need a configure option to enable it?
> bool initialized;
> };
>
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index b789cf32e9..905a816af6 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -68,6 +68,10 @@ void qemu_mutex_lock_impl(QemuMutex *mutex, const char
> *file, const int line)
> if (err)
> error_exit(err, __func__);
>
> +#ifdef CONFIG_DEBUG_MUTEX
> + mutex->file = file;
> + mutex->line = line;
> +#endif
> trace_qemu_mutex_locked(mutex, file, line);
> }
>
> --
> 2.14.3
>
Fam
