[PATCH RFC net-next] virtio_net: refill buffer right after being used

To even the latency, refill buffer right after being used.

Sign-off-by: Shirley Ma [off-list ref]
---

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0c7321c..c8201d4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -429,6 +429,22 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
 	return err;
 }
 
+static bool fill_one(struct virtio_net *vi, gfp_t gfp)
+{
+	int err;
+
+	if (vi->mergeable_rx_bufs)
+		err = add_recvbuf_mergeable(vi, gfp);
+	else if (vi->big_packets)
+		err = add_recvbuf_big(vi, gfp);
+	else
+		err = add_recvbuf_small(vi, gfp);
+
+	if (err >= 0)
+		++vi->num;
+	return err;
+}
+
 /* Returns false if we couldn't fill entirely (OOM). */
 static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 {

@@ -436,17 +452,10 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 	bool oom;
 
 	do {
-		if (vi->mergeable_rx_bufs)
-			err = add_recvbuf_mergeable(vi, gfp);
-		else if (vi->big_packets)
-			err = add_recvbuf_big(vi, gfp);
-		else
-			err = add_recvbuf_small(vi, gfp);
-
+		err = fill_one(vi, gfp);
 		oom = err == -ENOMEM;
 		if (err < 0)
 			break;
-		++vi->num;
 	} while (err > 0);
 	if (unlikely(vi->num > vi->max))
 		vi->max = vi->num;

@@ -506,13 +515,13 @@ again:
 		receive_buf(vi->dev, buf, len);
 		--vi->num;
 		received++;
-	}
-
-	if (vi->num < vi->max / 2) {
-		if (!try_fill_recv(vi, GFP_ATOMIC))
+		if (fill_one(vi, GFP_ATOMIC) < 0)
 			schedule_delayed_work(&vi->refill, 0);
 	}
 
+	/* notify buffers are refilled */
+	virtqueue_kick(vi->rvq);
+
 	/* Out of packets? */
 	if (received < budget) {
 		napi_complete(napi);

Resubmit it with a typo fix.

Signed-off-by: Shirley Ma <redacted>
---

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0c7321c..c8201d4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -429,6 +429,22 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
 	return err;
 }
 
+static int fill_one(struct virtnet_info *vi, gfp_t gfp)
+{
+	int err;
+
+	if (vi->mergeable_rx_bufs)
+		err = add_recvbuf_mergeable(vi, gfp);
+	else if (vi->big_packets)
+		err = add_recvbuf_big(vi, gfp);
+	else
+		err = add_recvbuf_small(vi, gfp);
+
+	if (err >= 0)
+		++vi->num;
+	return err;
+}
+
 /* Returns false if we couldn't fill entirely (OOM). */
 static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 {

@@ -436,17 +452,10 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 	bool oom;
 
 	do {
-		if (vi->mergeable_rx_bufs)
-			err = add_recvbuf_mergeable(vi, gfp);
-		else if (vi->big_packets)
-			err = add_recvbuf_big(vi, gfp);
-		else
-			err = add_recvbuf_small(vi, gfp);
-
+		err = fill_one(vi, gfp);
 		oom = err == -ENOMEM;
 		if (err < 0)
 			break;
-		++vi->num;
 	} while (err > 0);
 	if (unlikely(vi->num > vi->max))
 		vi->max = vi->num;

@@ -506,13 +515,13 @@ again:
 		receive_buf(vi->dev, buf, len);
 		--vi->num;
 		received++;
-	}
-
-	if (vi->num < vi->max / 2) {
-		if (!try_fill_recv(vi, GFP_ATOMIC))
+		if (fill_one(vi, GFP_ATOMIC) < 0)
 			schedule_delayed_work(&vi->refill, 0);
 	}
 
+	/* notify buffers are refilled */
+	virtqueue_kick(vi->rvq);
+
 	/* Out of packets? */
 	if (received < budget) {
 		napi_complete(napi);

On Fri, Jul 29, 2011 at 3:55 PM, Shirley Ma [off-list ref] wrote:

How does this reduce latency?   We are doing the same amount of work
in both cases, and in both cases the newly available buffers are not
visible to the device until the virtqueue_kick..

       /* Out of packets? */
       if (received < budget) {
               napi_complete(napi);


--
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

On Fri, 2011-07-29 at 16:58 -0700, Mike Waychison wrote:

On Fri, Jul 29, 2011 at 3:55 PM, Shirley Ma [off-list ref]
wrote:

quoted

Resubmit it with a typo fix.

Signed-off-by: Shirley Ma <redacted>
---

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0c7321c..c8201d4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -429,6 +429,22 @@ static int add_recvbuf_mergeable(struct

virtnet_info *vi, gfp_t gfp)

quoted

       return err;
 }

+static int fill_one(struct virtnet_info *vi, gfp_t gfp)
+{
+       int err;
+
+       if (vi->mergeable_rx_bufs)
+               err = add_recvbuf_mergeable(vi, gfp);
+       else if (vi->big_packets)
+               err = add_recvbuf_big(vi, gfp);
+       else
+               err = add_recvbuf_small(vi, gfp);
+
+       if (err >= 0)
+               ++vi->num;
+       return err;
+}
+
 /* Returns false if we couldn't fill entirely (OOM). */
 static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 {

@@ -436,17 +452,10 @@ static bool try_fill_recv(struct virtnet_info

*vi, gfp_t gfp)

quoted

       bool oom;

       do {
-               if (vi->mergeable_rx_bufs)
-                       err = add_recvbuf_mergeable(vi, gfp);
-               else if (vi->big_packets)
-                       err = add_recvbuf_big(vi, gfp);
-               else
-                       err = add_recvbuf_small(vi, gfp);
-
+               err = fill_one(vi, gfp);
               oom = err == -ENOMEM;
               if (err < 0)
                       break;
-               ++vi->num;
       } while (err > 0);
       if (unlikely(vi->num > vi->max))
               vi->max = vi->num;

@@ -506,13 +515,13 @@ again:
               receive_buf(vi->dev, buf, len);
               --vi->num;
               received++;
-       }
-
-       if (vi->num < vi->max / 2) {
-               if (!try_fill_recv(vi, GFP_ATOMIC))
+               if (fill_one(vi, GFP_ATOMIC) < 0)
                       schedule_delayed_work(&vi->refill, 0);
       }

+       /* notify buffers are refilled */
+       virtqueue_kick(vi->rvq);
+

How does this reduce latency?   We are doing the same amount of work
in both cases, and in both cases the newly available buffers are not
visible to the device until the virtqueue_kick..

It averages the latency between each receive by filling only one set of
buffers vs. either none buffers or 1/2 ring size buffers fill between
receives.

quoted

       /* Out of packets? */
       if (received < budget) {
               napi_complete(napi);


-- 

On Fri, Jul 29, 2011 at 03:55:31PM -0700, Shirley Ma wrote:

If we get a large packet, we might add less that what we removed from
the ring.  Isn't this a problem?

 	}
 
+	/* notify buffers are refilled */
+	virtqueue_kick(vi->rvq);
+
 	/* Out of packets? */
 	if (received < budget) {
 		napi_complete(napi);

-- 
MST

On Sat, Jul 30, 2011 at 10:16:15PM -0700, Shirley Ma wrote:

It averages the latency between each receive by filling only one set of
buffers vs. either none buffers or 1/2 ring size buffers fill between
receives.

I see how the overhead of allocating memory is spread more evenly.  Does this
actually help some workloads?