Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-06-01  9:50 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1306921434.git.mst@redhat.com>

Current code might introduce a lot of latency variation
if there are many pending bufs at the time we
attempt to transmit a new one. This is bad for
real-time applications and can't be good for TCP either.

Free up just enough to both clean up all buffers
eventually and to be able to xmit the next packet.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |   62 ++++++++++++++++++++++++++--------------------
 1 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a0ee78d..6045510 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,17 +509,27 @@ again:
 	return received;
 }
 
-static void free_old_xmit_skbs(struct virtnet_info *vi)
+/* Check capacity and try to free enough pending old buffers to enable queueing
+ * new ones.  If min_skbs > 0, try to free at least the specified number of skbs
+ * even if the ring already has sufficient capacity.  Return true if we can
+ * guarantee that a following virtqueue_add_buf will succeed. */
+static bool free_old_xmit_skbs(struct virtnet_info *vi, int min_skbs)
 {
 	struct sk_buff *skb;
 	unsigned int len;
+	bool r;
 
-	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+	while ((r = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2) ||
+	       min_skbs-- > 0) {
+		skb = virtqueue_get_buf(vi->svq, &len);
+		if (unlikely(!skb))
+			break;
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
 		dev_kfree_skb_any(skb);
 	}
+	return r;
 }
 
 static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -572,27 +582,24 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
 static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
-	int capacity;
+	int ret, n;
 
-	/* Free up any pending old buffers before queueing new ones. */
-	free_old_xmit_skbs(vi);
+	/* Free up space in the ring in case this is the first time we get
+	 * woken up after ring full condition.  Note: this might try to free
+	 * more than strictly necessary if the skb has a small
+	 * number of fragments, but keep it simple. */
+	free_old_xmit_skbs(vi, 0);
 
 	/* Try to transmit */
-	capacity = xmit_skb(vi, skb);
-
-	/* This can happen with OOM and indirect buffers. */
-	if (unlikely(capacity < 0)) {
-		if (net_ratelimit()) {
-			if (likely(capacity == -ENOMEM)) {
-				dev_warn(&dev->dev,
-					 "TX queue failure: out of memory\n");
-			} else {
-				dev->stats.tx_fifo_errors++;
-				dev_warn(&dev->dev,
-					 "Unexpected TX queue failure: %d\n",
-					 capacity);
-			}
-		}
+	ret = xmit_skb(vi, skb);
+
+	/* Failure to queue is unlikely. It's not a bug though: it might happen
+	 * if we get an interrupt while the queue is still mostly full.
+	 * We could stop the queue and re-enable callbacks (and possibly return
+	 * TX_BUSY), but as this should be rare, we don't bother. */
+	if (unlikely(ret < 0)) {
+		if (net_ratelimit())
+			dev_info(&dev->dev, "TX queue failure: %d\n", ret);
 		dev->stats.tx_dropped++;
 		kfree_skb(skb);
 		return NETDEV_TX_OK;
@@ -603,15 +610,16 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	skb_orphan(skb);
 	nf_reset(skb);
 
-	/* Apparently nice girls don't return TX_BUSY; stop the queue
-	 * before it gets out of hand.  Naturally, this wastes entries. */
-	if (capacity < 2+MAX_SKB_FRAGS) {
+	/* Apparently nice girls don't return TX_BUSY; check capacity and stop
+	 * the queue before it gets out of hand.
+	 * Naturally, this wastes entries. */
+	/* We transmit one skb, so try to free at least two pending skbs.
+	 * This is so that we don't hog the skb memory unnecessarily. */
+	if (!likely(free_old_xmit_skbs(vi, 2))) {
 		netif_stop_queue(dev);
 		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
-			/* More just got used, free them then recheck. */
-			free_old_xmit_skbs(vi);
-			capacity = virtqueue_min_capacity(vi->svq);
-			if (capacity >= 2+MAX_SKB_FRAGS) {
+			/* More just got used, free them and recheck. */
+			if (!likely(free_old_xmit_skbs(vi, 0))) {
 				netif_start_queue(dev);
 				virtqueue_disable_cb(vi->svq);
 			}
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH RFC 2/3] virtio_net: fix tx capacity checks using new API
From: Michael S. Tsirkin @ 2011-06-01  9:49 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1306921434.git.mst@redhat.com>

In the (rare) case where new descriptors are used
while virtio_net enables vq callback for the TX vq,
virtio_net uses the number of sg entries in the skb it frees to
calculate how many descriptors in the ring have just been made
available.  But this value is an overestimate: with indirect buffers
each skb only uses one descriptor entry, meaning we may wake the queue
only to find we still can't transmit anything.

Using the new virtqueue_min_capacity() call, we can determine
the remaining capacity, so we should use that instead.
This estimation is worst-case which is consistent with
the value returned by virtqueue_add_buf.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f685324..a0ee78d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,19 +509,17 @@ again:
 	return received;
 }
 
-static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
+static void free_old_xmit_skbs(struct virtnet_info *vi)
 {
 	struct sk_buff *skb;
-	unsigned int len, tot_sgs = 0;
+	unsigned int len;
 
 	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
-		tot_sgs += skb_vnet_hdr(skb)->num_sg;
 		dev_kfree_skb_any(skb);
 	}
-	return tot_sgs;
 }
 
 static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -611,7 +609,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_stop_queue(dev);
 		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
-			capacity += free_old_xmit_skbs(vi);
+			free_old_xmit_skbs(vi);
+			capacity = virtqueue_min_capacity(vi->svq);
 			if (capacity >= 2+MAX_SKB_FRAGS) {
 				netif_start_queue(dev);
 				virtqueue_disable_cb(vi->svq);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH RFC 1/3] virtio_ring: add capacity check API
From: Michael S. Tsirkin @ 2011-06-01  9:49 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1306921434.git.mst@redhat.com>

Add API to check ring capacity. Because of the option
to use indirect buffers, this returns the worst
case, not the normal case capacity.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_ring.c |    8 ++++++++
 include/linux/virtio.h       |    5 +++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 68b9136..23422f1 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -344,6 +344,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_buf);
 
+int virtqueue_min_capacity(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	return vq->num_free;
+}
+EXPORT_SYMBOL_GPL(virtqueue_min_capacity);
+
 void virtqueue_disable_cb(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7108857..209220d 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -42,6 +42,9 @@ struct virtqueue {
  *	vq: the struct virtqueue we're talking about.
  *	len: the length written into the buffer
  *	Returns NULL or the "data" token handed to add_buf.
+ * virtqueue_min_capacity: get the current capacity of the queue
+ *	vq: the struct virtqueue we're talking about.
+ *	Returns the current worst case capacity of the queue.
  * virtqueue_disable_cb: disable callbacks
  *	vq: the struct virtqueue we're talking about.
  *	Note that this is not necessarily synchronous, hence unreliable and only
@@ -89,6 +92,8 @@ void virtqueue_kick(struct virtqueue *vq);
 
 void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
 
+int virtqueue_min_capacity(struct virtqueue *vq);
+
 void virtqueue_disable_cb(struct virtqueue *vq);
 
 bool virtqueue_enable_cb(struct virtqueue *vq);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH RFC 0/3] virtio and vhost-net capacity handling
From: Michael S. Tsirkin @ 2011-06-01  9:49 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390

OK, here's a new attempt to use the new capacity api.  I also added more
comments to clarify the logic.  Hope this is more readable.  Let me know
pls.

This is on top of the patches applied by Rusty.

Note: there are now actually 2 calls to fee_old_xmit_skbs on
data path so instead of passing flag '2' to the
second one I thought we can just make each call free up
at least 1 skb. This will work and even might be a bit faster (less
branches), but in the end I discarded this idea as too fragile (relies
on two calls on data path to function properly).

Warning: untested. Posting now to give people chance to
comment on the API.

Michael S. Tsirkin (3):
  virtio_ring: add capacity check API
  virtio_net: fix tx capacity checks using new API
  virtio_net: limit xmit polling

 drivers/net/virtio_net.c     |   65 +++++++++++++++++++++++------------------
 drivers/virtio/virtio_ring.c |    8 +++++
 include/linux/virtio.h       |    5 +++
 3 files changed, 49 insertions(+), 29 deletions(-)

-- 
1.7.5.53.gc233e

^ permalink raw reply

* Re: [PATCH 1/1] [virt] virtio-blk: Use ida to allocate disk index
From: Mark Wu @ 2011-06-01  8:25 UTC (permalink / raw)
  To: Mark Wu; +Cc: virtualization, linux-kernel, kvm, Michael S. Tsirkin
In-Reply-To: <1306913069-23637-1-git-send-email-dwu@redhat.com>

On 06/01/2011 03:24 AM, Mark Wu wrote:
> -	if (index_to_minor(index)>= 1<<  MINORBITS)
> -		return -ENOSPC;
> +	do {
> +		if (!ida_pre_get(&vd_index_ida, GFP_KERNEL))
> +			return err;
> +
There's a problem in above code: err is not initialized before using, so 
change it to return -1;
+       do {
+               if (!ida_pre_get(&vd_index_ida, GFP_KERNEL))
+                       return -1;

^ permalink raw reply

* [PATCH 1/1] [virt] virtio-blk: Use ida to allocate disk index
From: Mark Wu @ 2011-06-01  7:24 UTC (permalink / raw)
  To: Rusty Russell, Michael S. Tsirkin
  Cc: Mark Wu, linux-kernel, kvm, virtualization

Current index allocation in virtio-blk is based on a monotonically
increasing variable "index". It could cause some confusion about disk
name in the case of hot-plugging disks. And it's impossible to find the
lowest available index by just maintaining a simple index. So it's
changed to use ida to allocate index via referring to the index
allocation in scsi disk.

Signed-off-by: Mark Wu <dwu@redhat.com>
---
 drivers/block/virtio_blk.c |   37 ++++++++++++++++++++++++++++++++-----
 1 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 079c088..ba734b3 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -8,10 +8,14 @@
 #include <linux/scatterlist.h>
 #include <linux/string_helpers.h>
 #include <scsi/scsi_cmnd.h>
+#include <linux/idr.h>
 
 #define PART_BITS 4
 
-static int major, index;
+static int major;
+static DEFINE_SPINLOCK(vd_index_lock);
+static DEFINE_IDA(vd_index_ida);
+
 struct workqueue_struct *virtblk_wq;
 
 struct virtio_blk
@@ -23,6 +27,7 @@ struct virtio_blk
 
 	/* The disk structure for the kernel. */
 	struct gendisk *disk;
+	u32 index;
 
 	/* Request tracking. */
 	struct list_head reqs;
@@ -343,12 +348,26 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 	struct request_queue *q;
 	int err;
 	u64 cap;
-	u32 v, blk_size, sg_elems, opt_io_size;
+	u32 v, blk_size, sg_elems, opt_io_size, index;
 	u16 min_io_size;
 	u8 physical_block_exp, alignment_offset;
 
-	if (index_to_minor(index) >= 1 << MINORBITS)
-		return -ENOSPC;
+	do {
+		if (!ida_pre_get(&vd_index_ida, GFP_KERNEL))
+			return err;
+
+		spin_lock(&vd_index_lock);
+		err = ida_get_new(&vd_index_ida, &index);
+		spin_unlock(&vd_index_lock);
+	} while (err == -EAGAIN);
+
+	if (err)
+		return err;
+
+	if (index_to_minor(index) >= 1 << MINORBITS) {
+		err =  -ENOSPC;
+		goto out_free_index;
+	}
 
 	/* We need to know how many segments before we allocate. */
 	err = virtio_config_val(vdev, VIRTIO_BLK_F_SEG_MAX,
@@ -421,7 +440,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 	vblk->disk->private_data = vblk;
 	vblk->disk->fops = &virtblk_fops;
 	vblk->disk->driverfs_dev = &vdev->dev;
-	index++;
+	vblk->index = index;
 
 	/* configure queue flush support */
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
@@ -516,6 +535,10 @@ out_free_vq:
 	vdev->config->del_vqs(vdev);
 out_free_vblk:
 	kfree(vblk);
+out_free_index:
+	spin_lock(&vd_index_lock);
+	ida_remove(&vd_index_ida, index);
+	spin_unlock(&vd_index_lock);
 out:
 	return err;
 }
@@ -529,6 +552,10 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
 	/* Nothing should be pending. */
 	BUG_ON(!list_empty(&vblk->reqs));
 
+	spin_lock(&vd_index_lock);
+	ida_remove(&vd_index_ida, vblk->index);
+	spin_unlock(&vd_index_lock);
+
 	/* Stop all the virtqueues. */
 	vdev->config->reset(vdev);
 
-- 
1.7.1

^ permalink raw reply related

* Re: [TRIVIAL PATCH V2 next 12/15] video: Convert vmalloc/memset to vzalloc
From: Konrad Rzeszutek Wilk @ 2011-05-31 14:28 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-fbdev, Jeremy Fitzhardinge, Jiri Kosina, Heiko Stübner,
	linux-kernel, virtualization, Paul Mundt, xen-devel, Jaya Kumar
In-Reply-To: <1306606413.20336.9.camel@Joe-Laptop>

On Sat, May 28, 2011 at 11:13:33AM -0700, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> on the:

> diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
> index a20218c..beac52f 100644
> --- a/drivers/video/xen-fbfront.c
> +++ b/drivers/video/xen-fbfront.c
> @@ -395,10 +395,9 @@ static int __devinit xenfb_probe(struct xenbus_device *dev,
>  	spin_lock_init(&info->dirty_lock);
>  	spin_lock_init(&info->resize_lock);
>  
> -	info->fb = vmalloc(fb_size);
> +	info->fb = vzalloc(fb_size);
>  	if (info->fb == NULL)
>  		goto error_nomem;
> -	memset(info->fb, 0, fb_size);
>  
>  	info->nr_pages = (fb_size + PAGE_SIZE - 1) >> PAGE_SHIFT;

^ permalink raw reply

* Re: [TRIVIAL PATCH next 12/15] video: Convert vmalloc/memset to vzalloc
From: Mel Gorman @ 2011-05-30 11:40 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-fbdev, Jeremy Fitzhardinge, Jiri Kosina,
	Konrad Rzeszutek Wilk, linux-kernel, virtualization, Paul Mundt,
	xen-devel, Jaya Kumar
In-Reply-To: <77538de911b8af99f68b7bcfbaaedce8e40db04f.1306603968.git.joe@perches.com>

On Sat, May 28, 2011 at 10:36:32AM -0700, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/video/arcfb.c        |    5 ++---
>  drivers/video/broadsheetfb.c |    4 +---
>  drivers/video/hecubafb.c     |    5 ++---
>  drivers/video/metronomefb.c  |    4 +---
>  drivers/video/xen-fbfront.c  |    3 +--
>  5 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/video/arcfb.c b/drivers/video/arcfb.c
> index 3ec4923..86573e2 100644
> --- a/drivers/video/arcfb.c
> +++ b/drivers/video/arcfb.c
> @@ -515,11 +515,10 @@ static int __devinit arcfb_probe(struct platform_device *dev)
>  
>  	/* We need a flat backing store for the Arc's
>  	   less-flat actual paged framebuffer */
> -	if (!(videomemory = vmalloc(videomemorysize)))
> +	videomemory = vmalloc(videomemorysize);
> +	if (!videomemory)
>  		return retval;
>  
> -	memset(videomemory, 0, videomemorysize);
> -
>  	info = framebuffer_alloc(sizeof(struct arcfb_par), &dev->dev);
>  	if (!info)
>  		goto err;

This is the first commit I saw and stopped reading at this point
because this hunk is not using vzalloc. I imagine grep for ^+ and
vmalloc throughout the series would be helpful?

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply

* Re: [PATCHv2 10/14] virtio_net: limit xmit polling
From: Rusty Russell @ 2011-05-30  6:27 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <20110528200204.GB7046@redhat.com>

On Sat, 28 May 2011 23:02:04 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, May 26, 2011 at 12:58:23PM +0930, Rusty Russell wrote:
> > ie. free two packets for every one we're about to add.  For steady state
> > that would work really well.
> 
> Sure, with indirect buffers, but if we
> don't use indirect (and we discussed switching indirect off
> dynamically in the past) this becomes harder to
> be sure about. I think I understand why but
> does not a simple capacity check make it more obvious?

...

> >  Then we hit the case where the ring
> > seems full after we do the add: at that point, screw latency, and just
> > try to free all the buffers we can.
> 
> I see. But the code currently does this:
> 
> 	for(..)
> 		get_buf
> 	add_buf
> 	if (capacity < max_sk_frags+2) {
> 		if (!enable_cb)
> 			for(..)
> 				get_buf
> 	}
> 
> 
> In other words the second get_buf is only called
> in the unlikely case of race condition.
> 
> So we'll need to add *another* call to get_buf.
> Is it just me or is this becoming messy?

Yes, good point.  I really wonder if anyone would be able to measure the
difference between simply freeing 2 every time (with possible extra
stalls for strange cases) and the more complete version.

But it runs against my grain to implement heuristics when one more call
would make it provably reliable.

Please find a way to make that for loop less ugly though!

Thanks,
Rusty.

^ permalink raw reply

* Re: [PATCHv2 10/14] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-05-28 20:02 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <87vcwyjg2w.fsf@rustcorp.com.au>

On Thu, May 26, 2011 at 12:58:23PM +0930, Rusty Russell wrote:
> On Wed, 25 May 2011 09:07:59 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Wed, May 25, 2011 at 11:05:04AM +0930, Rusty Russell wrote:
> > Hmm I'm not sure I got it, need to think about this.
> > I'd like to go back and document how my design was supposed to work.
> > This really should have been in commit log or even a comment.
> > I thought we need a min, not a max.
> > We start with this:
> > 
> > 	while ((c = (virtqueue_get_capacity(vq) < 2 + MAX_SKB_FRAGS) &&
> > 		(skb = get_buf)))
> > 		kfree_skb(skb);
> > 	return !c;
> > 
> > This is clean and simple, right? And it's exactly asking for what we need.
> 
> No, I started from the other direction:
> 
>         for (i = 0; i < 2; i++) {
>                 skb = get_buf();
>                 if (!skb)
>                         break;
>                 kfree_skb(skb);
>         }
> 
> ie. free two packets for every one we're about to add.  For steady state
> that would work really well.

Sure, with indirect buffers, but if we
don't use indirect (and we discussed switching indirect off
dynamically in the past) this becomes harder to
be sure about. I think I understand why but
does not a simple capacity check make it more obvious?

>  Then we hit the case where the ring
> seems full after we do the add: at that point, screw latency, and just
> try to free all the buffers we can.

I see. But the code currently does this:

	for(..)
		get_buf
	add_buf
	if (capacity < max_sk_frags+2) {
		if (!enable_cb)
			for(..)
				get_buf
	}


In other words the second get_buf is only called
in the unlikely case of race condition.

So we'll need to add *another* call to get_buf.
Is it just me or is this becoming messy?

I was also be worried that we are adding more
"modes" to the code: high and low latency
depending on different speeds between host and guest,
which would be hard to trigger and test.
That's why I tried hard to make the code behave the
same all the time and free up just a bit more than
the minimum necessary.

> > on the normal path min == 2 so we're low latency but we keep ahead on
> > average. min == 0 for the "we're out of capacity, we may have to stop
> > the queue".
> > 
> > Does the above make sense at all?
> 
> It makes sense, but I think it's a classic case where incremental
> improvements aren't as good as starting from scratch.
> 
> Cheers,
> Rusty.

The only difference on good path seems an extra capacity check,
so I don't expect the difference will be testable, do you?

-- 
MST

^ permalink raw reply

* Re: 2.6.40 event idx patches
From: Michael S. Tsirkin @ 2011-05-28 19:42 UTC (permalink / raw)
  To: Rusty Russell; +Cc: virtualization
In-Reply-To: <20110528193139.GA6650@redhat.com>

On Sat, May 28, 2011 at 10:34:26PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 25, 2011 at 11:34:07AM +0930, Rusty Russell wrote:
> > On Tue, 24 May 2011 12:23:45 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > Just checking: were you going to send the following to Linus for 2.6.40?
> > > virtio:event_index_interface.patch
> > > virtio:ring_inline_function_to_check_for_events.patch
> > > virtio:ring_support_event_idx_feature.patch
> > > misc:vhost_support_event_index.patch
> > > virtio:test_support_event_index.patch
> > > virtio:add_api_for_delayed_callbacks.patch
> > > virtio:net_delay_tx_callbacks.patch
> > > 
> > > I certainly hope so as these modify both host and guest which makes
> > > testing them out of tree hard for people.
> > > Once stuff lands on Linus' tree we can patch
> > > qemu and get more people to try out the patches.
> > > 
> > > Also, Linus said it's going to be a short window so ...
> > 
> > Yes.  They've been in linux-next since the weekend (ie. today will be the
> > third linux-next build), and my plan was to push them Friday.
> > 
> > Nothing goes into linux-next which is not for Linus until after Linus
> > closes the merge window.
> > 
> > I also want to find a few cycles to adapt lguest; it's *always* good to
> > have a second implementation.
> > 
> > Cheers,
> > Rusty.
> 
> Couldn't find any pull requests at lkml, and I read the window will
> likely close May 29.  Did you end up not sending it, or did I miss it?

Pls ignore, problem at my end.
Thanks, and sorry about the noise.

> -- 
> MST

^ permalink raw reply

* Re: 2.6.40 event idx patches
From: Michael S. Tsirkin @ 2011-05-28 19:34 UTC (permalink / raw)
  To: Rusty Russell; +Cc: virtualization
In-Reply-To: <87y61vilig.fsf@rustcorp.com.au>

On Wed, May 25, 2011 at 11:34:07AM +0930, Rusty Russell wrote:
> On Tue, 24 May 2011 12:23:45 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > Just checking: were you going to send the following to Linus for 2.6.40?
> > virtio:event_index_interface.patch
> > virtio:ring_inline_function_to_check_for_events.patch
> > virtio:ring_support_event_idx_feature.patch
> > misc:vhost_support_event_index.patch
> > virtio:test_support_event_index.patch
> > virtio:add_api_for_delayed_callbacks.patch
> > virtio:net_delay_tx_callbacks.patch
> > 
> > I certainly hope so as these modify both host and guest which makes
> > testing them out of tree hard for people.
> > Once stuff lands on Linus' tree we can patch
> > qemu and get more people to try out the patches.
> > 
> > Also, Linus said it's going to be a short window so ...
> 
> Yes.  They've been in linux-next since the weekend (ie. today will be the
> third linux-next build), and my plan was to push them Friday.
> 
> Nothing goes into linux-next which is not for Linus until after Linus
> closes the merge window.
> 
> I also want to find a few cycles to adapt lguest; it's *always* good to
> have a second implementation.
> 
> Cheers,
> Rusty.

Couldn't find any pull requests at lkml, and I read the window will
likely close May 29.  Did you end up not sending it, or did I miss it?

-- 
MST

^ permalink raw reply

* [TRIVIAL PATCH V2 next 12/15] video: Convert vmalloc/memset to vzalloc
From: Joe Perches @ 2011-05-28 18:13 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: linux-fbdev, Jeremy Fitzhardinge, Jiri Kosina,
	Konrad Rzeszutek Wilk, linux-kernel, virtualization, Paul Mundt,
	xen-devel, Jaya Kumar
In-Reply-To: <201105281948.12941.heiko@sntech.de>

Signed-off-by: Joe Perches <joe@perches.com>
---
Heiko Stübner <heiko@sntech.de> pointed out I can't type...
s/vmalloc/vmalloc/ doesn't do much.

 drivers/video/arcfb.c        |    5 ++---
 drivers/video/broadsheetfb.c |    4 +---
 drivers/video/hecubafb.c     |    5 ++---
 drivers/video/metronomefb.c  |    4 +---
 drivers/video/xen-fbfront.c  |    3 +--
 5 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/video/arcfb.c b/drivers/video/arcfb.c
index 3ec4923..86573e2 100644
--- a/drivers/video/arcfb.c
+++ b/drivers/video/arcfb.c
@@ -515,11 +515,10 @@ static int __devinit arcfb_probe(struct platform_device *dev)
 
 	/* We need a flat backing store for the Arc's
 	   less-flat actual paged framebuffer */
-	if (!(videomemory = vmalloc(videomemorysize)))
+	videomemory = vzalloc(videomemorysize);
+	if (!videomemory)
 		return retval;
 
-	memset(videomemory, 0, videomemorysize);
-
 	info = framebuffer_alloc(sizeof(struct arcfb_par), &dev->dev);
 	if (!info)
 		goto err;
diff --git a/drivers/video/broadsheetfb.c b/drivers/video/broadsheetfb.c
index ebda687..377dde3 100644
--- a/drivers/video/broadsheetfb.c
+++ b/drivers/video/broadsheetfb.c
@@ -1101,12 +1101,10 @@ static int __devinit broadsheetfb_probe(struct platform_device *dev)
 
 	videomemorysize = roundup((dpyw*dpyh), PAGE_SIZE);
 
-	videomemory = vmalloc(videomemorysize);
+	videomemory = vzalloc(videomemorysize);
 	if (!videomemory)
 		goto err_fb_rel;
 
-	memset(videomemory, 0, videomemorysize);
-
 	info->screen_base = (char *)videomemory;
 	info->fbops = &broadsheetfb_ops;
 
diff --git a/drivers/video/hecubafb.c b/drivers/video/hecubafb.c
index 1b94643..fbef15f 100644
--- a/drivers/video/hecubafb.c
+++ b/drivers/video/hecubafb.c
@@ -231,11 +231,10 @@ static int __devinit hecubafb_probe(struct platform_device *dev)
 
 	videomemorysize = (DPY_W*DPY_H)/8;
 
-	if (!(videomemory = vmalloc(videomemorysize)))
+	videomemory = vzalloc(videomemorysize);
+	if (!videomemory)
 		return retval;
 
-	memset(videomemory, 0, videomemorysize);
-
 	info = framebuffer_alloc(sizeof(struct hecubafb_par), &dev->dev);
 	if (!info)
 		goto err_fballoc;
diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
index ed64edf..97d45e5 100644
--- a/drivers/video/metronomefb.c
+++ b/drivers/video/metronomefb.c
@@ -628,12 +628,10 @@ static int __devinit metronomefb_probe(struct platform_device *dev)
 	/* we need to add a spare page because our csum caching scheme walks
 	 * to the end of the page */
 	videomemorysize = PAGE_SIZE + (fw * fh);
-	videomemory = vmalloc(videomemorysize);
+	videomemory = vzalloc(videomemorysize);
 	if (!videomemory)
 		goto err_fb_rel;
 
-	memset(videomemory, 0, videomemorysize);
-
 	info->screen_base = (char __force __iomem *)videomemory;
 	info->fbops = &metronomefb_ops;
 
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index a20218c..beac52f 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -395,10 +395,9 @@ static int __devinit xenfb_probe(struct xenbus_device *dev,
 	spin_lock_init(&info->dirty_lock);
 	spin_lock_init(&info->resize_lock);
 
-	info->fb = vmalloc(fb_size);
+	info->fb = vzalloc(fb_size);
 	if (info->fb == NULL)
 		goto error_nomem;
-	memset(info->fb, 0, fb_size);
 
 	info->nr_pages = (fb_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
 
-- 


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

^ permalink raw reply related

* Re: [TRIVIAL PATCH next 12/15] video: Convert vmalloc/memset to vzalloc
From: Heiko Stübner @ 2011-05-28 17:48 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-fbdev, Jeremy Fitzhardinge, Jiri Kosina,
	Konrad Rzeszutek Wilk, linux-kernel, virtualization, Paul Mundt,
	xen-devel, Jaya Kumar
In-Reply-To: <77538de911b8af99f68b7bcfbaaedce8e40db04f.1306603968.git.joe@perches.com>

Am Samstag 28 Mai 2011, 19:36:32 schrieb Joe Perches:
> diff --git a/drivers/video/arcfb.c b/drivers/video/arcfb.c
> index 3ec4923..86573e2 100644
> --- a/drivers/video/arcfb.c
> +++ b/drivers/video/arcfb.c
> @@ -515,11 +515,10 @@ static int __devinit arcfb_probe(struct
> platform_device *dev)
> 
>  	/* We need a flat backing store for the Arc's
>  	   less-flat actual paged framebuffer */
> -	if (!(videomemory = vmalloc(videomemorysize)))
> +	videomemory = vmalloc(videomemorysize);
> +	if (!videomemory)
>  		return retval;
shouldn't this be vzalloc too?

> -	memset(videomemory, 0, videomemorysize);
> -
>  	info = framebuffer_alloc(sizeof(struct arcfb_par), &dev->dev);
>  	if (!info)
>  		goto err;


Heiko

^ permalink raw reply

* [TRIVIAL PATCH next 00/15] treewide: Convert vmalloc/memset to vzalloc
From: Joe Perches @ 2011-05-28 17:36 UTC (permalink / raw)
  To: linux-atm-general, netdev, drbd-user, dm-devel, linux-raid,
	linux-mtd
  Cc: devel, linux-s390, xfs, linux-kernel, linux-media

Resubmittal of patches from November 2010 and a few new ones.

Joe Perches (15):
  s390: Convert vmalloc/memset to vzalloc
  x86: Convert vmalloc/memset to vzalloc
  atm: Convert vmalloc/memset to vzalloc
  drbd: Convert vmalloc/memset to vzalloc
  char: Convert vmalloc/memset to vzalloc
  isdn: Convert vmalloc/memset to vzalloc
  md: Convert vmalloc/memset to vzalloc
  media: Convert vmalloc/memset to vzalloc
  mtd: Convert vmalloc/memset to vzalloc
  scsi: Convert vmalloc/memset to vzalloc
  staging: Convert vmalloc/memset to vzalloc
  video: Convert vmalloc/memset to vzalloc
  fs: Convert vmalloc/memset to vzalloc
  mm: Convert vmalloc/memset to vzalloc
  net: Convert vmalloc/memset to vzalloc

 arch/s390/hypfs/hypfs_diag.c           |    3 +--
 arch/x86/mm/pageattr-test.c            |    3 +--
 drivers/atm/idt77252.c                 |   11 ++++++-----
 drivers/atm/lanai.c                    |    3 +--
 drivers/block/drbd/drbd_bitmap.c       |    5 ++---
 drivers/char/agp/backend.c             |    3 +--
 drivers/char/raw.c                     |    3 +--
 drivers/isdn/i4l/isdn_common.c         |    4 ++--
 drivers/isdn/mISDN/dsp_core.c          |    3 +--
 drivers/isdn/mISDN/l1oip_codec.c       |    6 ++----
 drivers/md/dm-log.c                    |    3 +--
 drivers/md/dm-snap-persistent.c        |    3 +--
 drivers/md/dm-table.c                  |    4 +---
 drivers/media/video/videobuf2-dma-sg.c |    8 ++------
 drivers/mtd/mtdswap.c                  |    3 +--
 drivers/s390/cio/blacklist.c           |    3 +--
 drivers/scsi/bfa/bfad.c                |    3 +--
 drivers/scsi/bfa/bfad_debugfs.c        |    8 ++------
 drivers/scsi/cxgbi/libcxgbi.h          |    6 ++----
 drivers/scsi/qla2xxx/qla_attr.c        |    6 ++----
 drivers/scsi/qla2xxx/qla_bsg.c         |    3 +--
 drivers/scsi/scsi_debug.c              |    7 ++-----
 drivers/staging/rts_pstor/ms.c         |    3 +--
 drivers/staging/rts_pstor/rtsx_chip.c  |    6 ++----
 drivers/video/arcfb.c                  |    5 ++---
 drivers/video/broadsheetfb.c           |    4 +---
 drivers/video/hecubafb.c               |    5 ++---
 drivers/video/metronomefb.c            |    4 +---
 drivers/video/xen-fbfront.c            |    3 +--
 fs/coda/coda_linux.h                   |    5 ++---
 fs/reiserfs/journal.c                  |    9 +++------
 fs/reiserfs/resize.c                   |    4 +---
 fs/xfs/linux-2.6/kmem.h                |    7 +------
 mm/page_cgroup.c                       |    3 +--
 net/netfilter/x_tables.c               |    5 ++---
 net/rds/ib_cm.c                        |    6 ++----
 36 files changed, 57 insertions(+), 113 deletions(-)

-- 
1.7.5.rc3.dirty

^ permalink raw reply

* Re: [PATCH v2] arch/tile: more /proc and /sys file support
From: Arnd Bergmann @ 2011-05-27 14:23 UTC (permalink / raw)
  To: Chris Metcalf; +Cc: Andrew Morton, linux-kernel, Al Viro, virtualization
In-Reply-To: <201105261648.p4QGmNKf001636@farm-0023.internal.tilera.com>

On Thursday 26 May 2011, Chris Metcalf wrote:
> This change introduces a few of the less controversial /proc and
> /proc/sys interfaces for tile, along with sysfs attributes for
> various things that were originally proposed as /proc/tile files.
> It also adjusts the "hardwall" proc API.

Looks good to me now, except

> Finally, after some feedback from Arnd Berghamm for the previous

typo					^^^^

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* [PATCH 2/2] staging: hv: use delayed_work for netvsc_send_garp()
From: Haiyang Zhang @ 2011-05-27 13:21 UTC (permalink / raw)
  To: haiyangz, hjanssen, kys, v-abkane, gregkh, linux-kernel, devel,
	vir
  Cc: Stephen Hemminger
In-Reply-To: <1306502515-18961-1-git-send-email-haiyangz@microsoft.com>

Instead of sleeping in a scheduled work, we now use delayed_work
for netvsc_send_garp().

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/staging/hv/netvsc_drv.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index f510959..33cab9c 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -46,7 +46,7 @@ struct net_device_context {
 	/* point back to our device context */
 	struct hv_device *device_ctx;
 	unsigned long avail;
-	struct work_struct work;
+	struct delayed_work dwork;
 };
 
 
@@ -217,7 +217,7 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
 		netif_wake_queue(net);
 		netif_notify_peers(net);
 		ndev_ctx = netdev_priv(net);
-		schedule_work(&ndev_ctx->work);
+		schedule_delayed_work(&ndev_ctx->dwork, msecs_to_jiffies(20));
 	} else {
 		netif_carrier_off(net);
 		netif_stop_queue(net);
@@ -315,7 +315,7 @@ static const struct net_device_ops device_ops = {
  * Send GARP packet to network peers after migrations.
  * After Quick Migration, the network is not immediately operational in the
  * current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, add
- * another netif_notify_peers() into a scheduled work, otherwise GARP packet
+ * another netif_notify_peers() into a delayed work, otherwise GARP packet
  * will not be sent after quick migration, and cause network disconnection.
  */
 static void netvsc_send_garp(struct work_struct *w)
@@ -323,8 +323,7 @@ static void netvsc_send_garp(struct work_struct *w)
 	struct net_device_context *ndev_ctx;
 	struct net_device *net;
 
-	msleep(20);
-	ndev_ctx = container_of(w, struct net_device_context, work);
+	ndev_ctx = container_of(w, struct net_device_context, dwork.work);
 	net = dev_get_drvdata(&ndev_ctx->device_ctx->device);
 	netif_notify_peers(net);
 }
@@ -348,7 +347,7 @@ static int netvsc_probe(struct hv_device *dev)
 	net_device_ctx->device_ctx = dev;
 	net_device_ctx->avail = ring_size;
 	dev_set_drvdata(&dev->device, net);
-	INIT_WORK(&net_device_ctx->work, netvsc_send_garp);
+	INIT_DELAYED_WORK(&net_device_ctx->dwork, netvsc_send_garp);
 
 	/* Notify the netvsc driver of the new device */
 	device_info.ring_size = ring_size;
@@ -387,12 +386,16 @@ static int netvsc_probe(struct hv_device *dev)
 static int netvsc_remove(struct hv_device *dev)
 {
 	struct net_device *net = dev_get_drvdata(&dev->device);
+	struct net_device_context *ndev_ctx;
 
 	if (net == NULL) {
 		dev_err(&dev->device, "No net device to remove\n");
 		return 0;
 	}
 
+	ndev_ctx = netdev_priv(net);
+	cancel_delayed_work_sync(&ndev_ctx->dwork);
+
 	/* Stop outbound asap */
 	netif_stop_queue(net);
 
-- 
1.6.3.2

^ permalink raw reply related

* [PATCH 1/2] staging: hv: convert DPRINT_DBG() to netdev_dbg() in dump_rndis_message()
From: Haiyang Zhang @ 2011-05-27 13:21 UTC (permalink / raw)
  To: haiyangz, hjanssen, kys, v-abkane, gregkh, linux-kernel, devel,
	vir
  Cc: Stephen Hemminger

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/staging/hv/rndis_filter.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/hv/rndis_filter.c b/drivers/staging/hv/rndis_filter.c
index 5a5bf64..5674a13 100644
--- a/drivers/staging/hv/rndis_filter.c
+++ b/drivers/staging/hv/rndis_filter.c
@@ -139,14 +139,17 @@ static void put_rndis_request(struct rndis_device *dev,
 	kfree(req);
 }
 
-static void dump_rndis_message(struct rndis_message *rndis_msg)
+static void dump_rndis_message(struct hv_device *hv_dev,
+			struct rndis_message *rndis_msg)
 {
+	struct net_device *netdev = dev_get_drvdata(&hv_dev->device);
+
 	switch (rndis_msg->ndis_msg_type) {
 	case REMOTE_NDIS_PACKET_MSG:
-		DPRINT_DBG(NETVSC, "REMOTE_NDIS_PACKET_MSG (len %u, "
+		netdev_dbg(netdev, "REMOTE_NDIS_PACKET_MSG (len %u, "
 			   "data offset %u data len %u, # oob %u, "
 			   "oob offset %u, oob len %u, pkt offset %u, "
-			   "pkt len %u",
+			   "pkt len %u\n",
 			   rndis_msg->msg_len,
 			   rndis_msg->msg.pkt.data_offset,
 			   rndis_msg->msg.pkt.data_len,
@@ -158,10 +161,10 @@ static void dump_rndis_message(struct rndis_message *rndis_msg)
 		break;
 
 	case REMOTE_NDIS_INITIALIZE_CMPLT:
-		DPRINT_DBG(NETVSC, "REMOTE_NDIS_INITIALIZE_CMPLT "
+		netdev_dbg(netdev, "REMOTE_NDIS_INITIALIZE_CMPLT "
 			"(len %u, id 0x%x, status 0x%x, major %d, minor %d, "
 			"device flags %d, max xfer size 0x%x, max pkts %u, "
-			"pkt aligned %u)",
+			"pkt aligned %u)\n",
 			rndis_msg->msg_len,
 			rndis_msg->msg.init_complete.req_id,
 			rndis_msg->msg.init_complete.status,
@@ -176,9 +179,9 @@ static void dump_rndis_message(struct rndis_message *rndis_msg)
 		break;
 
 	case REMOTE_NDIS_QUERY_CMPLT:
-		DPRINT_DBG(NETVSC, "REMOTE_NDIS_QUERY_CMPLT "
+		netdev_dbg(netdev, "REMOTE_NDIS_QUERY_CMPLT "
 			"(len %u, id 0x%x, status 0x%x, buf len %u, "
-			"buf offset %u)",
+			"buf offset %u)\n",
 			rndis_msg->msg_len,
 			rndis_msg->msg.query_complete.req_id,
 			rndis_msg->msg.query_complete.status,
@@ -189,16 +192,16 @@ static void dump_rndis_message(struct rndis_message *rndis_msg)
 		break;
 
 	case REMOTE_NDIS_SET_CMPLT:
-		DPRINT_DBG(NETVSC,
-			"REMOTE_NDIS_SET_CMPLT (len %u, id 0x%x, status 0x%x)",
+		netdev_dbg(netdev,
+			"REMOTE_NDIS_SET_CMPLT (len %u, id 0x%x, status 0x%x)\n",
 			rndis_msg->msg_len,
 			rndis_msg->msg.set_complete.req_id,
 			rndis_msg->msg.set_complete.status);
 		break;
 
 	case REMOTE_NDIS_INDICATE_STATUS_MSG:
-		DPRINT_DBG(NETVSC, "REMOTE_NDIS_INDICATE_STATUS_MSG "
-			"(len %u, status 0x%x, buf len %u, buf offset %u)",
+		netdev_dbg(netdev, "REMOTE_NDIS_INDICATE_STATUS_MSG "
+			"(len %u, status 0x%x, buf len %u, buf offset %u)\n",
 			rndis_msg->msg_len,
 			rndis_msg->msg.indicate_status.status,
 			rndis_msg->msg.indicate_status.status_buflen,
@@ -206,7 +209,7 @@ static void dump_rndis_message(struct rndis_message *rndis_msg)
 		break;
 
 	default:
-		DPRINT_DBG(NETVSC, "0x%x (len %u)",
+		netdev_dbg(netdev, "0x%x (len %u)\n",
 			rndis_msg->ndis_msg_type,
 			rndis_msg->msg_len);
 		break;
@@ -387,7 +390,7 @@ int rndis_filter_receive(struct hv_device *dev,
 
 	kunmap_atomic(rndis_hdr - pkt->page_buf[0].offset, KM_IRQ0);
 
-	dump_rndis_message(&rndis_msg);
+	dump_rndis_message(dev, &rndis_msg);
 
 	switch (rndis_msg.ndis_msg_type) {
 	case REMOTE_NDIS_PACKET_MSG:
-- 
1.6.3.2

^ permalink raw reply related

* [PATCH v2] arch/tile: more /proc and /sys file support
From: Chris Metcalf @ 2011-05-26 16:40 UTC (permalink / raw)
  To: Arnd Bergmann, linux-kernel, virtualization, Andrew Morton
In-Reply-To: <201105181807.p4II7C5g015224@farm-0002.internal.tilera.com>

This change introduces a few of the less controversial /proc and
/proc/sys interfaces for tile, along with sysfs attributes for
various things that were originally proposed as /proc/tile files.
It also adjusts the "hardwall" proc API.

Arnd Bergmann reviewed the initial arch/tile submission, which
included a complete set of all the /proc/tile and /proc/sys/tile
knobs that we had added in a somewhat ad hoc way during initial
development, and provided feedback on where most of them should go.

One knob turned out to be similar enough to the existing
/proc/sys/debug/exception-trace that it was re-implemented to use
that model instead.

Another knob was /proc/tile/grid, which reported the "grid" dimensions
of a tile chip (e.g. 8x8 processors = 64-core chip).  Arnd suggested
looking at sysfs for that, so this change moves that information
to a pair of sysfs attributes (chip_width and chip_height) in the
/sys/devices/system/cpu directory.  We also put the "chip_serial"
and "chip_revision" information from our old /proc/tile/board file
as attributes in /sys/devices/system/cpu.

Other information collected via hypervisor APIs is now placed in
/sys/hypervisor.  We create a /sys/hypervisor/type file (holding the
constant string "tilera") to be parallel with the Xen use of
/sys/hypervisor/type holding "xen".  We create three top-level files,
"version" (the hypervisor's own version), "config_version" (the
version of the configuration file), and "hvconfig" (the contents of
the configuration file).  The remaining information from our old
/proc/tile/board and /proc/tile/switch files becomes an attribute
group appearing under /sys/hypervisor/board/.

Finally, after some feedback from Arnd Berghamm for the previous
version of this patch, the /proc/tile/hardwall file is split up into
two conceptual parts.  First, a directory /proc/tile/hardwall/ which
contains one file per active hardwall, each file named after the
hardwall's ID and holding a cpulist that says which cpus are enclosed by
the hardwall.  Second, a /proc/PID file "hardwall" that is either
empty (for non-hardwall-using processes) or contains the hardwall ID.

Finally, this change pushes the /proc/sys/tile/unaligned_fixup/
directory, with knobs controlling the kernel code for handling the
fixup of unaligned exceptions.

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
 arch/tile/Kconfig                |    1 +
 arch/tile/include/asm/hardwall.h |   15 +++-
 arch/tile/kernel/Makefile        |    2 +-
 arch/tile/kernel/hardwall.c      |   90 ++++++++++++++-----
 arch/tile/kernel/proc.c          |   73 +++++++++++++++
 arch/tile/kernel/sysfs.c         |  185 ++++++++++++++++++++++++++++++++++++++
 fs/proc/base.c                   |    9 ++
 7 files changed, 347 insertions(+), 28 deletions(-)
 create mode 100644 arch/tile/kernel/sysfs.c

diff --git a/arch/tile/Kconfig b/arch/tile/Kconfig
index 635e1bf..3f7d63c 100644
--- a/arch/tile/Kconfig
+++ b/arch/tile/Kconfig
@@ -12,6 +12,7 @@ config TILE
 	select GENERIC_IRQ_PROBE
 	select GENERIC_PENDING_IRQ if SMP
 	select GENERIC_IRQ_SHOW
+	select SYS_HYPERVISOR
 
 # FIXME: investigate whether we need/want these options.
 #	select HAVE_IOREMAP_PROT
diff --git a/arch/tile/include/asm/hardwall.h b/arch/tile/include/asm/hardwall.h
index 0bed3ec..2ac4228 100644
--- a/arch/tile/include/asm/hardwall.h
+++ b/arch/tile/include/asm/hardwall.h
@@ -40,6 +40,10 @@
 #define HARDWALL_DEACTIVATE \
  _IO(HARDWALL_IOCTL_BASE, _HARDWALL_DEACTIVATE)
 
+#define _HARDWALL_GET_ID 4
+#define HARDWALL_GET_ID \
+ _IO(HARDWALL_IOCTL_BASE, _HARDWALL_GET_ID)
+
 #ifndef __KERNEL__
 
 /* This is the canonical name expected by userspace. */
@@ -47,9 +51,14 @@
 
 #else
 
-/* Hook for /proc/tile/hardwall. */
-struct seq_file;
-int proc_tile_hardwall_show(struct seq_file *sf, void *v);
+/* /proc hooks for hardwall. */
+struct proc_dir_entry;
+#ifdef CONFIG_HARDWALL
+void proc_tile_hardwall_init(struct proc_dir_entry *root);
+int proc_pid_hardwall(struct task_struct *task, char *buffer);
+#else
+static inline void proc_tile_hardwall_init(struct proc_dir_entry *root) {}
+#endif
 
 #endif
 
diff --git a/arch/tile/kernel/Makefile b/arch/tile/kernel/Makefile
index b4c8e8e..b4dbc05 100644
--- a/arch/tile/kernel/Makefile
+++ b/arch/tile/kernel/Makefile
@@ -5,7 +5,7 @@
 extra-y := vmlinux.lds head_$(BITS).o
 obj-y := backtrace.o entry.o init_task.o irq.o messaging.o \
 	pci-dma.o proc.o process.o ptrace.o reboot.o \
-	setup.o signal.o single_step.o stack.o sys.o time.o traps.o \
+	setup.o signal.o single_step.o stack.o sys.o sysfs.o time.o traps.o \
 	intvec_$(BITS).o regs_$(BITS).o tile-desc_$(BITS).o
 
 obj-$(CONFIG_HARDWALL)		+= hardwall.o
diff --git a/arch/tile/kernel/hardwall.c b/arch/tile/kernel/hardwall.c
index 3bddef7..8c41891 100644
--- a/arch/tile/kernel/hardwall.c
+++ b/arch/tile/kernel/hardwall.c
@@ -40,16 +40,25 @@
 struct hardwall_info {
 	struct list_head list;             /* "rectangles" list */
 	struct list_head task_head;        /* head of tasks in this hardwall */
+	struct cpumask cpumask;            /* cpus in the rectangle */
 	int ulhc_x;                        /* upper left hand corner x coord */
 	int ulhc_y;                        /* upper left hand corner y coord */
 	int width;                         /* rectangle width */
 	int height;                        /* rectangle height */
+	int id;                            /* integer id for this hardwall */
 	int teardown_in_progress;          /* are we tearing this one down? */
 };
 
 /* Currently allocated hardwall rectangles */
 static LIST_HEAD(rectangles);
 
+/* /proc/tile/hardwall */
+static struct proc_dir_entry *hardwall_proc_dir;
+
+/* Functions to manage files in /proc/tile/hardwall. */
+static void hardwall_add_proc(struct hardwall_info *rect);
+static void hardwall_remove_proc(struct hardwall_info *rect);
+
 /*
  * Guard changes to the hardwall data structures.
  * This could be finer grained (e.g. one lock for the list of hardwall
@@ -105,6 +114,8 @@ static int setup_rectangle(struct hardwall_info *r, struct cpumask *mask)
 	r->ulhc_y = cpu_y(ulhc);
 	r->width = cpu_x(lrhc) - r->ulhc_x + 1;
 	r->height = cpu_y(lrhc) - r->ulhc_y + 1;
+	cpumask_copy(&r->cpumask, mask);
+	r->id = ulhc;   /* The ulhc cpu id can be the hardwall id. */
 
 	/* Width and height must be positive */
 	if (r->width <= 0 || r->height <= 0)
@@ -388,6 +399,9 @@ static struct hardwall_info *hardwall_create(
 	/* Set up appropriate hardwalling on all affected cpus. */
 	hardwall_setup(rect);
 
+	/* Create a /proc/tile/hardwall entry. */
+	hardwall_add_proc(rect);
+
 	return rect;
 }
 
@@ -645,6 +659,9 @@ static void hardwall_destroy(struct hardwall_info *rect)
 	/* Restart switch and disable firewall. */
 	on_each_cpu_mask(&mask, restart_udn_switch, NULL, 1);
 
+	/* Remove the /proc/tile/hardwall entry. */
+	hardwall_remove_proc(rect);
+
 	/* Now free the rectangle from the list. */
 	spin_lock_irqsave(&hardwall_lock, flags);
 	BUG_ON(!list_empty(&rect->task_head));
@@ -654,35 +671,57 @@ static void hardwall_destroy(struct hardwall_info *rect)
 }
 
 
-/*
- * Dump hardwall state via /proc; initialized in arch/tile/sys/proc.c.
- */
-int proc_tile_hardwall_show(struct seq_file *sf, void *v)
+static int hardwall_proc_show(struct seq_file *sf, void *v)
 {
-	struct hardwall_info *r;
+	struct hardwall_info *rect = sf->private;
+	char buf[256];
 
-	if (udn_disabled) {
-		seq_printf(sf, "%dx%d 0,0 pids:\n", smp_width, smp_height);
-		return 0;
-	}
-
-	spin_lock_irq(&hardwall_lock);
-	list_for_each_entry(r, &rectangles, list) {
-		struct task_struct *p;
-		seq_printf(sf, "%dx%d %d,%d pids:",
-			   r->width, r->height, r->ulhc_x, r->ulhc_y);
-		list_for_each_entry(p, &r->task_head, thread.hardwall_list) {
-			unsigned int cpu = cpumask_first(&p->cpus_allowed);
-			unsigned int x = cpu % smp_width;
-			unsigned int y = cpu / smp_width;
-			seq_printf(sf, " %d@%d,%d", p->pid, x, y);
-		}
-		seq_printf(sf, "\n");
-	}
-	spin_unlock_irq(&hardwall_lock);
+	int rc = cpulist_scnprintf(buf, sizeof(buf), &rect->cpumask);
+	buf[rc++] = '\n';
+	seq_write(sf, buf, rc);
 	return 0;
 }
 
+static int hardwall_proc_open(struct inode *inode,
+			      struct file *file)
+{
+	return single_open(file, hardwall_proc_show, PDE(inode)->data);
+}
+
+static const struct file_operations hardwall_proc_fops = {
+	.open		= hardwall_proc_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static void hardwall_add_proc(struct hardwall_info *rect)
+{
+	char buf[64];
+	snprintf(buf, sizeof(buf), "%d", rect->id);
+	proc_create_data(buf, 0444, hardwall_proc_dir,
+			 &hardwall_proc_fops, rect);
+}
+
+static void hardwall_remove_proc(struct hardwall_info *rect)
+{
+	char buf[64];
+	snprintf(buf, sizeof(buf), "%d", rect->id);
+	remove_proc_entry(buf, hardwall_proc_dir);
+}
+
+int proc_pid_hardwall(struct task_struct *task, char *buffer)
+{
+	struct hardwall_info *rect = task->thread.hardwall;
+	return rect ? sprintf(buffer, "%d\n", rect->id) : 0;
+}
+
+void proc_tile_hardwall_init(struct proc_dir_entry *root)
+{
+	if (!udn_disabled)
+		hardwall_proc_dir = proc_mkdir("hardwall", root);
+}
+
 
 /*
  * Character device support via ioctl/close.
@@ -716,6 +755,9 @@ static long hardwall_ioctl(struct file *file, unsigned int a, unsigned long b)
 			return -EINVAL;
 		return hardwall_deactivate(current);
 
+	case _HARDWALL_GET_ID:
+		return rect ? rect->id : -EINVAL;
+
 	default:
 		return -EINVAL;
 	}
diff --git a/arch/tile/kernel/proc.c b/arch/tile/kernel/proc.c
index 2e02c41..62d8208 100644
--- a/arch/tile/kernel/proc.c
+++ b/arch/tile/kernel/proc.c
@@ -27,6 +27,7 @@
 #include <asm/processor.h>
 #include <asm/sections.h>
 #include <asm/homecache.h>
+#include <asm/hardwall.h>
 #include <arch/chip.h>
 
 
@@ -88,3 +89,75 @@ const struct seq_operations cpuinfo_op = {
 	.stop	= c_stop,
 	.show	= show_cpuinfo,
 };
+
+/*
+ * Support /proc/tile directory
+ */
+
+static int __init proc_tile_init(void)
+{
+	struct proc_dir_entry *root = proc_mkdir("tile", NULL);
+	if (root == NULL)
+		return 0;
+
+	proc_tile_hardwall_init(root);
+
+	return 0;
+}
+
+arch_initcall(proc_tile_init);
+
+/*
+ * Support /proc/sys/tile directory
+ */
+
+#ifndef __tilegx__  /* FIXME: GX: no support for unaligned access yet */
+static ctl_table unaligned_subtable[] = {
+	{
+		.procname	= "enabled",
+		.data		= &unaligned_fixup,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec
+	},
+	{
+		.procname	= "printk",
+		.data		= &unaligned_printk,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec
+	},
+	{
+		.procname	= "count",
+		.data		= &unaligned_fixup_count,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec
+	},
+	{}
+};
+
+static ctl_table unaligned_table[] = {
+	{
+		.procname	= "unaligned_fixup",
+		.mode		= 0555,
+		.child		= unaligned_subtable
+	},
+	{}
+};
+#endif
+
+static struct ctl_path tile_path[] = {
+	{ .procname = "tile" },
+	{ }
+};
+
+static int __init proc_sys_tile_init(void)
+{
+#ifndef __tilegx__  /* FIXME: GX: no support for unaligned access yet */
+	register_sysctl_paths(tile_path, unaligned_table);
+#endif
+	return 0;
+}
+
+arch_initcall(proc_sys_tile_init);
diff --git a/arch/tile/kernel/sysfs.c b/arch/tile/kernel/sysfs.c
new file mode 100644
index 0000000..b671a86
--- /dev/null
+++ b/arch/tile/kernel/sysfs.c
@@ -0,0 +1,185 @@
+/*
+ * Copyright 2011 Tilera Corporation. All Rights Reserved.
+ *
+ *   This program is free software; you can redistribute it and/or
+ *   modify it under the terms of the GNU General Public License
+ *   as published by the Free Software Foundation, version 2.
+ *
+ *   This program is distributed in the hope that it will be useful, but
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ *   NON INFRINGEMENT.  See the GNU General Public License for
+ *   more details.
+ *
+ * /sys entry support.
+ */
+
+#include <linux/sysdev.h>
+#include <linux/cpu.h>
+#include <linux/slab.h>
+#include <linux/smp.h>
+#include <hv/hypervisor.h>
+
+/* Return a string queried from the hypervisor, truncated to page size. */
+static ssize_t get_hv_confstr(char *page, int query)
+{
+	ssize_t n = hv_confstr(query, (unsigned long)page, PAGE_SIZE - 1);
+	n = n < 0 ? 0 : min(n, (ssize_t)PAGE_SIZE - 1) - 1;
+	if (n)
+		page[n++] = '\n';
+	page[n] = '\0';
+	return n;
+}
+
+static ssize_t chip_width_show(struct sysdev_class *dev,
+			       struct sysdev_class_attribute *attr,
+			       char *page)
+{
+	return sprintf(page, "%u\n", smp_width);
+}
+static SYSDEV_CLASS_ATTR(chip_width, 0444, chip_width_show, NULL);
+
+static ssize_t chip_height_show(struct sysdev_class *dev,
+				struct sysdev_class_attribute *attr,
+				char *page)
+{
+	return sprintf(page, "%u\n", smp_height);
+}
+static SYSDEV_CLASS_ATTR(chip_height, 0444, chip_height_show, NULL);
+
+static ssize_t chip_serial_show(struct sysdev_class *dev,
+				struct sysdev_class_attribute *attr,
+				char *page)
+{
+	return get_hv_confstr(page, HV_CONFSTR_CHIP_SERIAL_NUM);
+}
+static SYSDEV_CLASS_ATTR(chip_serial, 0444, chip_serial_show, NULL);
+
+static ssize_t chip_revision_show(struct sysdev_class *dev,
+				  struct sysdev_class_attribute *attr,
+				  char *page)
+{
+	return get_hv_confstr(page, HV_CONFSTR_CHIP_REV);
+}
+static SYSDEV_CLASS_ATTR(chip_revision, 0444, chip_revision_show, NULL);
+
+
+static ssize_t type_show(struct sysdev_class *dev,
+			    struct sysdev_class_attribute *attr,
+			    char *page)
+{
+	return sprintf(page, "tilera\n");
+}
+static SYSDEV_CLASS_ATTR(type, 0444, type_show, NULL);
+
+#define HV_CONF_ATTR(name, conf)					\
+	static ssize_t name ## _show(struct sysdev_class *dev,		\
+				     struct sysdev_class_attribute *attr, \
+				     char *page)			\
+	{								\
+		return get_hv_confstr(page, conf);			\
+	}								\
+	static SYSDEV_CLASS_ATTR(name, 0444, name ## _show, NULL);
+
+HV_CONF_ATTR(version,		HV_CONFSTR_HV_SW_VER)
+HV_CONF_ATTR(config_version,	HV_CONFSTR_HV_CONFIG_VER)
+
+HV_CONF_ATTR(board_part,	HV_CONFSTR_BOARD_PART_NUM)
+HV_CONF_ATTR(board_serial,	HV_CONFSTR_BOARD_SERIAL_NUM)
+HV_CONF_ATTR(board_revision,	HV_CONFSTR_BOARD_REV)
+HV_CONF_ATTR(board_description,	HV_CONFSTR_BOARD_DESC)
+HV_CONF_ATTR(mezz_part,		HV_CONFSTR_MEZZ_PART_NUM)
+HV_CONF_ATTR(mezz_serial,	HV_CONFSTR_MEZZ_SERIAL_NUM)
+HV_CONF_ATTR(mezz_revision,	HV_CONFSTR_MEZZ_REV)
+HV_CONF_ATTR(mezz_description,	HV_CONFSTR_MEZZ_DESC)
+HV_CONF_ATTR(switch_control,	HV_CONFSTR_SWITCH_CONTROL)
+
+static struct attribute *board_attrs[] = {
+	&attr_board_part.attr,
+	&attr_board_serial.attr,
+	&attr_board_revision.attr,
+	&attr_board_description.attr,
+	&attr_mezz_part.attr,
+	&attr_mezz_serial.attr,
+	&attr_mezz_revision.attr,
+	&attr_mezz_description.attr,
+	&attr_switch_control.attr,
+	NULL
+};
+
+static struct attribute_group board_attr_group = {
+	.name   = "board",
+	.attrs  = board_attrs,
+};
+
+
+static struct bin_attribute hvconfig_bin;
+
+static ssize_t
+hvconfig_bin_read(struct file *filp, struct kobject *kobj,
+		  struct bin_attribute *bin_attr,
+		  char *buf, loff_t off, size_t count)
+{
+	static size_t size;
+
+	/* Lazily learn the true size (minus the trailing NUL). */
+	if (size == 0)
+		size = hv_confstr(HV_CONFSTR_HV_CONFIG, 0, 0) - 1;
+
+	/* Check and adjust input parameters. */
+	if (off > size)
+		return -EINVAL;
+	if (count > size - off)
+		count = size - off;
+
+	if (count) {
+		/* Get a copy of the hvc and copy out the relevant portion. */
+		char *hvc;
+
+		size = off + count;
+		hvc = kmalloc(size, GFP_KERNEL);
+		if (hvc == NULL)
+			return -ENOMEM;
+		hv_confstr(HV_CONFSTR_HV_CONFIG, (unsigned long)hvc, size);
+		memcpy(buf, hvc + off, count);
+		kfree(hvc);
+	}
+
+	return count;
+}
+
+static int __init create_sysfs_entries(void)
+{
+	struct sysdev_class *cls = &cpu_sysdev_class;
+	int err = 0;
+
+#define create_cpu_attr(name)						\
+	if (!err)							\
+		err = sysfs_create_file(&cls->kset.kobj, &attr_##name.attr);
+	create_cpu_attr(chip_width);
+	create_cpu_attr(chip_height);
+	create_cpu_attr(chip_serial);
+	create_cpu_attr(chip_revision);
+
+#define create_hv_attr(name)						\
+	if (!err)							\
+		err = sysfs_create_file(hypervisor_kobj, &attr_##name.attr);
+	create_hv_attr(type);
+	create_hv_attr(version);
+	create_hv_attr(config_version);
+
+	if (!err)
+		err = sysfs_create_group(hypervisor_kobj, &board_attr_group);
+
+	if (!err) {
+		sysfs_bin_attr_init(&hvconfig_bin);
+		hvconfig_bin.attr.name = "hvconfig";
+		hvconfig_bin.attr.mode = S_IRUGO;
+		hvconfig_bin.read = hvconfig_bin_read;
+		hvconfig_bin.size = PAGE_SIZE;
+		err = sysfs_create_bin_file(hypervisor_kobj, &hvconfig_bin);
+	}
+
+	return err;
+}
+subsys_initcall(create_sysfs_entries);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index dfa5327..3ad615f 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -83,6 +83,9 @@
 #include <linux/pid_namespace.h>
 #include <linux/fs_struct.h>
 #include <linux/slab.h>
+#ifdef CONFIG_HARDWALL
+#include <asm/hardwall.h>
+#endif
 #include "internal.h"
 
 /* NOTE:
@@ -2894,6 +2897,9 @@ static const struct pid_entry tgid_base_stuff[] = {
 #ifdef CONFIG_TASK_IO_ACCOUNTING
 	INF("io",	S_IRUGO, proc_tgid_io_accounting),
 #endif
+#ifdef CONFIG_HARDWALL
+	INF("hardwall",   S_IRUGO, proc_pid_hardwall),
+#endif
 };
 
 static int proc_tgid_base_readdir(struct file * filp,
@@ -3232,6 +3238,9 @@ static const struct pid_entry tid_base_stuff[] = {
 #ifdef CONFIG_TASK_IO_ACCOUNTING
 	INF("io",	S_IRUGO, proc_tid_io_accounting),
 #endif
+#ifdef CONFIG_HARDWALL
+	INF("hardwall",   S_IRUGO, proc_pid_hardwall),
+#endif
 };
 
 static int proc_tid_base_readdir(struct file * filp,
-- 
1.6.5.2

^ permalink raw reply related

* Re: [PERF RESULTS] virtio and vhost-net performance enhancements
From: Krishna Kumar2 @ 2011-05-26 16:29 UTC (permalink / raw)
  To: Krishna Kumar2
  Cc: habanero, lguest, Shirley Ma, kvm, Carsten Otte, linux-s390,
	Michael S. Tsirkin, Heiko Carstens, linux-kernel, virtualization,
	Steve Dobbelstein, Christian Borntraeger, Tom Lendacky, netdev,
	Martin Schwidefsky, linux390
In-Reply-To: <OFF9D0E604.B865A006-ON6525789C.00597010-6525789C.0059987A@LocalDomain>

Krishna Kumar2/India/IBM wrote on 05/26/2011 09:51:32 PM:

> > Could you please try TCP_RRs as well?
>
> Right. Here's the result for TCP_RR:

The actual transaction rate/second numbers are:

_____________________________________________________________
#     RR1      RR2 (%)          SD1        SD2 (%)
_____________________________________________________________
1     9476     9903 (4.5)       28.9       19.8 (-31.4)
2     17337    18225 (5.1)      92.7       83.7 (-9.7)
4     17385    27902 (60.4)     364.8      315.8 (-13.4)
8     25560    42912 (67.8)     1428.1     1234.0 (-13.5)
16    35898    55934 (55.8)     4391.6     4038.1 (-8.0)
32    48048    80228 (66.9)     17391.4    14932.0 (-14.1)
64    60412    88929 (47.2)     71087.7    54230.1 (-23.7)
96    71263    92439 (29.7)     145434.1   128214.0 (-11.8)
128   84208    91014 (8.0)      233668.2   238888.6 (2.2)
_____________________________________________________________
RR: 37.3%     SD: -6.7%
_____________________________________________________________

Thanks,

- KK

^ permalink raw reply

* Re: [PERF RESULTS] virtio and vhost-net performance enhancements
From: Krishna Kumar2 @ 2011-05-26 16:21 UTC (permalink / raw)
  To: Shirley Ma
  Cc: habanero, lguest, kvm, Carsten Otte, linux-s390,
	Michael S. Tsirkin, Heiko Carstens, linux-kernel, virtualization,
	Steve Dobbelstein, Christian Borntraeger, Tom Lendacky, netdev,
	Martin Schwidefsky, linux390
In-Reply-To: <OFE30C8D78.526FF88D-ON8725789C.00563B8C-8825789C.0056469F@us.ibm.com>

Shirley Ma <xma@us.ibm.com> wrote on 05/26/2011 09:12:22 PM:

> Could you please try TCP_RRs as well?

Right. Here's the result for TCP_RR:

__________________________________
#       RR%     SD%     CPU%
__________________________________
1       4.5       -31.4    -27.9
2       5.1       -9.7      -5.4
4       60.4     -13.4     38.8
8       67.8     -13.5     45.0
16     55.8     -8.0       43.2
32     66.9     -14.1     43.3
64     47.2     -23.7     12.2
96     29.7     -11.8     14.3
128    8.0       2.2       10.7
___________________________________
BW: 37.3%   SD: -6.7%   CPU: 15.7%
___________________________________

Thanks,

- KK

^ permalink raw reply

* Re: [PERF RESULTS] virtio and vhost-net performance enhancements
From: Shirley Ma @ 2011-05-26 15:42 UTC (permalink / raw)
  To: Krishna Kumar2
  Cc: habanero, lguest, kvm, Carsten Otte, linux-s390,
	Michael S. Tsirkin, Heiko Carstens, linux-kernel, virtualization,
	Steve Dobbelstein, Christian Borntraeger, Tom Lendacky, netdev,
	Martin Schwidefsky, linux390
In-Reply-To: <OF0EFA7BA9.BB2C2860-ON6525789C.0053EE3A-6525789C.005513CC@in.ibm.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 4977 bytes --]


Hello KK,

	Could you please try TCP_RRs as well?

Thanks
Shirley


                                                                       
             Krishna Kumar2                                            
             <krkumar2@in.ibm.                                         
             com>                                                       To
                                       "Michael S. Tsirkin"            
             05/26/2011 08:32          <mst@redhat.com>                
             AM                                                         cc
                                       Christian Borntraeger           
                                       <borntraeger@de.ibm.com>, Carsten
                                       Otte <cotte@de.ibm.com>,        
                                       habanero@linux.vnet.ibm.com, Heiko
                                       Carstens                        
                                       <heiko.carstens@de.ibm.com>,    
                                       kvm@vger.kernel.org,            
                                       lguest@lists.ozlabs.org,        
                                       linux-kernel@vger.kernel.org,   
                                       linux-s390@vger.kernel.org,     
                                       linux390@de.ibm.com,            
                                       netdev@vger.kernel.org, Rusty   
                                       Russell <rusty@rustcorp.com.au>,
                                       Martin Schwidefsky              
                                       <schwidefsky@de.ibm.com>, Steve 
                                       Dobbelstein/Austin/IBM@IBMUS, Tom
                                       Lendacky <tahm@linux.vnet.ibm.com>,
                                       virtualization@lists.linux-foundati
                                       on.org, Shirley                 
                                       Ma/Beaverton/IBM@IBMUS          
                                                                   Subject
                                       [PERF RESULTS] virtio and vhost-net
                                       performance enhancements        
                                                                       
                                                                       
                                                                       
                                                                       
                                                                       
                                                                       




"Michael S. Tsirkin" <mst@redhat.com> wrote on 05/20/2011 04:40:07 AM:

> OK, here is the large patchset that implements the virtio spec update
> that I sent earlier (the spec itself needs a minor update, will send
> that out too next week, but I think we are on the same page here
> already). It supercedes the PUBLISH_USED_IDX patches I sent
> out earlier.

I was able to get this tested by applying the v2 patches
to git-next tree (somehow MST's git tree hung on my guest
which never got resolved). Testing was from Guest -> Remote
node, using an ixgbe 10g card. The test results are
*excellent* (table: #netperf sesssions, BW% improvement,
SD% improvement, CPU% improvement):

___________________________________
           512 byte I/O
#     BW%     SD%      CPU%
____________________________________
1     151.6   -65.1    -10.7
2     180.6   -66.6    -6.4
4     15.5    -35.8    -26.1
8     1.8     -28.4    -26.7
16    3.1     -29.0    -26.5
32    1.1     -27.4    -27.5
64    3.8     -30.9    -26.7
96    5.4     -21.7    -24.2
128   5.7     -24.4    -25.5
____________________________________
BW: 16.6%   SD: -24.6%    CPU: -25.5%


____________________________________
            1K I/O
#     BW%     SD%      CPU%
____________________________________
1     233.9   -76.5    -18.0
2     112.2   -64.0    -23.2
4     9.2     -31.6    -26.1
8    -1.7     -26.8    -30.3
16    3.5     -31.5    -30.6
32    4.8     -25.2    -30.5
64    5.7     -31.0    -28.9
96    5.3     -32.2    -31.7
128   4.6     -38.2    -33.6
____________________________________
BW: 16.4%   SD: -35.%    CPU: -31.5%


____________________________________
             16K I/O
#     BW%     SD%      CPU%
____________________________________
1     18.8    -27.2    -18.3
2     14.8    -36.7    -27.7
4     12.7    -45.2    -38.1
8     4.4     -56.4    -54.4
16    4.8     -38.3    -36.1
32    0        78.0     79.2
64    3.8     -38.1    -37.5
96    7.3     -35.2    -31.1
128   3.4     -31.1    -32.1
____________________________________
BW: 7.6%   SD: -30.1%   CPU: -23.7%


I plan to run some more tests tomorrow. Please let
me know if any other scenario will help.

Thanks,

- KK


[-- Attachment #1.1.2: Type: text/html, Size: 6741 bytes --]

[-- Attachment #1.2: graycol.gif --]
[-- Type: image/gif, Size: 105 bytes --]

[-- Attachment #1.3: pic17152.gif --]
[-- Type: image/gif, Size: 1255 bytes --]

[-- Attachment #1.4: ecblank.gif --]
[-- Type: image/gif, Size: 45 bytes --]

[-- Attachment #2: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* [PERF RESULTS] virtio and vhost-net performance enhancements
From: Krishna Kumar2 @ 2011-05-26 15:32 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: habanero, lguest, Shirley Ma, kvm, Carsten Otte, linux-s390,
	Heiko Carstens, linux-kernel, virtualization, steved,
	Christian Borntraeger, Tom Lendacky, netdev, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>

"Michael S. Tsirkin" <mst@redhat.com> wrote on 05/20/2011 04:40:07 AM:

> OK, here is the large patchset that implements the virtio spec update
> that I sent earlier (the spec itself needs a minor update, will send
> that out too next week, but I think we are on the same page here
> already). It supercedes the PUBLISH_USED_IDX patches I sent
> out earlier.

I was able to get this tested by applying the v2 patches
to git-next tree (somehow MST's git tree hung on my guest
which never got resolved). Testing was from Guest -> Remote
node, using an ixgbe 10g card. The test results are
*excellent* (table: #netperf sesssions, BW% improvement,
SD% improvement, CPU% improvement):

___________________________________
           512 byte I/O
#     BW%     SD%      CPU%
____________________________________
1     151.6   -65.1    -10.7
2     180.6   -66.6    -6.4
4     15.5    -35.8    -26.1
8     1.8     -28.4    -26.7
16    3.1     -29.0    -26.5
32    1.1     -27.4    -27.5
64    3.8     -30.9    -26.7
96    5.4     -21.7    -24.2
128   5.7     -24.4    -25.5
____________________________________
BW: 16.6%   SD: -24.6%    CPU: -25.5%


____________________________________
            1K I/O
#     BW%     SD%      CPU%
____________________________________
1     233.9   -76.5    -18.0
2     112.2   -64.0    -23.2
4     9.2     -31.6    -26.1
8    -1.7     -26.8    -30.3
16    3.5     -31.5    -30.6
32    4.8     -25.2    -30.5
64    5.7     -31.0    -28.9
96    5.3     -32.2    -31.7
128   4.6     -38.2    -33.6
____________________________________
BW: 16.4%   SD: -35.%    CPU: -31.5%


____________________________________
             16K I/O
#     BW%     SD%      CPU%
____________________________________
1     18.8    -27.2    -18.3
2     14.8    -36.7    -27.7
4     12.7    -45.2    -38.1
8     4.4     -56.4    -54.4
16    4.8     -38.3    -36.1
32    0        78.0     79.2
64    3.8     -38.1    -37.5
96    7.3     -35.2    -31.1
128   3.4     -31.1    -32.1
____________________________________
BW: 7.6%   SD: -30.1%   CPU: -23.7%


I plan to run some more tests tomorrow. Please let
me know if any other scenario will help.

Thanks,

- KK

^ permalink raw reply

* Re: [PATCHv2 10/14] virtio_net: limit xmit polling
From: Rusty Russell @ 2011-05-26  3:28 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <20110525060759.GC26352@redhat.com>

On Wed, 25 May 2011 09:07:59 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, May 25, 2011 at 11:05:04AM +0930, Rusty Russell wrote:
> Hmm I'm not sure I got it, need to think about this.
> I'd like to go back and document how my design was supposed to work.
> This really should have been in commit log or even a comment.
> I thought we need a min, not a max.
> We start with this:
> 
> 	while ((c = (virtqueue_get_capacity(vq) < 2 + MAX_SKB_FRAGS) &&
> 		(skb = get_buf)))
> 		kfree_skb(skb);
> 	return !c;
> 
> This is clean and simple, right? And it's exactly asking for what we need.

No, I started from the other direction:

        for (i = 0; i < 2; i++) {
                skb = get_buf();
                if (!skb)
                        break;
                kfree_skb(skb);
        }

ie. free two packets for every one we're about to add.  For steady state
that would work really well.  Then we hit the case where the ring seems
full after we do the add: at that point, screw latency, and just try to
free all the buffers we can.

> on the normal path min == 2 so we're low latency but we keep ahead on
> average. min == 0 for the "we're out of capacity, we may have to stop
> the queue".
> 
> Does the above make sense at all?

It makes sense, but I think it's a classic case where incremental
improvements aren't as good as starting from scratch.

Cheers,
Rusty.

^ permalink raw reply

* [PATCH 1/1] staging: hv: remove netvsc send buffer and related functions
From: Haiyang Zhang @ 2011-05-25 22:02 UTC (permalink / raw)
  To: haiyangz, hjanssen, kys, v-abkane, gregkh, linux-kernel, devel,
	vir

netvsc send buffer is not used, so remove it.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/staging/hv/hyperv_net.h |   10 ---
 drivers/staging/hv/netvsc.c     |  161 ---------------------------------------
 2 files changed, 0 insertions(+), 171 deletions(-)

diff --git a/drivers/staging/hv/hyperv_net.h b/drivers/staging/hv/hyperv_net.h
index cf762bd..27f987b 100644
--- a/drivers/staging/hv/hyperv_net.h
+++ b/drivers/staging/hv/hyperv_net.h
@@ -355,10 +355,6 @@ struct nvsp_message {
 /* #define NVSC_MIN_PROTOCOL_VERSION		1 */
 /* #define NVSC_MAX_PROTOCOL_VERSION		1 */
 
-#define NETVSC_SEND_BUFFER_SIZE			(64*1024)	/* 64K */
-#define NETVSC_SEND_BUFFER_ID			0xface
-
-
 #define NETVSC_RECEIVE_BUFFER_SIZE		(1024*1024)	/* 1MB */
 
 #define NETVSC_RECEIVE_BUFFER_ID		0xcafe
@@ -383,12 +379,6 @@ struct netvsc_device {
 	struct list_head recv_pkt_list;
 	spinlock_t recv_pkt_list_lock;
 
-	/* Send buffer allocated by us but manages by NetVSP */
-	void *send_buf;
-	u32 send_buf_size;
-	u32 send_buf_gpadl_handle;
-	u32 send_section_size;
-
 	/* Receive buffer allocated by us but manages by NetVSP */
 	void *recv_buf;
 	u32 recv_buf_size;
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index 41cbb26..7b5bf0d 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -323,162 +323,6 @@ exit:
 	return ret;
 }
 
-static int netvsc_destroy_send_buf(struct netvsc_device *net_device)
-{
-	struct nvsp_message *revoke_packet;
-	int ret = 0;
-
-	/*
-	 * If we got a section count, it means we received a
-	 *  SendReceiveBufferComplete msg (ie sent
-	 *  NvspMessage1TypeSendReceiveBuffer msg) therefore, we need
-	 *  to send a revoke msg here
-	 */
-	if (net_device->send_section_size) {
-		/* Send the revoke send buffer */
-		revoke_packet = &net_device->revoke_packet;
-		memset(revoke_packet, 0, sizeof(struct nvsp_message));
-
-		revoke_packet->hdr.msg_type =
-			NVSP_MSG1_TYPE_REVOKE_SEND_BUF;
-		revoke_packet->msg.v1_msg.
-			revoke_send_buf.id = NETVSC_SEND_BUFFER_ID;
-
-		ret = vmbus_sendpacket(net_device->dev->channel,
-				       revoke_packet,
-				       sizeof(struct nvsp_message),
-				       (unsigned long)revoke_packet,
-				       VM_PKT_DATA_INBAND, 0);
-		/*
-		 * If we failed here, we might as well return and have a leak
-		 * rather than continue and a bugchk
-		 */
-		if (ret != 0) {
-			dev_err(&net_device->dev->device, "unable to send "
-				"revoke send buffer to netvsp");
-			return -1;
-		}
-	}
-
-	/* Teardown the gpadl on the vsp end */
-	if (net_device->send_buf_gpadl_handle) {
-		ret = vmbus_teardown_gpadl(net_device->dev->channel,
-					   net_device->send_buf_gpadl_handle);
-
-		/*
-		 * If we failed here, we might as well return and have a leak
-		 * rather than continue and a bugchk
-		 */
-		if (ret != 0) {
-			dev_err(&net_device->dev->device,
-				"unable to teardown send buffer's gpadl");
-			return -1;
-		}
-		net_device->send_buf_gpadl_handle = 0;
-	}
-
-	if (net_device->send_buf) {
-		/* Free up the receive buffer */
-		free_pages((unsigned long)net_device->send_buf,
-				get_order(net_device->send_buf_size));
-		net_device->send_buf = NULL;
-	}
-
-	return ret;
-}
-
-static int netvsc_init_send_buf(struct hv_device *device)
-{
-	int ret = 0;
-	int t;
-	struct netvsc_device *net_device;
-	struct nvsp_message *init_packet;
-
-	net_device = get_outbound_net_device(device);
-	if (!net_device) {
-		dev_err(&device->device, "unable to get net device..."
-			   "device being destroyed?");
-		return -1;
-	}
-	if (net_device->send_buf_size <= 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
-
-	net_device->send_buf =
-		(void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
-				get_order(net_device->send_buf_size));
-	if (!net_device->send_buf) {
-		dev_err(&device->device, "unable to allocate send "
-			"buffer of size %d", net_device->send_buf_size);
-		ret = -1;
-		goto cleanup;
-	}
-
-	/*
-	 * Establish the gpadl handle for this buffer on this
-	 * channel.  Note: This call uses the vmbus connection rather
-	 * than the channel to establish the gpadl handle.
-	 */
-	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
-				    net_device->send_buf_size,
-				    &net_device->send_buf_gpadl_handle);
-	if (ret != 0) {
-		dev_err(&device->device, "unable to establish send buffer's gpadl");
-		goto cleanup;
-	}
-
-	/* Notify the NetVsp of the gpadl handle */
-	init_packet = &net_device->channel_init_pkt;
-
-	memset(init_packet, 0, sizeof(struct nvsp_message));
-
-	init_packet->hdr.msg_type = NVSP_MSG1_TYPE_SEND_SEND_BUF;
-	init_packet->msg.v1_msg.send_recv_buf.
-		gpadl_handle = net_device->send_buf_gpadl_handle;
-	init_packet->msg.v1_msg.send_recv_buf.id =
-		NETVSC_SEND_BUFFER_ID;
-
-	/* Send the gpadl notification request */
-	ret = vmbus_sendpacket(device->channel, init_packet,
-			       sizeof(struct nvsp_message),
-			       (unsigned long)init_packet,
-			       VM_PKT_DATA_INBAND,
-			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-	if (ret != 0) {
-		dev_err(&device->device,
-			   "unable to send receive buffer's gpadl to netvsp");
-		goto cleanup;
-	}
-
-	t = wait_for_completion_timeout(&net_device->channel_init_wait, HZ);
-
-	BUG_ON(t == 0);
-
-	/* Check the response */
-	if (init_packet->msg.v1_msg.
-	    send_send_buf_complete.status != NVSP_STAT_SUCCESS) {
-		dev_err(&device->device, "Unable to complete send buffer "
-			   "initialzation with NetVsp - status %d",
-			   init_packet->msg.v1_msg.
-			   send_send_buf_complete.status);
-		ret = -1;
-		goto cleanup;
-	}
-
-	net_device->send_section_size = init_packet->
-	msg.v1_msg.send_send_buf_complete.section_size;
-
-	goto exit;
-
-cleanup:
-	netvsc_destroy_send_buf(net_device);
-
-exit:
-	put_net_device(device);
-	return ret;
-}
-
 
 static int netvsc_connect_vsp(struct hv_device *device)
 {
@@ -556,8 +400,6 @@ static int netvsc_connect_vsp(struct hv_device *device)
 
 	/* Post the big receive buffer to NetVSP */
 	ret = netvsc_init_recv_buf(device);
-	if (ret == 0)
-		ret = netvsc_init_send_buf(device);
 
 cleanup:
 	put_net_device(device);
@@ -567,7 +409,6 @@ cleanup:
 static void netvsc_disconnect_vsp(struct netvsc_device *net_device)
 {
 	netvsc_destroy_recv_buf(net_device);
-	netvsc_destroy_send_buf(net_device);
 }
 
 /*
@@ -1099,8 +940,6 @@ int netvsc_device_add(struct hv_device *device, void *additional_info)
 	net_device->recv_buf_size = NETVSC_RECEIVE_BUFFER_SIZE;
 	spin_lock_init(&net_device->recv_pkt_list_lock);
 
-	net_device->send_buf_size = NETVSC_SEND_BUFFER_SIZE;
-
 	INIT_LIST_HEAD(&net_device->recv_pkt_list);
 
 	for (i = 0; i < NETVSC_RECEIVE_PACKETLIST_COUNT; i++) {
-- 
1.6.3.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox