netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC net-next] virtio_net: refill buffer right after being used
@ 2011-07-29 22:44 Shirley Ma
  2011-07-29 22:55 ` Shirley Ma
  0 siblings, 1 reply; 6+ messages in thread
From: Shirley Ma @ 2011-07-29 22:44 UTC (permalink / raw)
  To: Rusty Russell, mst; +Cc: kvm, virtualization, netdev

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

Sign-off-by: Shirley Ma <xma@us.ibm.com>
---

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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC net-next] virtio_net: refill buffer right after being used
  2011-07-29 22:44 [PATCH RFC net-next] virtio_net: refill buffer right after being used Shirley Ma
@ 2011-07-29 22:55 ` Shirley Ma
  2011-07-29 23:58   ` Mike Waychison
  2011-08-03 20:26   ` Michael S. Tsirkin
  0 siblings, 2 replies; 6+ messages in thread
From: Shirley Ma @ 2011-07-29 22:55 UTC (permalink / raw)
  To: Rusty Russell; +Cc: mst, kvm, virtualization, netdev

Resubmit it with a typo fix.

Signed-off-by: Shirley Ma <xma@us.ibm.com>
---

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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC net-next] virtio_net: refill buffer right after being used
  2011-07-29 22:55 ` Shirley Ma
@ 2011-07-29 23:58   ` Mike Waychison
  2011-07-31  5:16     ` Shirley Ma
  2011-08-03 20:26   ` Michael S. Tsirkin
  1 sibling, 1 reply; 6+ messages in thread
From: Mike Waychison @ 2011-07-29 23:58 UTC (permalink / raw)
  To: Shirley Ma; +Cc: Rusty Russell, mst, kvm, virtualization, netdev

On Fri, Jul 29, 2011 at 3:55 PM, Shirley Ma <mashirle@us.ibm.com> wrote:
> Resubmit it with a typo fix.
>
> Signed-off-by: Shirley Ma <xma@us.ibm.com>
> ---
>
> 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);
> +

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC net-next] virtio_net: refill buffer right after being used
  2011-07-29 23:58   ` Mike Waychison
@ 2011-07-31  5:16     ` Shirley Ma
  2011-08-03 20:30       ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Shirley Ma @ 2011-07-31  5:16 UTC (permalink / raw)
  To: Mike Waychison; +Cc: virtualization, netdev, kvm, mst

On Fri, 2011-07-29 at 16:58 -0700, Mike Waychison wrote:
> On Fri, Jul 29, 2011 at 3:55 PM, Shirley Ma <mashirle@us.ibm.com>
> wrote:
> > Resubmit it with a typo fix.
> >
> > Signed-off-by: Shirley Ma <xma@us.ibm.com>
> > ---
> >
> > 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);
> > +
> 
> 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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC net-next] virtio_net: refill buffer right after being used
  2011-07-29 22:55 ` Shirley Ma
  2011-07-29 23:58   ` Mike Waychison
@ 2011-08-03 20:26   ` Michael S. Tsirkin
  1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2011-08-03 20:26 UTC (permalink / raw)
  To: Shirley Ma; +Cc: Rusty Russell, kvm, virtualization, netdev

On Fri, Jul 29, 2011 at 03:55:31PM -0700, Shirley Ma wrote:
> Resubmit it with a typo fix.
> 
> Signed-off-by: Shirley Ma <xma@us.ibm.com>
> ---
> 
> 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);

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC net-next] virtio_net: refill buffer right after being used
  2011-07-31  5:16     ` Shirley Ma
@ 2011-08-03 20:30       ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2011-08-03 20:30 UTC (permalink / raw)
  To: Shirley Ma; +Cc: Mike Waychison, Rusty Russell, kvm, virtualization, netdev

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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-08-03 20:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-29 22:44 [PATCH RFC net-next] virtio_net: refill buffer right after being used Shirley Ma
2011-07-29 22:55 ` Shirley Ma
2011-07-29 23:58   ` Mike Waychison
2011-07-31  5:16     ` Shirley Ma
2011-08-03 20:30       ` Michael S. Tsirkin
2011-08-03 20:26   ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).