From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39130) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1behfN-0007Bi-8g for qemu-devel@nongnu.org; Tue, 30 Aug 2016 07:57:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1behfK-00026V-Th for qemu-devel@nongnu.org; Tue, 30 Aug 2016 07:57:20 -0400 Date: Tue, 30 Aug 2016 14:57:08 +0300 From: "Michael S. Tsirkin" Message-ID: <20160830145633-mutt-send-email-mst@kernel.org> References: <1472526419-5900-1-git-send-email-jasowang@redhat.com> <1472526419-5900-3-git-send-email-jasowang@redhat.com> <20160830093127.49463d9d.cornelia.huck@de.ibm.com> <20160830130128-mutt-send-email-mst@kernel.org> <20160830130926-mutt-send-email-mst@kernel.org> <20160830131105.74f78a45.cornelia.huck@de.ibm.com> <20160830141341-mutt-send-email-mst@kernel.org> <20160830133741.776e8ff0.cornelia.huck@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160830133741.776e8ff0.cornelia.huck@de.ibm.com> Subject: Re: [Qemu-devel] qom and debug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: Jason Wang , qemu-devel@nongnu.org, pbonzini@redhat.com, peterx@redhat.com, wexu@redhat.com, vkaplans@redhat.com, Stefan Hajnoczi , Kevin Wolf , Amit Shah , qemu-block@nongnu.org On Tue, Aug 30, 2016 at 01:37:41PM +0200, Cornelia Huck wrote: > On Tue, 30 Aug 2016 14:15:14 +0300 > "Michael S. Tsirkin" wrote: > > > On Tue, Aug 30, 2016 at 01:11:05PM +0200, Cornelia Huck wrote: > > > On Tue, 30 Aug 2016 13:21:23 +0300 > > > "Michael S. Tsirkin" wrote: > > > > > > > BTW downstreams are building with --disable-qom-cast-debug which drops > > > > all QOM casts on data path - one way is to say we just make this the > > > > default upstream as well. Another to say that we want to distinguish > > > > fast path calls from slow path, this way we will be able to bring back > > > > some of the checks. > > > > > > I find CONFIG_QOM_CAST_DEBUG a bit inconsistent, btw: > > > > > > - for object casts, we optimize away all checks and just return the > > > object for !debug > > > - for class casts, we optimize away only the caching and still keep the > > > checking (why would we drop the caching if this can speed up things?) > > > > > > We certainly want to have debug turned on during development to avoid > > > nasty surprises later (otherwise, why even bother?), but it makes sense > > > to turn it off for a release. (Is there an easy way to turn it off for > > > the release, normal or stable, and keep it during the development > > > cycle?) > > > > I think the assumption was class casts are not on data path. > > Ideally we'd keep it on for release too for non-datapath things, > > to help improve security. > > This would probably need some more fine-grained configuration. We can start with Jason's patch :) > For now, what about this completely untested patch that at least adds > caching for the class->interfaces case if debug is off? Fine with me. > diff --git a/qom/object.c b/qom/object.c > index 8166b7d..05f1fe4 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -696,12 +696,16 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class, > const char *func) > { > ObjectClass *ret; > + int i; > > trace_object_class_dynamic_cast_assert(class ? class->type->name : "(null)", > typename, file, line, func); > > -#ifdef CONFIG_QOM_CAST_DEBUG > - int i; > +#ifndef CONFIG_QOM_CAST_DEBUG > + if (!class || !class->interfaces) { > + return class; > + } > +#endif > > for (i = 0; class && i < OBJECT_CLASS_CAST_CACHE; i++) { > if (class->class_cast_cache[i] == typename) { > @@ -709,11 +713,6 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class, > goto out; > } > } > -#else > - if (!class || !class->interfaces) { > - return class; > - } > -#endif > > ret = object_class_dynamic_cast(class, typename); > if (!ret && class) { > @@ -722,7 +721,6 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class, > abort(); > } > > -#ifdef CONFIG_QOM_CAST_DEBUG > if (class && ret == class) { > for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) { > class->class_cast_cache[i - 1] = class->class_cast_cache[i]; > @@ -730,7 +728,6 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class, > class->class_cast_cache[i - 1] = typename; > } > out: > -#endif > return ret; > } >