From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54273) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxLqN-0006vv-NS for qemu-devel@nongnu.org; Fri, 13 Nov 2015 16:25:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZxLqJ-0006Ot-MJ for qemu-devel@nongnu.org; Fri, 13 Nov 2015 16:25:15 -0500 Received: from mx2.suse.de ([195.135.220.15]:35370) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxLqJ-0006Ol-Ca for qemu-devel@nongnu.org; Fri, 13 Nov 2015 16:25:11 -0500 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> From: =?UTF-8?Q?Andreas_F=c3=a4rber?= Message-ID: <56465533.3030501@suse.de> Date: Fri, 13 Nov 2015 22:25:07 +0100 MIME-Version: 1.0 In-Reply-To: <56464F8A.3070709@de.ibm.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 6/7] qom: replace object property list with GHashTable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger Cc: Peter Maydell , Pavel Fedin , qemu-devel@nongnu.org, Markus Armbruster , Paolo Bonzini Am 13.11.2015 um 22:00 schrieb Christian Borntraeger: > On 11/13/2015 07:14 PM, Andreas F=E4rber 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 be= comes >>> 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. Th= e only >>> drawback is that object_child_foreach() and object_child_foreach_recu= rsive() >>> cannot modify their objects during traversal, since GHashTableIter do= es not >>> have modify-safe version. However, the code seems not to modify objec= ts via >>> these functions. >>> >>> Signed-off-by: Daniel P. Berrange >>> Signed-off-by: Pavel Fedin >> >> (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=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 >> >> (process:4102): GLib-CRITICAL **: g_hash_table_iter_next: assertion >> 'ri->version =3D=3D ri->hash_table->version' failed >> >> (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: assertion >> 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 >> >> 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 sympt= om >> 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. Thanks! sclp_init() seems to violate several QOM design principles in that it uses object_new() during TypeInfo::instance_init() and uses a TYPE_... constant as property name. But nothing else stands out immediate= ly. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Graham Norton; HRB 21284 (AG N=FC= rnberg)