From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53932) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyExY-0001Cl-HK for qemu-devel@nongnu.org; Mon, 16 Nov 2015 03:16:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZyExT-0003wh-F3 for qemu-devel@nongnu.org; Mon, 16 Nov 2015 03:16:20 -0500 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:52387) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyExT-0003wN-3k for qemu-devel@nongnu.org; Mon, 16 Nov 2015 03:16:15 -0500 Received: from localhost by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 16 Nov 2015 08:16:12 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id D404B1B0805F for ; Mon, 16 Nov 2015 08:16:29 +0000 (GMT) Received: from d06av07.portsmouth.uk.ibm.com (d06av07.portsmouth.uk.ibm.com [9.149.37.248]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id tAG8GAfQ2621708 for ; Mon, 16 Nov 2015 08:16:10 GMT Received: from d06av07.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av07.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id tAG8GAP3028444 for ; Mon, 16 Nov 2015 01:16:10 -0700 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> <56465533.3030501@suse.de> <008001d1203e$51838510$f48a8f30$@samsung.com> From: Christian Borntraeger Message-ID: <564990C9.1060801@de.ibm.com> Date: Mon, 16 Nov 2015 09:16:09 +0100 MIME-Version: 1.0 In-Reply-To: <008001d1203e$51838510$f48a8f30$@samsung.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Pavel Fedin , =?UTF-8?Q?'Andreas_F=c3=a4rber'?= Cc: 'Peter Maydell' , qemu-devel@nongnu.org, 'Markus Armbruster' , David Hildenbrand , Cornelia Huck , 'Paolo Bonzini' On 11/16/2015 08:13 AM, Pavel Fedin wrote: > Hello! > >>>> (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 > > Wow... Actually this may come from attempts to modify the tree inside iteration. > >> 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 immediately. > > I think we should refactor this and retry. If not all problems go away, then we are indeed modifying the tree during iteration, and > we have to find some solution. David, Conny, the current tree of afaerber https://github.com/afaerber/qemu-cpu/commits/qom-next has this patch: > 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 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 > Signed-off-by: Pavel Fedin which causes failures in make check. A simple reproducer is qemu-system-s390x -device sclp,help any idea what would be the most simple fix? Can we refactor this to create the event facility and the bus in the machine or whatever? > I wonder... Could we have both list and hashtable? hashtable for searching by name and list for iteration. In this case we would > not have to use glib's iterators, and would be free of problems with them. Just keep the list and hashtable in sync. > Or, is there any hashtable implementation out there which would keep iterators valid during modification? > OTOH, glib has a function "remove the element at iterator's position", and we could postpone addition. So, perhaps, using both > containers would be an overkill, just refactor the code to adapt to the new behavior.