From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIgio-0007Ot-Rq for qemu-devel@nongnu.org; Mon, 11 Jan 2016 12:57:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aIgil-0004RC-LR for qemu-devel@nongnu.org; Mon, 11 Jan 2016 12:57:38 -0500 Received: from mx2.suse.de ([195.135.220.15]:39873) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIgil-0004Qe-Aq for qemu-devel@nongnu.org; Mon, 11 Jan 2016 12:57:35 -0500 References: <1452087782-980-1-git-send-email-caoj.fnst@cn.fujitsu.com> <568E320E.9@redhat.com> <568E35A7.4060403@cn.fujitsu.com> <568E5476.9010002@redhat.com> <87lh7wt3u7.fsf@blackfin.pond.sub.org> From: =?UTF-8?Q?Andreas_F=c3=a4rber?= Message-ID: <5693ED0D.2080504@suse.de> Date: Mon, 11 Jan 2016 18:57:33 +0100 MIME-Version: 1.0 In-Reply-To: <87lh7wt3u7.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] SCSI bus: fix to incomplete QOMify List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Paolo Bonzini , Cao jin , qemu-devel@nongnu.org Am 11.01.2016 um 18:37 schrieb Markus Armbruster: > Paolo Bonzini writes: >> On 07/01/2016 10:53, Cao jin wrote: >>> On 01/07/2016 05:38 PM, Paolo Bonzini wrote: >>>> On 06/01/2016 14:43, Cao jin wrote: >>>> These functions are called in the data path; changes to use SCSI_BUS= () >>>> should come with performance data proving that it doesn't slow down = I/O. >>> >>> I see. I am not familiar with the procedure of scsi i/o performance >>> test, do have some guideline about it? >> >> Usually people test with FIO, but I think it's simpler to just keep >> DO_UPCAST here. >=20 > DO_UPCAST() needs to die. >=20 > SCSI_BUS() is a readable wrapper around OBJECT_CHECK(): >=20 > #define SCSI_BUS(obj) OBJECT_CHECK(SCSIBus, (obj), TYPE_SCSI_BUS) >=20 > OBJECT_CHECK() is semantically a type cast, but in actual code it does > more: >=20 > #define OBJECT_CHECK(type, obj, name) \ > ((type *)object_dynamic_cast_assert(OBJECT(obj), (name), \ > __FILE__, __LINE__, __func_= _)) >=20 > Object *object_dynamic_cast_assert(Object *obj, const char *typenam= e, > const char *file, int line, cons= t char *func) > { > trace_object_dynamic_cast_assert(obj ? obj->class->type->name := "(null)", > typename, file, line, func); >=20 > #ifdef CONFIG_QOM_CAST_DEBUG > [...] > #endif > return obj; > } >=20 > If CONFIG_QOM_CAST_DEBUG is on, it checks. That's a feature. >=20 > If CONFIG_QOM_CAST_DEBUG is off, it still calls to trace. That might > be a misfeature. Ugh, I was not aware of that tracing! Also, might it make sense to make that ..._assert() function inline? > If OBJECT_CHECK() isn't fit for fast paths because of that, perhaps QOM > should provide a conversion macro that is. Andreas, what do you think? We had a similar issue with virtio fast paths. I'm not sure whether we kept DO_UPCAST() or used a direct (Foo *) cast though. I think mst was a fan of upcast, and I used the latter in TCG innards. The general idea (from Anthony) was that derived types should not know about the implementation detail that the struct has a field of a certain name (for example, a switch to C++ classes or something would make it go away). So there's two issues mixed here, a) going from foo(x,y,z) to just bar(x) and b) the type checking as part of bar(x). If you want to propose a FOO_FAST() or whatever, I'm all ears. Cheers, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Graham Norton; HRB 21284 (AG N=FC= rnberg)