From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53685 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OqZb7-0007dc-K7 for qemu-devel@nongnu.org; Tue, 31 Aug 2010 18:46:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OqZb3-0006g2-HL for qemu-devel@nongnu.org; Tue, 31 Aug 2010 18:46:30 -0400 Received: from mail-vw0-f45.google.com ([209.85.212.45]:51660) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OqZb3-0006fx-DK for qemu-devel@nongnu.org; Tue, 31 Aug 2010 18:46:29 -0400 Received: by vws19 with SMTP id 19so6136297vws.4 for ; Tue, 31 Aug 2010 15:46:28 -0700 (PDT) Message-ID: <4C7D8641.3080507@codemonkey.ws> Date: Tue, 31 Aug 2010 17:46:25 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <20100827223659.2696.3589.stgit@s20.home> <20100827223745.2696.18845.stgit@s20.home> <20100831202553.GD31570@redhat.com> <1283293974.5826.154.camel@x201> In-Reply-To: <1283293974.5826.154.camel@x201> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 5/5] virtio-net: Switch default to new bottom half TX handler for iothread List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: jes.sorensen@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, "Michael S. Tsirkin" On 08/31/2010 05:32 PM, Alex Williamson wrote: > On Tue, 2010-08-31 at 23:25 +0300, Michael S. Tsirkin wrote: > >> On Fri, Aug 27, 2010 at 04:37:45PM -0600, Alex Williamson wrote: >> >>> The bottom half handler shows big improvements over the timer >>> with few downsides, default to it when the iothread is enabled. >>> >>> Using the following tests, with the guest and host connected >>> via tap+bridge: >>> >>> guest> netperf -t TCP_STREAM -H $HOST >>> host> netperf -t TCP_STREAM -H $GUEST >>> guest> netperf -t UDP_STREAM -H $HOST >>> host> netperf -t UDP_STREAM -H $GUEST >>> guest> netperf -t TCP_RR -H $HOST >>> >>> Results: base throughput, exits/throughput -> >>> patched throughput, exits/throughput >>> >>> --enable-io-thread >>> TCP guest->host 2737.77, 47.82 -> 6767.09, 29.15 = 247%, 61% >>> TCP host->guest 2231.33, 74.00 -> 4125.80, 67.61 = 185%, 91% >>> UDP guest->host 6281.68, 14.66 -> 12569.27, 1.98 = 200%, 14% >>> UDP host->guest 275.91, 289.22 -> 264.80, 293.53 = 96%, 101% >>> interations/s 1949.65, 82.97 -> 7417.56, 84.31 = 380%, 102% >>> >>> No --enable-io-thread >>> TCP guest->host 3041.57, 55.11 -> 1038.93, 517.57 = 34%, 939% >>> TCP host->guest 2416.03, 76.67 -> 5655.92, 55.52 = 234%, 72% >>> UDP guest->host 12255.82, 6.11 -> 7775.87, 31.32 = 63%, 513% >>> UDP host->guest 587.92, 245.95 -> 611.88, 239.92 = 104%, 98% >>> interations/s 1975.59, 83.21 -> 8935.50, 88.18 = 452%, 106% >>> >>> Signed-off-by: Alex Williamson >>> >> parameter having different settings based on config >> options might surprise some users. I don't think >> we really need a parameter here ... >> > I'm not a bit fan of this either, but I'd also prefer not to introduce a > regression for a performance difference we know about in advance. It > gets even more complicated when we factor in qemu-kvm, as it doesn't > build with iothread enabled, but seems to get and even better boost in > performance across the board thanks largely to the kvm-irqchip. Should > we instead make this a configure option? --enable-virtio-net-txbh? > No, at this stage, we should ignore no --enable-io-thread with -enable-kvm. Regards, Anthony Liguori > Thanks, > > Alex > > >>> --- >>> >>> hw/s390-virtio-bus.c | 3 ++- >>> hw/syborg_virtio.c | 3 ++- >>> hw/virtio-pci.c | 3 ++- >>> hw/virtio.h | 6 ++++++ >>> 4 files changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c >>> index 1483362..985f99a 100644 >>> --- a/hw/s390-virtio-bus.c >>> +++ b/hw/s390-virtio-bus.c >>> @@ -328,7 +328,8 @@ static VirtIOS390DeviceInfo s390_virtio_net = { >>> .qdev.size = sizeof(VirtIOS390Device), >>> .qdev.props = (Property[]) { >>> DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic), >>> - DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer, 1), >>> + DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer, >>> + TXTIMER_DEFAULT), >>> DEFINE_PROP_INT32("txburst", VirtIOS390Device, txburst, 256), >>> DEFINE_PROP_END_OF_LIST(), >>> }, >>> diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c >>> index 7b76972..ee5746d 100644 >>> --- a/hw/syborg_virtio.c >>> +++ b/hw/syborg_virtio.c >>> @@ -300,7 +300,8 @@ static SysBusDeviceInfo syborg_virtio_net_info = { >>> .qdev.props = (Property[]) { >>> DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic), >>> DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features), >>> - DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer, 1), >>> + DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer, >>> + TXTIMER_DEFAULT), >>> DEFINE_PROP_INT32("txburst", SyborgVirtIOProxy, txburst, 256), >>> DEFINE_PROP_END_OF_LIST(), >>> } >>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >>> index e025c09..9740f57 100644 >>> --- a/hw/virtio-pci.c >>> +++ b/hw/virtio-pci.c >>> @@ -695,7 +695,8 @@ static PCIDeviceInfo virtio_info[] = { >>> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3), >>> DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features), >>> DEFINE_NIC_PROPERTIES(VirtIOPCIProxy, nic), >>> - DEFINE_PROP_UINT32("txtimer", VirtIOPCIProxy, txtimer, 1), >>> + DEFINE_PROP_UINT32("txtimer", VirtIOPCIProxy, txtimer, >>> + TXTIMER_DEFAULT), >>> DEFINE_PROP_INT32("txburst", VirtIOPCIProxy, txburst, 256), >>> DEFINE_PROP_END_OF_LIST(), >>> }, >>> diff --git a/hw/virtio.h b/hw/virtio.h >>> index 4051889..a1a17a2 100644 >>> --- a/hw/virtio.h >>> +++ b/hw/virtio.h >>> @@ -183,6 +183,12 @@ void virtio_update_irq(VirtIODevice *vdev); >>> void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding, >>> void *opaque); >>> >>> +#ifdef CONFIG_IOTHREAD >>> + #define TXTIMER_DEFAULT 0 >>> +#else >>> + #define TXTIMER_DEFAULT 1 >>> +#endif >>> + >>> >> Add a comment explaning that this is just a performance optimization? >> >> >>> /* Base devices. */ >>> VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf); >>> VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe kvm" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> > > >