Re: [Qemu-devel] [PATCH v4 6/7] qom: replace object property list with GHashTable

[Daniel P. Berrange]

On Fri, Nov 13, 2015 at 10:00:58PM +0100, Christian Borntraeger wrote:
> On 11/13/2015 07:14 PM, Andreas Färber wrote:
> > Am 13.10.2015 um 14:37 schrieb Daniel P. Berrange:
> >> From: Pavel Fedin <address@hidden>
> >>
> >> ARM GICv3 systems with large number of CPUs create lots of IRQ pins. Since
> >> every pin is represented as a property, number of these properties becomes
> >> very large. Every property add first makes sure there's no duplicates.
> >> Traversing the list becomes very slow, therefore qemu initialization takes
> >> significant time (several seconds for e. g. 16 CPUs).
> >>
> >> This patch replaces list with GHashTable, making lookup very fast. The only
> >> drawback is that object_child_foreach() and 
> >> object_child_foreach_recursive()
> >> cannot modify their objects during traversal, since GHashTableIter does not
> >> have modify-safe version. However, the code seems not to modify objects via
> >> these functions.
> >>
> >> Signed-off-by: Daniel P. Berrange <address@hidden>
> >> Signed-off-by: Pavel Fedin <address@hidden>
> > 
> > (note these seemed misordered)
> > 
> > I have queued things up to 6/7 on qom-next:
> > https://github.com/afaerber/qemu-cpu/commits/qom-next
> > 
> > This patch didn't apply and I had to hand-apply one hunk (which I
> > double-checked, but you never know).
> > 
> > Unfortunately I run into this test failure:
> > 
> > TEST: tests/device-introspect-test... (pid=4094)
> >   /s390x/device/introspect/list:                                       OK
> >   /s390x/device/introspect/none:                                       OK
> >   /s390x/device/introspect/abstract:                                   OK
> >   /s390x/device/introspect/concrete:
> > (process:4102): GLib-CRITICAL **: g_hash_table_iter_next: assertion
> > 'ri->version == ri->hash_table->version' failed
> > 
> > (process:4102): GLib-CRITICAL **: g_hash_table_iter_next: assertion
> > 'ri->version == ri->hash_table->version' failed
> > 
> > (process:4102): GLib-CRITICAL **: iter_remove_or_steal: assertion
> > 'ri->version == ri->hash_table->version' failed
> > **
> > ERROR:/home/andreas/QEMU/qemu/qom/object.c:867:object_unref: assertion
> > failed: (obj->ref > 0)
> > Broken pipe
> > FAIL
> > GTester: last random seed: R02S4fa2068506971129a7ebe2323dbe03b7
> > (pid=4104)
> > FAIL: tests/device-introspect-test
> > TEST: tests/qom-test... (pid=4105)
> >   /s390x/qom/s390-ccw-virtio-2.5:                                      OK
> >   /s390x/qom/s390-ccw-virtio-2.4:                                      OK
> >   /s390x/qom/none:                                                     OK
> >   /s390x/qom/s390-virtio:
> > WARNING
> > The s390-virtio machine (non-ccw) is deprecated.
> > It will be removed in 2.6. Please use s390-ccw-virtio
> > OK
> > PASS: tests/qom-test
> > 
> > Are you sure you tested all targets?
> > Any hunch where this might stem from?
> > 
> > The below patch reveals that the ref count is 0. Might be just a symptom
> > of the actual problem though.
> 
> A simpler reproducer is
> s390x-softmmu/qemu-system-s390x  -device sclp,help
> which fails with this patch and succeeds without.

The problems arise when unref'ing the object, which is where we iterate
over properties deleting them. From my investigation it seems we have
a re-entrancy issue where by child objects are calling back to the
parent to delete properties while iteration is taking place. The glib
hash iterators are not re-entrant safe, so this causes the glib
errors showing we're modifying the hash table while iteration is
taking place.

(qemu-system-s390x:21872): GLib-CRITICAL **: iter_remove_or_steal: assertion 
'ri->version == ri->hash_table->version' failed

I'm looking into how to fix this now...

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|