[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] correctly handle resize of empty hash tree
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] correctly handle resize of empty hash tree |
Date: |
Wed, 10 Aug 2016 17:19:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
On 10/08/2016 03:05, Brent W. Baccala wrote:
> Hi -
>
> This is a bug report that includes a patch. My understanding of your
> submission guidelines in that it should go to qemu-devel and not to the bug
> tracker. Sorry if I didn't understand your procedures.
>
> I'm running qemu-system-i386 built from the master branch from git://
> git.qemu-project.org/qemu.git.
>
> The problem shows up with a Debian/Hurd guest using remote GDB on the Mach
> kernel. Setting a breakpoint in the kernel with GDB and then hitting the
> breakpoint triggers a SEGV in qemu, i.e:
>
> address@hidden:~/src/qemu$ gdb i386-softmmu/qemu-system-i386
> (gdb) run -enable-kvm -m 512 -nographic -drive cache=writeback,format=raw,
> file=/home/baccala/hurd/debian-hurd.img -net nic -net
> user,hostfwd=tcp::3333-:22 -s
>
> After Hurd boots, in a different window, I attach GDB and set a kernel
> breakpoint:
>
> address@hidden:~/hurd$ gdb gnumach-17-486-dbg
> (gdb) target remote localhost:1234
> Remote debugging using localhost:1234
> machine_idle (cpu=0) at ../i386/i386at/model_dep.c:207
> 207 ../i386/i386at/model_dep.c: No such file or directory.
> (gdb) where
> #0 machine_idle (cpu=0) at ../i386/i386at/model_dep.c:207
> #1 0x810293fa in idle_thread_continue () at ../kern/sched_prim.c:1674
> #2 0x00000000 in ?? ()
> (gdb) break elf_db_search_symbol
> Breakpoint 1 at 0x8100d080: file ../ddb/db_elf.c, line 159.
> (gdb) cont
> Continuing.
>
> Now, I trigger the kernel breakpoint (by hitting Cntl-Alt-d to enter the
> Mach debugger), and qemu segfaults. Here's what GDB on qemu itself shows:
>
> Thread 1 "qemu-system-i38" received signal SIGSEGV, Segmentation fault.
> 0x0000555555b9d0c3 in qht_reset_size (ht=0x55555617f698 <tcg_ctx+216>,
> n_elems=32768) at util/qht.c:422
> 422 if (n_buckets != map->n_buckets) {
> (gdb) where
> #0 0x0000555555b9d0c3 in qht_reset_size (ht=0x55555617f698 <tcg_ctx+216>,
> n_elems=32768) at util/qht.c:422
> #1 0x000055555573f9d1 in tb_flush (cpu=0x0) at /home/baccala/src/qemu/
> translate-all.c:855
> #2 0x000055555577ac96 in gdb_vm_state_change (opaque=0x0, running=0,
> state=RUN_STATE_DEBUG)
> at /home/baccala/src/qemu/gdbstub.c:1276
> #3 0x000055555589d167 in vm_state_notify (running=0,
> state=RUN_STATE_DEBUG) at vl.c:1585
> #4 0x000055555576bd9c in do_vm_stop (state=RUN_STATE_DEBUG) at
> /home/baccala/src/qemu/cpus.c:743
> #5 0x000055555576d550 in vm_stop (state=RUN_STATE_DEBUG) at
> /home/baccala/src/qemu/cpus.c:1476
> #6 0x000055555589d7c4 in main_loop_should_exit () at vl.c:1856
> #7 0x000055555589d933 in main_loop () at vl.c:1912
> #8 0x00005555558a4ee7 in main (argc=12, argv=0x7fffffffddc8,
> envp=0x7fffffffde30) at vl.c:4603
> (gdb)
>
> "map" is NULL at this point. I'm not sure what should happen when
> ght_reset_size() gets called on a hash tree with a NULL map. I applied the
> following patch and everything seems to work. I can now hit kernel
> breakpoints and step/next through the Mach kernel code without segfaults
> from qemu. I don't know qemu, so it's just an educated guess.
>
> agape
> brent
>
> Signed-off-by: Brent Baccala <address@hidden>
> ---
> util/qht.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/util/qht.c b/util/qht.c
> index 16a8d79..9af21e3 100644
> --- a/util/qht.c
> +++ b/util/qht.c
> @@ -419,6 +419,14 @@ bool qht_reset_size(struct qht *ht, size_t n_elems)
>
> qemu_mutex_lock(&ht->lock);
> map = ht->map;
> +
> + if (!map) {
> + new = qht_map_create(n_buckets);
> + atomic_rcu_set(&ht->map, new);
> + qemu_mutex_unlock(&ht->lock);
> + return true;
> + }
> +
> if (n_buckets != map->n_buckets) {
> new = qht_map_create(n_buckets);
> resize = true;
>
The patch makes sense, but I think we don't need to call qht_reset_size
at all.
tb_flush should not do anything if using KVM. There are several ways to
do this:
- put the tb_flush call under "if (tcg_enabled())"
- add an "if (!tcg_enabled()) return;" in tb_flush
- add an "if (!tcg_ctx.tb_ctx.nb_tbs) return;" in tb_flush
Thanks,
Paolo