Skip to content

Commit

Permalink
cpus.s: make the iothread "recursive" (MTTCG)
Browse files Browse the repository at this point in the history
While I was testing multi-threaded TCG I discovered once consequence of
using locking around memory_region_dispatch is that virt-io transactions
could dead lock trying to grab the main mutex. This is due to the
virt-io driver writing data back into the system memory:

    #0  0x00007ffff119dcc9 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
    qemu#1  0x00007ffff11a10d8 in __GI_abort () at abort.c:89
    qemu#2  0x00005555555f9b24 in error_exit (err=<optimised out>, msg=msg@entry=0x5555559f3710 <__func__.6011> "qemu_mutex_lock") at util/qemu-thread-posix.c:48
    qemu#3  0x000055555594d630 in qemu_mutex_lock (mutex=mutex@entry=0x555555e62e60 <qemu_global_mutex>) at util/qemu-thread-posix.c:79
    qemu#4  0x0000555555631a84 in qemu_mutex_lock_iothread () at /home/alex/lsrc/qemu/qemu.git/cpus.c:1128
    qemu#5  0x000055555560dd1a in stw_phys_internal (endian=DEVICE_LITTLE_ENDIAN, val=1, addr=<optimised out>, as=0x555555e08060 <address_space_memory>) at /home/alex/lsrc/qemu/qemu.git/exec.c:3010
    qemu#6  stw_le_phys (as=as@entry=0x555555e08060 <address_space_memory>, addr=<optimised out>, val=1) at /home/alex/lsrc/qemu/qemu.git/exec.c:3024
    qemu#7  0x0000555555696ae5 in virtio_stw_phys (vdev=<optimised out>, value=<optimised out>, pa=<optimised out>) at /home/alex/lsrc/qemu/qemu.git/include/hw/virtio/virtio-access.h:61
    qemu#8  vring_avail_event (vq=0x55555648dc00, vq=0x55555648dc00, vq=0x55555648dc00, val=<optimised out>) at /home/alex/lsrc/qemu/qemu.git/hw/virtio/virtio.c:214
    qemu#9  virtqueue_pop (vq=0x55555648dc00, elem=elem@entry=0x7fff1403fd98) at /home/alex/lsrc/qemu/qemu.git/hw/virtio/virtio.c:472
    qemu#10 0x0000555555653cd1 in virtio_blk_get_request (s=0x555556486830) at /home/alex/lsrc/qemu/qemu.git/hw/block/virtio-blk.c:122
    qemu#11 virtio_blk_handle_output (vdev=<optimised out>, vq=<optimised out>) at /home/alex/lsrc/qemu/qemu.git/hw/block/virtio-blk.c:446
    qemu#12 0x00005555556414e1 in access_with_adjusted_size (addr=addr@entry=80, value=value@entry=0x7fffa93052b0, size=size@entry=4, access_size_min=<optimised out>,
        access_size_max=<optimised out>, access=0x5555556413e0 <memory_region_write_accessor>, mr=0x555556b80388) at /home/alex/lsrc/qemu/qemu.git/memory.c:461
    qemu#13 0x00005555556471b7 in memory_region_dispatch_write (size=4, data=0, addr=80, mr=0x555556b80388) at /home/alex/lsrc/qemu/qemu.git/memory.c:1103
    qemu#14 io_mem_write (mr=mr@entry=0x555556b80388, addr=80, val=<optimised out>, size=size@entry=4) at /home/alex/lsrc/qemu/qemu.git/memory.c:2003
    qemu#15 0x000055555560ad6b in address_space_rw_internal (as=<optimised out>, addr=167788112, buf=buf@entry=0x7fffa9305380 "", len=4, is_write=is_write@entry=true, unlocked=<optimised out>,
        unlocked@entry=false) at /home/alex/lsrc/qemu/qemu.git/exec.c:2318
    qemu#16 0x000055555560aea8 in address_space_rw (is_write=true, len=<optimised out>, buf=0x7fffa9305380 "", addr=<optimised out>, as=<optimised out>) at /home/alex/lsrc/qemu/qemu.git/exec.c:2392
    qemu#17 address_space_write (len=<optimised out>, buf=0x7fffa9305380 "", addr=<optimised out>, as=<optimised out>) at /home/alex/lsrc/qemu/qemu.git/exec.c:2404
    qemu#18 subpage_write (opaque=<optimised out>, addr=<optimised out>, value=<optimised out>, len=<optimised out>) at /home/alex/lsrc/qemu/qemu.git/exec.c:1963
    qemu#19 0x00005555556414e1 in access_with_adjusted_size (addr=addr@entry=592, value=value@entry=0x7fffa9305420, size=size@entry=4, access_size_min=<optimised out>,
        access_size_max=<optimised out>, access=0x5555556413e0 <memory_region_write_accessor>, mr=0x555556bfca20) at /home/alex/lsrc/qemu/qemu.git/memory.c:461
    qemu#20 0x00005555556471b7 in memory_region_dispatch_write (size=4, data=0, addr=592, mr=0x555556bfca20) at /home/alex/lsrc/qemu/qemu.git/memory.c:1103
    qemu#21 io_mem_write (mr=mr@entry=0x555556bfca20, addr=addr@entry=592, val=val@entry=0, size=size@entry=4) at /home/alex/lsrc/qemu/qemu.git/memory.c:2003
    qemu#22 0x000055555564ce16 in io_writel (retaddr=140736492182797, addr=4027616848, val=0, physaddr=592, env=0x55555649e9b0) at /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:386
    qemu#23 helper_le_stl_mmu (env=0x55555649e9b0, addr=<optimised out>, val=0, mmu_idx=<optimised out>, retaddr=140736492182797) at /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:426
    qemu#24 0x00007fffc49f9d0f in code_gen_buffer ()
    qemu#25 0x00005555556109dc in cpu_tb_exec (tb_ptr=0x7fffc49f9c60 <code_gen_buffer+8371296> "A\213n\374\205\355\017\205\233\001", cpu=0x555556496750)
        at /home/alex/lsrc/qemu/qemu.git/cpu-exec.c:179
    qemu#26 cpu_arm_exec (env=env@entry=0x55555649e9b0) at /home/alex/lsrc/qemu/qemu.git/cpu-exec.c:524
    qemu#27 0x0000555555630f28 in tcg_cpu_exec (env=0x55555649e9b0) at /home/alex/lsrc/qemu/qemu.git/cpus.c:1344
    qemu#28 tcg_exec_all (cpu=0x555556496750) at /home/alex/lsrc/qemu/qemu.git/cpus.c:1392
    qemu#29 qemu_tcg_cpu_thread_fn (arg=0x555556496750) at /home/alex/lsrc/qemu/qemu.git/cpus.c:1037
    qemu#30 0x00007ffff1534182 in start_thread (arg=0x7fffa9306700) at pthread_create.c:312
    qemu#31 0x00007ffff126147d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

The fix in this patch makes the global/iothread mutex recursive.
However as condition variables are involved we deal with this in our
own code by tracking the number of locks held by the thread.

This seems neater than having to do a trylock each time.

Tested-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - don't actually use pthread recursion mechanism
  - convert rest of qemu_mutex_lock(&global_global_mutex) to iothread
  - emulate recursion with thread local tracking
  • Loading branch information
stsquad committed Jun 24, 2015
1 parent 9ec345d commit 6dde79c
Showing 1 changed file with 15 additions and 3 deletions.
18 changes: 15 additions & 3 deletions cpus.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@
int64_t max_delay;
int64_t max_advance;

static __thread int iothread_lock_count;

bool cpu_is_stopped(CPUState *cpu)
{
return cpu->stopped || !runstate_is_running();
Expand Down Expand Up @@ -928,7 +930,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
CPUState *cpu = arg;
int r;

qemu_mutex_lock(&qemu_global_mutex);
qemu_mutex_lock_iothread();
qemu_thread_get_self(cpu->thread);
cpu->thread_id = qemu_get_thread_id();
cpu->can_do_io = 1;
Expand Down Expand Up @@ -1115,14 +1117,24 @@ bool qemu_in_vcpu_thread(void)

void __qemu_mutex_lock_iothread(const char *func, int line)
{
qemu_mutex_lock(&qemu_global_mutex);
if (iothread_lock_count == 0) {
qemu_mutex_lock(&qemu_global_mutex);
}
iothread_lock_count++;
qemu_global_mutex.func = func;
qemu_global_mutex.line = line;
}

void qemu_mutex_unlock_iothread(void)
{
qemu_mutex_unlock(&qemu_global_mutex);
iothread_lock_count--;
if (iothread_lock_count==0) {
qemu_mutex_unlock(&qemu_global_mutex);
}
if (iothread_lock_count < 0) {
fprintf(stderr,"%s: error, too many unlocks %d\n", __func__,
iothread_lock_count);
}
}

static int all_vcpus_paused(void)
Expand Down

0 comments on commit 6dde79c

Please sign in to comment.