| Message ID | 20160830133741.776e8ff0.cornelia.huck@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
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" <mst@redhat.com> 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" <mst@redhat.com> 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; > } >
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; }