All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: qemu-devel <qemu-devel@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>
Subject: QOM address space handling
Date: Tue, 10 Nov 2020 11:14:39 +0000	[thread overview]
Message-ID: <0ad53d69-ce4a-c5ea-fba4-fa19daada11c@ilande.co.uk> (raw)

Hi all,

This email follows on from my investigation of intermittent Travis-CI failures in 
make check's device-introspect test when trying to add the patch at 
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg06093.html to my last 
qemu-sparc pull request.

The patch itself seems fairly harmless: moving the sun4u-iommu device as a QOM child 
of the sabre PCI host bridge device. So why was "make check" randomly segfaulting on 
Travis-CI?

The hardest part was trying to reproduce the issue to debug it: eventually after a 
number of Travis-CI runs I discovered I could generate the same problem locally if I 
ran "make check" around 15-20 times in a row, and that gave me a backtrace that 
looked like this:

0x0000000000614b69 in address_space_init (as=0x16f684d8,
root=0x16f68530, name=0x9a1db2 "iommu-as") at ../softmmu/memory.c:2780
2780        QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
(gdb) bt
#0  0x0000000000614b69 in address_space_init (as=0x16f684d8,
  root=0x16f68530, name=0x9a1db2 "iommu-as") at
../softmmu/memory.c:2780
#1  0x00000000005b8f6a in iommu_init (obj=0x16f681c0) at ../hw/sparc64/sun4u_iommu.c:301
#2  0x000000000070a997 in object_init_with_type (obj=0x16f681c0,
  ti=0x1629fac0) at ../qom/object.c:375

With the debugger attached I was able to figure out what was happening: the 
sun4u-iommu device creates the iommu-as address space during instance init, but 
doesn't have a corresponding instance finalize to remove it which leaves a dangling 
pointer in the address_spaces QTAILQ.

Normally this doesn't matter because IOMMUs are created once during machine init, but 
device-introspect-test instantiates sun4u-iommu (and with the patch sabre also adds 
it as a child object during instance init) which adds more dangling pointers to the 
address_spaces list. Every so often the dangling pointers end up pointing to memory 
that gets reused by another QOM object, eventually causing random segfaults during 
instance finalize and/or property iteration.

There are 2 possible solutions here: 1) ensure QOM objects that add address spaces 
during instance init have a corresponding instance finalize function to remove them 
or 2) move the creation of address spaces from instance init to realize.

Does anyone have any arguments for which solution is preferred?

As part of this work I hacked up an address_space_count() function in memory.c that 
returns the size of the address_spaces QTAILQ and added a printf() to display the 
value during instance init and finalize which demonstrates the problem nicely. This 
means it should be possible to add a similar to check to device-introspect-test in 
future to prevent similar errors from happening again.


ATB,

Mark.


             reply	other threads:[~2020-11-10 11:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 11:14 Mark Cave-Ayland [this message]
2020-11-10 11:40 ` QOM address space handling Paolo Bonzini
2020-12-18  7:49   ` Mark Cave-Ayland
2020-12-18 22:32     ` Eduardo Habkost
2020-12-20  9:25       ` Paolo Bonzini
2020-12-21 18:54         ` Eduardo Habkost
2020-12-21 19:16           ` Paolo Bonzini
2020-12-21 19:28             ` Eduardo Habkost
2020-11-10 15:03 ` Eduardo Habkost
2020-11-10 15:08   ` Paolo Bonzini
2020-11-10 17:46     ` Eduardo Habkost
2020-11-10 18:36       ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0ad53d69-ce4a-c5ea-fba4-fa19daada11c@ilande.co.uk \
    --to=mark.cave-ayland@ilande.co.uk \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.