On Thu, Apr 19, 2018 at 09:56:31AM +0800, Fam Zheng wrote:
> 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?
Yeah; I am not 100% sure about whether it's cheap or not, hence with
that. I can remove that part if we are sure we want it always.
--
Peter Xu
