From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38438) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dv0EH-0000gE-J2 for qemu-devel@nongnu.org; Thu, 21 Sep 2017 08:05:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dv0EB-0007Uq-IO for qemu-devel@nongnu.org; Thu, 21 Sep 2017 08:05:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39916) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dv0EB-0007UF-BJ for qemu-devel@nongnu.org; Thu, 21 Sep 2017 08:05:11 -0400 Date: Thu, 21 Sep 2017 14:05:06 +0200 From: Cornelia Huck Message-ID: <20170921140506.42c5f7f6.cohuck@redhat.com> In-Reply-To: References: <20170920172314.102710-1-pasic@linux.vnet.ibm.com> <20170920172314.102710-2-pasic@linux.vnet.ibm.com> <20170921111552.41167e80.cohuck@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via CcwDataStream List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Halil Pasic Cc: qemu-devel@nongnu.org, Alexander Graf , Christian Borntraeger , "Jason J . Herne" , Dong Jia Shi , Richard Henderson On Thu, 21 Sep 2017 13:22:44 +0200 Halil Pasic wrote: > On 09/21/2017 11:15 AM, Cornelia Huck wrote: > >> +static inline CcwDataStream *get_cds(Terminal3270 *t) > >> +{ > >> + return &(CCW_DEVICE(&t->cdev)->sch->cds); > >> +} > >> + > >> +static int read_payload_3270(EmulatedCcw3270Device *dev) > >> { > >> Terminal3270 *t = TERMINAL_3270(dev); > >> int len; > >> > >> - len = MIN(count, t->in_len); > >> - cpu_physical_memory_write(cda, t->inv, len); > >> + len = MIN(ccw_dstream_avail(get_cds(t)), t->in_len); > >> + ccw_dstream_write_buf(get_cds(t), t->inv, len); > > CCW_DEVICE() as called by get_cds() goes through qom, which implies a > > bit of overhead. Not sure if it makes sense to cache it in this > > function so you don't go through it multiple times. (Dito for the other > > callback.) > > > > I've cargo-culted this way of getting CCW_DEVICE(&t->cdev) to the CcwDevice > form terminal_read (the pattern used at multiple places in the file). As > far as I can tell, the overhead basically depends on CONFIG_QOM_CAST_DEBUG. > > I have no idea what do we have in production, but my guess is, that > ONFIG_QOM_CAST_DEBUG makes only sense for development and testing > (especially if proper test coverage is assumed). > Can you enlighten me? > > CCW_DEVICE() may contain a run-time check (depending on CONFIG_QOM_CAST_DEBUG), > we however can make sure things are OK at compile time. This brings > me to the next question. Does it even make sense to use OBJECT_CHECK based > constructs when going from specific to general (we don't actually need a > cast here)? Obviously, for the other direction we really need a cast, so doing > a run-time check there does indeed provide added value. The basic rule seems to be "use a qom cast, unless you are on a fast path" - even though qom debug makes the most sense while developing. But let's not turn that into a big discussion: It's probably not really worth optimizing here.