Re: [Qemu-devel] [PATCH] correctly handle resize of empty hash tree
On 08/11/2016 05:41 AM, Emilio G. Cota wrote: > On Thu, Aug 11, 2016 at 10:45:02 +0200, Igor Mammedov wrote: >> On Wed, 10 Aug 2016 17:19:48 +0200 >> Paolo Bonzini wrote: >>> 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 > > I like this option the most. I can still reproduce the crash even with 2.7.0-rc4, so the fix does not seem to help on my system. (gdb) bt #0 0x1043a726 in qht_reset_size (ht=0x10625d98 , n_elems=32768) at /home/cborntra/REPOS/qemu/util/qht.c:422 #1 0x10021ab8 in tb_flush (cpu=0x0) at /home/cborntra/REPOS/qemu/translate-all.c:855 #2 0x10076d02 in gdb_vm_state_change (opaque=0x0, running=0, state=RUN_STATE_DEBUG) at /home/cborntra/REPOS/qemu/gdbstub.c:1276 #3 0x1018f106 in vm_state_notify (running=0, state=RUN_STATE_DEBUG) at /home/cborntra/REPOS/qemu/vl.c:1585 #4 0x100632e6 in do_vm_stop (state=RUN_STATE_DEBUG) at /home/cborntra/REPOS/qemu/cpus.c:743 #5 0x10065450 in vm_stop (state=RUN_STATE_DEBUG) at /home/cborntra/REPOS/qemu/cpus.c:1476 #6 0x1018fc46 in main_loop_should_exit () at /home/cborntra/REPOS/qemu/vl.c:1856 #7 0x1018fe6a in main_loop () at /home/cborntra/REPOS/qemu/vl.c:1912 #8 0x1019809c in main (argc=11, argv=0x3fff368, envp=0x3fff3c8) at /home/cborntra/REPOS/qemu/vl.c:4604 (gdb) Doing the "tcg_enabled()" thing does help, though. Shall I send a patch? This should still make it into 2.7 I think. > > My patch to fix this issue was written thinking that tb_flush was > strangely needed for gdb to work under KVM. If that's not the > case, then let's go for the real fix [above]. > > Thanks, > > E. >
Re: [Qemu-devel] [PATCH] correctly handle resize of empty hash tree
On Thu, Aug 11, 2016 at 10:45:02 +0200, Igor Mammedov wrote: > On Wed, 10 Aug 2016 17:19:48 +0200 > Paolo Bonzini wrote: > > 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 I like this option the most. My patch to fix this issue was written thinking that tb_flush was strangely needed for gdb to work under KVM. If that's not the case, then let's go for the real fix [above]. Thanks, E.
Re: [Qemu-devel] [PATCH] correctly handle resize of empty hash tree
On Wed, 10 Aug 2016 17:19:48 +0200
Paolo Bonzini wrote:
CCing original author,
here is his varinat how to fix it
https://www.mail-archive.com/qemu-devel@nongnu.org/msg391790.html
> 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:
> >
> > baccala@baccala:~/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::-:22 -s
> >
> > After Hurd boots, in a different window, I attach GDB and set a kernel
> > breakpoint:
> >
> > baccala@baccala:~/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 0x 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.
> > 0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
> > n_elems=32768) at util/qht.c:422
> > 422if (n_buckets != map->n_buckets) {
> > (gdb) where
> > #0 0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
> > n_elems=32768) at util/qht.c:422
> > #1 0x5573f9d1 in tb_flush (cpu=0x0) at /home/baccala/src/qemu/
> > translate-all.c:855
> > #2 0x5577ac96 in gdb_vm_state_change (opaque=0x0, running=0,
> > state=RUN_STATE_DEBUG)
> > at /home/baccala/src/qemu/gdbstub.c:1276
> > #3 0x5589d167 in vm_state_notify (running=0,
> > state=RUN_STATE_DEBUG) at vl.c:1585
> > #4 0x5576bd9c in do_vm_stop (state=RUN_STATE_DEBUG) at
> > /home/baccala/src/qemu/cpus.c:743
> > #5 0x5576d550 in vm_stop (state=RUN_STATE_DEBUG) at
> > /home/baccala/src/qemu/cpus.c:1476
> > #6 0x5589d7c4 in main_loop_should_exit () at vl.c:1856
> > #7 0x5589d933 in main_loop () at vl.c:1912
> > #8 0x558a4ee7 in main (argc=12, argv=0x7fffddc8,
> > envp=0x7fffde30) 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
> > ---
> > 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
>
Re: [Qemu-devel] [PATCH] correctly handle resize of empty hash tree
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:
>
> baccala@baccala:~/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::-:22 -s
>
> After Hurd boots, in a different window, I attach GDB and set a kernel
> breakpoint:
>
> baccala@baccala:~/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 0x 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.
> 0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
> n_elems=32768) at util/qht.c:422
> 422if (n_buckets != map->n_buckets) {
> (gdb) where
> #0 0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
> n_elems=32768) at util/qht.c:422
> #1 0x5573f9d1 in tb_flush (cpu=0x0) at /home/baccala/src/qemu/
> translate-all.c:855
> #2 0x5577ac96 in gdb_vm_state_change (opaque=0x0, running=0,
> state=RUN_STATE_DEBUG)
> at /home/baccala/src/qemu/gdbstub.c:1276
> #3 0x5589d167 in vm_state_notify (running=0,
> state=RUN_STATE_DEBUG) at vl.c:1585
> #4 0x5576bd9c in do_vm_stop (state=RUN_STATE_DEBUG) at
> /home/baccala/src/qemu/cpus.c:743
> #5 0x5576d550 in vm_stop (state=RUN_STATE_DEBUG) at
> /home/baccala/src/qemu/cpus.c:1476
> #6 0x5589d7c4 in main_loop_should_exit () at vl.c:1856
> #7 0x5589d933 in main_loop () at vl.c:1912
> #8 0x558a4ee7 in main (argc=12, argv=0x7fffddc8,
> envp=0x7fffde30) 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
> ---
> 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
Re: [Qemu-devel] [PATCH] correctly handle resize of empty hash tree
This fixes regression caused by commit 909eaac9bbc
that I've reported today:
"Re: [Qemu-devel] [PULL 14/15] tb hash: track translated blocks with qht"
On Tue, 9 Aug 2016 15:05:36 -1000
"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:
>
> baccala@baccala:~/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::-:22 -s
>
> After Hurd boots, in a different window, I attach GDB and set a kernel
> breakpoint:
>
> baccala@baccala:~/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 0x 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.
> 0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
> n_elems=32768) at util/qht.c:422
> 422if (n_buckets != map->n_buckets) {
> (gdb) where
> #0 0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
> n_elems=32768) at util/qht.c:422
> #1 0x5573f9d1 in tb_flush (cpu=0x0) at /home/baccala/src/qemu/
> translate-all.c:855
> #2 0x5577ac96 in gdb_vm_state_change (opaque=0x0, running=0,
> state=RUN_STATE_DEBUG)
> at /home/baccala/src/qemu/gdbstub.c:1276
> #3 0x5589d167 in vm_state_notify (running=0,
> state=RUN_STATE_DEBUG) at vl.c:1585
> #4 0x5576bd9c in do_vm_stop (state=RUN_STATE_DEBUG) at
> /home/baccala/src/qemu/cpus.c:743
> #5 0x5576d550 in vm_stop (state=RUN_STATE_DEBUG) at
> /home/baccala/src/qemu/cpus.c:1476
> #6 0x5589d7c4 in main_loop_should_exit () at vl.c:1856
> #7 0x5589d933 in main_loop () at vl.c:1912
> #8 0x558a4ee7 in main (argc=12, argv=0x7fffddc8,
> envp=0x7fffde30) 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
> ---
> 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;
[Qemu-devel] [PATCH] correctly handle resize of empty hash tree
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:
baccala@baccala:~/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::-:22 -s
After Hurd boots, in a different window, I attach GDB and set a kernel
breakpoint:
baccala@baccala:~/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 0x 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.
0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
n_elems=32768) at util/qht.c:422
422if (n_buckets != map->n_buckets) {
(gdb) where
#0 0x55b9d0c3 in qht_reset_size (ht=0x5617f698 ,
n_elems=32768) at util/qht.c:422
#1 0x5573f9d1 in tb_flush (cpu=0x0) at /home/baccala/src/qemu/
translate-all.c:855
#2 0x5577ac96 in gdb_vm_state_change (opaque=0x0, running=0,
state=RUN_STATE_DEBUG)
at /home/baccala/src/qemu/gdbstub.c:1276
#3 0x5589d167 in vm_state_notify (running=0,
state=RUN_STATE_DEBUG) at vl.c:1585
#4 0x5576bd9c in do_vm_stop (state=RUN_STATE_DEBUG) at
/home/baccala/src/qemu/cpus.c:743
#5 0x5576d550 in vm_stop (state=RUN_STATE_DEBUG) at
/home/baccala/src/qemu/cpus.c:1476
#6 0x5589d7c4 in main_loop_should_exit () at vl.c:1856
#7 0x5589d933 in main_loop () at vl.c:1912
#8 0x558a4ee7 in main (argc=12, argv=0x7fffddc8,
envp=0x7fffde30) 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
---
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;
--
2.7.4
