From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41806) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsKdY-0007iz-9D for qemu-devel@nongnu.org; Mon, 07 Jan 2013 16:53:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TsKdX-0001FQ-3O for qemu-devel@nongnu.org; Mon, 07 Jan 2013 16:53:40 -0500 Received: from e9.ny.us.ibm.com ([32.97.182.139]:37126) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsKdW-0001FL-VP for qemu-devel@nongnu.org; Mon, 07 Jan 2013 16:53:39 -0500 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 7 Jan 2013 16:53:37 -0500 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 6F3D3C9003E for ; Mon, 7 Jan 2013 16:53:36 -0500 (EST) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r07LrZnf59637938 for ; Mon, 7 Jan 2013 16:53:36 -0500 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r07LrYHp015563 for ; Mon, 7 Jan 2013 19:53:35 -0200 From: Anthony Liguori In-Reply-To: <20130107213320.GA11305@redhat.com> References: <1357584074-10852-1-git-send-email-fred.konrad@greensocs.com> <1357584074-10852-20-git-send-email-fred.konrad@greensocs.com> <20130107211611.GE8679@redhat.com> <87hams1yn5.fsf@codemonkey.ws> <20130107213320.GA11305@redhat.com> Date: Mon, 07 Jan 2013 15:53:18 -0600 Message-ID: <87sj6cirsh.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 19/61] virtio-net : cleanup : use QOM cast. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: kwolf@redhat.com, peter.maydell@linaro.org, e.voevodin@samsung.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, agraf@suse.de, amit.shah@redhat.com, aneesh.kumar@linux.vnet.ibm.com, stefanha@redhat.com, cornelia.huck@de.ibm.com, pbonzini@redhat.com, afaerber@suse.de, fred.konrad@greensocs.com "Michael S. Tsirkin" writes: > On Mon, Jan 07, 2013 at 03:17:18PM -0600, Anthony Liguori wrote: >> "Michael S. Tsirkin" writes: >> >> > On Mon, Jan 07, 2013 at 07:40:32PM +0100, fred.konrad@greensocs.com wrote: >> >> @@ -130,7 +124,9 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) >> >> >> >> static void virtio_net_set_link_status(NetClientState *nc) >> >> { >> >> - VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque; >> >> + void *opaque = DO_UPCAST(NICState, nc, nc)->opaque; >> >> + VirtIONet *n = VIRTIO_NET(opaque); >> >> + VirtIODevice *vdev = VIRTIO_DEVICE(n); >> >> uint16_t old_status = n->status; >> >> >> >> if (nc->link_down) >> > >> > I note this adds more pointer chasing due to runtime casts on data path >> > operations. Can well be trivial but this really needs to be verified >> > with a performance test. Was this done? Same comment applies to block. >> > An alternative is to add _fast casts without runtime checks. >> >> I'm pretty sure other things like the one big global mutex are more >> important than the number of pointer dereferences... > > True. But we have plans to fix that, right? > >> Do you have any evidence to suggest that this would be a problem in practice? >> >> Regards, >> >> Anthony Liguori > > No. It's not this change specifically, I'm generally concerned if we let > ourselves bloat datapath in an uncontrolled way, the waste will add up > over time. I don't think a pointer indirection can be called "bloat[ing] datapath in an uncontrolled way". We shouldn't focus on minor things like this, but rather on things like notification mitigation and lock contention which we know has a direct impact on performance. Regards, Anthony Liguori > >> > >> > -- >> > MST