On Thu, Sep 06, 2012 at 10:32:48AM +0930, Rusty Russell wrote: > Sasha Levin <levinsasha928@xxxxxxxxx> writes: > >> On Wed, Aug 29, 2012 at 05:03:03PM +0200, Sasha Levin wrote: > >>> I've also re-ran it on a IBM server type host instead of my laptop. Here are the > >>> results: > >>> > >>> Vanilla kernel: > >>> > >>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.33.1 > >>> () port 0 AF_INET > >>> enable_enobufs failed: getprotobyname > >>> Recv Send Send > >>> Socket Socket Message Elapsed > >>> Size Size Size Time Throughput > >>> bytes bytes bytes secs. 10^6bits/sec > >>> > >>> 87380 16384 16384 10.00 7922.72 > >>> > >>> Patch 1, with threshold=16: > >>> > >>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.33.1 > >>> () port 0 AF_INET > >>> enable_enobufs failed: getprotobyname > >>> Recv Send Send > >>> Socket Socket Message Elapsed > >>> Size Size Size Time Throughput > >>> bytes bytes bytes secs. 10^6bits/sec > >>> > >>> 87380 16384 16384 10.00 8415.07 > >>> > >>> Patch 2: > >>> > >>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.33.1 > >>> () port 0 AF_INET > >>> enable_enobufs failed: getprotobyname > >>> Recv Send Send > >>> Socket Socket Message Elapsed > >>> Size Size Size Time Throughput > >>> bytes bytes bytes secs. 10^6bits/sec > >>> > >>> 87380 16384 16384 10.00 8931.05 > >>> > >>> > >>> Note that these are simple tests with netperf listening on one end and a simple > >>> 'netperf -H [host]' within the guest. If there are other tests which may be > >>> interesting please let me know. > > It might be worth just unconditionally having a cache for the 2 > descriptor case. This is what I get with qemu tap, though for some > reason the device features don't have guest or host CSUM, so my setup is > probably screwed: Yes without checksum net core always linearizes packets, so yes it is screwed. For -net, skb always allocates space for 17 frags + linear part so it seems sane to do same in virtio core, and allocate, for -net, up to max_frags + 1 from cache. We can adjust it: no _SG -> 2 otherwise 18. Not sure about other drivers, maybe really use 2 there for now. > Queue histogram for virtio0: > Size distribution for input (max=128427): > 1: 128427 ################################################################ > Size distribution for output (max=256485): > 2: 256485 ################################################################ > Size distribution for control (max=10): > 3: 10 ################################################################ > 4: 5 ################################ > > Here's a patch, what do you get (run ifconfig to trigger the dump; yeah, > it's a hack!) > > Hack: histogram of buffer sizes for virtio devices. > > Currently triggered by a stats query (eg ifconfig) on a net device. > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -727,6 +727,8 @@ static struct rtnl_link_stats64 *virtnet > tot->rx_length_errors = dev->stats.rx_length_errors; > tot->rx_frame_errors = dev->stats.rx_frame_errors; > > + virtio_dev_dump_histogram(vi->vdev); > + > return tot; > } > > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c > --- a/drivers/virtio/virtio.c > +++ b/drivers/virtio/virtio.c > @@ -108,6 +108,16 @@ void virtio_check_driver_offered_feature > } > EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature); > > +void virtio_dev_dump_histogram(const struct virtio_device *vdev) > +{ > + const struct virtqueue *vq; > + > + printk("Queue histogram for %s:\n", dev_name(&vdev->dev)); > + list_for_each_entry(vq, &vdev->vqs, list) > + virtqueue_dump_histogram(vq); > +} > +EXPORT_SYMBOL_GPL(virtio_dev_dump_histogram); > + > static int virtio_dev_probe(struct device *_d) > { > int err, i; > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -120,6 +120,8 @@ struct vring_virtqueue > ktime_t last_add_time; > #endif > > + unsigned int *histo; > + > /* Tokens for callbacks. */ > void *data[]; > }; > @@ -259,6 +261,8 @@ int virtqueue_add_buf(struct virtqueue * > BUG_ON(out + in > vq->vring.num); > BUG_ON(out + in == 0); > > + vq->histo[out+in]++; > + > /* If the host supports indirect descriptor tables, consider it. */ > if (vq->indirect) { > bool try_indirect; > @@ -726,6 +730,7 @@ struct virtqueue *vring_new_virtqueue(un > } > vq->data[i] = NULL; > > + vq->histo = kzalloc(num * sizeof(vq->histo[0]), GFP_KERNEL); > return &vq->vq; > } > EXPORT_SYMBOL_GPL(vring_new_virtqueue); > @@ -772,4 +777,33 @@ unsigned int virtqueue_get_vring_size(st > } > EXPORT_SYMBOL_GPL(virtqueue_get_vring_size); > > +void virtqueue_dump_histogram(const struct virtqueue *_vq) > +{ > + const struct vring_virtqueue *vq = to_vvq(_vq); > + int i, j, start = 0, end = 0, max = 1; > + char line[120]; > + > + for (i = 0; i < vq->vring.num; i++) { > + if (!vq->histo[i]) > + continue; > + > + end = i; > + if (!vq->histo[start]) > + start = i; > + > + if (vq->histo[i] > max) > + max = vq->histo[i]; > + } > + > + printk("Size distribution for %s (max=%u):\n", _vq->name, max); > + for (i = start; i <= end; i++) { > + unsigned int off; > + off = sprintf(line, "%3u: %-7u ", i, vq->histo[i]); > + for (j = 0; j < vq->histo[i] * 64 / max; j++) > + line[off++] = '#'; > + line[off] = '\0'; > + printk("%s\n", line); > + } > +} > + > MODULE_LICENSE("GPL"); > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > --- a/include/linux/virtio.h > +++ b/include/linux/virtio.h > @@ -52,6 +52,9 @@ unsigned int virtqueue_get_vring_size(st > > int virtqueue_get_queue_index(struct virtqueue *vq); > > +void virtio_dev_dump_histogram(const struct virtio_device *vdev); > +void virtqueue_dump_histogram(const struct virtqueue *vq); > + > /** > * virtio_device - representation of a device using virtio > * @index: unique position on the virtio bus -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html