From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53715) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7P55-0007fG-0y for qemu-devel@nongnu.org; Tue, 23 Jun 2015 10:21:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7P4z-00032A-Rd for qemu-devel@nongnu.org; Tue, 23 Jun 2015 10:21:42 -0400 Received: from greensocs.com ([193.104.36.180]:39529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7P4z-00031W-BC for qemu-devel@nongnu.org; Tue, 23 Jun 2015 10:21:37 -0400 Message-ID: <55896B6B.7020605@greensocs.com> Date: Tue, 23 Jun 2015 16:21:31 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1435062084-25332-1-git-send-email-alex.bennee@linaro.org> In-Reply-To: <1435062084-25332-1-git-send-email-alex.bennee@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH] qemu_mutux: make the iothread recursive (MTTCG) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QWxleCBCZW5uw6ll?= , mttcg@listserver.greensocs.com, mark.burton@greensocs.com, pbonzini@redhat.com Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org On 23/06/2015 14:21, Alex Benn=C3=A9e wrote: > While I was testing multi-threaded TCG I discovered once consequence of > using locking around memory_region_dispatch is that virt-io transaction= s > could dead lock trying to grab the main mutex. This is due to the > virt-io driver writing data back into the system memory: Hi, Thanks for that. Didn't qemu abort in this case with a pthread error? Maybe that did=20 change since the last time I had this error. Thanks, Fred > #0 0x00007ffff119dcc9 in __GI_raise (sig=3Dsig@entry=3D6) at ../n= ptl/sysdeps/unix/sysv/linux/raise.c:56 > #1 0x00007ffff11a10d8 in __GI_abort () at abort.c:89 > #2 0x00005555555f9b24 in error_exit (err=3D, msg=3D= msg@entry=3D0x5555559f3710 <__func__.6011> "qemu_mutex_lock") at util/qem= u-thread-posix.c:48 > #3 0x000055555594d630 in qemu_mutex_lock (mutex=3Dmutex@entry=3D0= x555555e62e60 ) at util/qemu-thread-posix.c:79 > #4 0x0000555555631a84 in qemu_mutex_lock_iothread () at /home/ale= x/lsrc/qemu/qemu.git/cpus.c:1128 > #5 0x000055555560dd1a in stw_phys_internal (endian=3DDEVICE_LITTL= E_ENDIAN, val=3D1, addr=3D, as=3D0x555555e08060 ) at /home/alex/lsrc/qemu/qemu.git/exec.c:3010 > #6 stw_le_phys (as=3Das@entry=3D0x555555e08060 , addr=3D, val=3D1) at /home/alex/lsrc/qemu/qemu.git/e= xec.c:3024 > #7 0x0000555555696ae5 in virtio_stw_phys (vdev=3D,= value=3D, pa=3D) at /home/alex/lsrc/qemu/q= emu.git/include/hw/virtio/virtio-access.h:61 > #8 vring_avail_event (vq=3D0x55555648dc00, vq=3D0x55555648dc00, v= q=3D0x55555648dc00, val=3D) at /home/alex/lsrc/qemu/qemu.g= it/hw/virtio/virtio.c:214 > #9 virtqueue_pop (vq=3D0x55555648dc00, elem=3Delem@entry=3D0x7fff= 1403fd98) at /home/alex/lsrc/qemu/qemu.git/hw/virtio/virtio.c:472 > #10 0x0000555555653cd1 in virtio_blk_get_request (s=3D0x5555564868= 30) at /home/alex/lsrc/qemu/qemu.git/hw/block/virtio-blk.c:122 > #11 virtio_blk_handle_output (vdev=3D, vq=3D) at /home/alex/lsrc/qemu/qemu.git/hw/block/virtio-blk.c:446 > #12 0x00005555556414e1 in access_with_adjusted_size (addr=3Daddr@e= ntry=3D80, value=3Dvalue@entry=3D0x7fffa93052b0, size=3Dsize@entry=3D4, a= ccess_size_min=3D, > access_size_max=3D, access=3D0x5555556413e0 , mr=3D0x555556b80388) at /home/alex/lsrc/qemu= /qemu.git/memory.c:461 > #13 0x00005555556471b7 in memory_region_dispatch_write (size=3D4, = data=3D0, addr=3D80, mr=3D0x555556b80388) at /home/alex/lsrc/qemu/qemu.gi= t/memory.c:1103 > #14 io_mem_write (mr=3Dmr@entry=3D0x555556b80388, addr=3D80, val=3D= , size=3Dsize@entry=3D4) at /home/alex/lsrc/qemu/qemu.git/= memory.c:2003 > #15 0x000055555560ad6b in address_space_rw_internal (as=3D, addr=3D167788112, buf=3Dbuf@entry=3D0x7fffa9305380 "", len=3D4, = is_write=3Dis_write@entry=3Dtrue, unlocked=3D, > unlocked@entry=3Dfalse) at /home/alex/lsrc/qemu/qemu.git/exec.= c:2318 > #16 0x000055555560aea8 in address_space_rw (is_write=3Dtrue, len=3D= , buf=3D0x7fffa9305380 "", addr=3D, as=3D) at /home/alex/lsrc/qemu/qemu.git/exec.c:2392 > #17 address_space_write (len=3D, buf=3D0x7fffa93053= 80 "", addr=3D, as=3D) at /home/alex/lsrc/q= emu/qemu.git/exec.c:2404 > #18 subpage_write (opaque=3D, addr=3D, value=3D, len=3D) at /home/alex/lsrc/qem= u/qemu.git/exec.c:1963 > #19 0x00005555556414e1 in access_with_adjusted_size (addr=3Daddr@e= ntry=3D592, value=3Dvalue@entry=3D0x7fffa9305420, size=3Dsize@entry=3D4, = access_size_min=3D, > access_size_max=3D, access=3D0x5555556413e0 , mr=3D0x555556bfca20) at /home/alex/lsrc/qemu= /qemu.git/memory.c:461 > #20 0x00005555556471b7 in memory_region_dispatch_write (size=3D4, = data=3D0, addr=3D592, mr=3D0x555556bfca20) at /home/alex/lsrc/qemu/qemu.g= it/memory.c:1103 > #21 io_mem_write (mr=3Dmr@entry=3D0x555556bfca20, addr=3Daddr@entr= y=3D592, val=3Dval@entry=3D0, size=3Dsize@entry=3D4) at /home/alex/lsrc/q= emu/qemu.git/memory.c:2003 > #22 0x000055555564ce16 in io_writel (retaddr=3D140736492182797, ad= dr=3D4027616848, val=3D0, physaddr=3D592, env=3D0x55555649e9b0) at /home/= alex/lsrc/qemu/qemu.git/softmmu_template.h:386 > #23 helper_le_stl_mmu (env=3D0x55555649e9b0, addr=3D, val=3D0, mmu_idx=3D, retaddr=3D140736492182797) at /hom= e/alex/lsrc/qemu/qemu.git/softmmu_template.h:426 > #24 0x00007fffc49f9d0f in code_gen_buffer () > #25 0x00005555556109dc in cpu_tb_exec (tb_ptr=3D0x7fffc49f9c60 "A\213n\374\205\355\017\205\233\001", cpu=3D0x5555= 56496750) > at /home/alex/lsrc/qemu/qemu.git/cpu-exec.c:179 > #26 cpu_arm_exec (env=3Denv@entry=3D0x55555649e9b0) at /home/alex/= lsrc/qemu/qemu.git/cpu-exec.c:524 > #27 0x0000555555630f28 in tcg_cpu_exec (env=3D0x55555649e9b0) at /= home/alex/lsrc/qemu/qemu.git/cpus.c:1344 > #28 tcg_exec_all (cpu=3D0x555556496750) at /home/alex/lsrc/qemu/qe= mu.git/cpus.c:1392 > #29 qemu_tcg_cpu_thread_fn (arg=3D0x555556496750) at /home/alex/ls= rc/qemu/qemu.git/cpus.c:1037 > #30 0x00007ffff1534182 in start_thread (arg=3D0x7fffa9306700) at p= thread_create.c:312 > #31 0x00007ffff126147d in clone () at ../sysdeps/unix/sysv/linux/x= 86_64/clone.S:111 > > The fix in this patch makes the global/iothread mutex recursive (only i= f > called from the same thread-id). As other threads are still blocked > memory is still consistent all the way through. > > This seems neater than having to do a trylock each time. > > Tested-by: Alex Benn=C3=A9e > --- > cpus.c | 2 +- > include/qemu/thread.h | 1 + > util/qemu-thread-posix.c | 12 ++++++++++++ > util/qemu-thread-win32.c | 5 +++++ > 4 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/cpus.c b/cpus.c > index d161bb9..412ab04 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -804,7 +804,7 @@ void qemu_init_cpu_loop(void) > qemu_cond_init(&qemu_pause_cond); > qemu_cond_init(&qemu_work_cond); > qemu_cond_init(&qemu_io_proceeded_cond); > - qemu_mutex_init(&qemu_global_mutex); > + qemu_mutex_init_recursive(&qemu_global_mutex); > =20 > qemu_thread_get_self(&io_thread); > } > diff --git a/include/qemu/thread.h b/include/qemu/thread.h > index c9389f4..4519d2f 100644 > --- a/include/qemu/thread.h > +++ b/include/qemu/thread.h > @@ -22,6 +22,7 @@ typedef struct QemuThread QemuThread; > #define QEMU_THREAD_DETACHED 1 > =20 > void qemu_mutex_init(QemuMutex *mutex); > +void qemu_mutex_init_recursive(QemuMutex *mutex); > void qemu_mutex_destroy(QemuMutex *mutex); > void __qemu_mutex_lock(QemuMutex *mutex, const char *func, int line); > #define qemu_mutex_lock(mutex) __qemu_mutex_lock(mutex, __func__, __L= INE__) > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c > index 98eb0f0..ba2fb97 100644 > --- a/util/qemu-thread-posix.c > +++ b/util/qemu-thread-posix.c > @@ -57,6 +57,18 @@ void qemu_mutex_init(QemuMutex *mutex) > error_exit(err, __func__); > } > =20 > +void qemu_mutex_init_recursive(QemuMutex *mutex) > +{ > + int err; > + pthread_mutexattr_t attr; > + > + pthread_mutexattr_init(&attr); > + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE_NP); > + err =3D pthread_mutex_init(&mutex->lock, &attr); > + if (err) > + error_exit(err, __func__); > +} > + > void qemu_mutex_destroy(QemuMutex *mutex) > { > int err; > diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c > index 406b52f..f055067 100644 > --- a/util/qemu-thread-win32.c > +++ b/util/qemu-thread-win32.c > @@ -44,6 +44,11 @@ void qemu_mutex_init(QemuMutex *mutex) > InitializeCriticalSection(&mutex->lock); > } > =20 > +void qemu_mutex_init_recursive(QemuMutex *mutex) > +{ > + error_exit(0, "%s: not implemented for win32\n", __func__); > +} > + > void qemu_mutex_destroy(QemuMutex *mutex) > { > assert(mutex->owner =3D=3D 0);