From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45264) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyI4s-00008f-Iv for qemu-devel@nongnu.org; Mon, 16 Nov 2015 06:36:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZyI4o-0003qQ-Ua for qemu-devel@nongnu.org; Mon, 16 Nov 2015 06:36:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40687) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyI4o-0003qB-Mn for qemu-devel@nongnu.org; Mon, 16 Nov 2015 06:36:02 -0500 Date: Mon, 16 Nov 2015 11:35:57 +0000 From: "Daniel P. Berrange" Message-ID: <20151116113557.GC20157@redhat.com> References: <1444739866-14798-1-git-send-email-berrange@redhat.com> <1444739866-14798-7-git-send-email-berrange@redhat.com> <5646286B.2030307@suse.de> <56464F8A.3070709@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <56464F8A.3070709@de.ibm.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 6/7] qom: replace object property list with GHashTable Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger Cc: Peter Maydell , Pavel Fedin , Markus Armbruster , qemu-devel@nongnu.org, Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= On Fri, Nov 13, 2015 at 10:00:58PM +0100, Christian Borntraeger wrote: > On 11/13/2015 07:14 PM, Andreas F=C3=A4rber wrote: > > Am 13.10.2015 um 14:37 schrieb Daniel P. Berrange: > >> From: Pavel Fedin > >> > >> 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 b= ecomes > >> very large. Every property add first makes sure there's no duplicate= s. > >> 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. T= he only > >> drawback is that object_child_foreach() and object_child_foreach_rec= ursive() > >> cannot modify their objects during traversal, since GHashTableIter d= oes not > >> have modify-safe version. However, the code seems not to modify obje= cts via > >> these functions. > >> > >> Signed-off-by: Daniel P. Berrange > >> Signed-off-by: Pavel Fedin > >=20 > > (note these seemed misordered) > >=20 > > I have queued things up to 6/7 on qom-next: > > https://github.com/afaerber/qemu-cpu/commits/qom-next > >=20 > > This patch didn't apply and I had to hand-apply one hunk (which I > > double-checked, but you never know). > >=20 > > Unfortunately I run into this test failure: > >=20 > > TEST: tests/device-introspect-test... (pid=3D4094) > > /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 =3D=3D ri->hash_table->version' failed > >=20 > > (process:4102): GLib-CRITICAL **: g_hash_table_iter_next: assertion > > 'ri->version =3D=3D ri->hash_table->version' failed > >=20 > > (process:4102): GLib-CRITICAL **: iter_remove_or_steal: assertion > > 'ri->version =3D=3D ri->hash_table->version' failed > > ** > > ERROR:/home/andreas/QEMU/qemu/qom/object.c:867:object_unref: assertio= n > > failed: (obj->ref > 0) > > Broken pipe > > FAIL > > GTester: last random seed: R02S4fa2068506971129a7ebe2323dbe03b7 > > (pid=3D4104) > > FAIL: tests/device-introspect-test > > TEST: tests/qom-test... (pid=3D4105) > > /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 > >=20 > > Are you sure you tested all targets? > > Any hunch where this might stem from? > >=20 > > The below patch reveals that the ref count is 0. Might be just a symp= tom > > of the actual problem though. >=20 > 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: assert= ion 'ri->version =3D=3D ri->hash_table->version' failed I'm looking into how to fix this now... Regards, Daniel --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|