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

[Andreas Färber]

Am 13.11.2015 um 22:00 schrieb Christian Borntraeger:
> 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.

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.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)