qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: mst@redhat.com, "Andreas Färber" <afaerber@suse.de>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases
Date: Fri, 10 May 2013 16:59:52 +0200	[thread overview]
Message-ID: <518D0B68.505@redhat.com> (raw)
In-Reply-To: <871u9ezypj.fsf@codemonkey.ws>

Il 10/05/2013 16:39, Anthony Liguori ha scritto:
> I just oppose the notion of disabling casts and *especially* only
> disabling casts for official builds.

This actually happens all the time.  Exactly this kind of type-safe cast
is disabled in releases of GCC, but enabled when building from svn trunk.

I have hardly seen any of these failures _during development_, much less
on a release.  I appreciate the advantage of type-safe casts, but in
QEMU they are a solution in search of a problem.  They are cheap to
implement (though not that cheap to execute ;)) so it's perfectly fine
to have them, but they are not _needed_; disabling them is anyway a good
build-time option to have.

Type-safe casts make sense in GTK+/GObject where: 1) type-safe casts
return NULL and log a "critical" error, they do not abort; 2) all
functions fail with another "critical" error if they are passed NULL.
We do neither, so we're just trading a crash now for a crash very soon
after (our call stacks tend to be relatively shallow, with some
exceptions such as the block layer).

Also, in GTK+/GObject the code paths are unpredictable because they
depend on user interaction, and a crash can lead to data loss.  By
contrast, in QEMU most of the code is hardly ever run, and the possible
paths are very few because driver writers tend to use always the same
path.  The day someone is bringing up a new guest OS and encounters such
a crash, we'll tell them to either build from git, or to use
--enable-qom-cast-debug.

I'm sure it will be a long time before that happens...

Paolo

> Regards,
> 
> Anthony Liguori
> 
>>> Either way, it would be nice to see the call sites of those
>>> most-impacting dynamic casts! So far I held back my APIC RFC since I'm
>>> not sure how to reproducibly profile things.
>>
>> It's interrupts (both sending and returning from them).
>>
>> Paolo
> 
> 
> 

  reply	other threads:[~2013-05-10 15:00 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-10 12:16 [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 1/9] qom: improve documentation of cast functions Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 2/9] qom: allow casting of a NULL class Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 3/9] qom: add a fast path to object_class_dynamic_cast Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 4/9] qom: pass file/line/function to asserting casts Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 5/9] qom: trace " Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 6/9] qom: allow turning cast debugging off Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 7/9] build: disable QOM cast debugging for official releases Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 8/9] qom: simplify object_class_dynamic_cast, part 1 Paolo Bonzini
2013-05-10 17:52   ` Anthony Liguori
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 9/9] qom: simplify object_class_dynamic_cast, part 2 Paolo Bonzini
2013-05-10 13:01 ` [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases Anthony Liguori
2013-05-10 13:08   ` Paolo Bonzini
2013-05-10 13:23     ` Andreas Färber
2013-05-10 13:30       ` Paolo Bonzini
2013-05-10 14:22         ` Aurelien Jarno
2013-05-10 14:39         ` Anthony Liguori
2013-05-10 14:59           ` Paolo Bonzini [this message]
2013-05-10 17:41             ` Anthony Liguori
2013-05-10 19:00               ` Aurelien Jarno
2013-05-10 19:35                 ` Anthony Liguori
2013-05-10 20:59               ` Paolo Bonzini
2013-05-10 23:06                 ` Anthony Liguori
2013-05-11  7:59                   ` Paolo Bonzini
2013-05-11  7:23                 ` Aurelien Jarno
2013-05-10 14:27     ` Anthony Liguori
2013-05-10 14:46       ` Aurelien Jarno
2013-05-10 15:04         ` Andreas Färber
2013-05-10 15:56 ` Aurelien Jarno
2013-05-10 16:18   ` Andreas Färber
2013-05-10 16:40     ` Paolo Bonzini
2013-05-10 18:50   ` Anthony Liguori
2013-05-13 16:46 ` Anthony Liguori

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=518D0B68.505@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=afaerber@suse.de \
    --cc=aliguori@us.ibm.com \
    --cc=aurelien@aurel32.net \
    --cc=mst@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).