| Message ID | 1311980131.24300.30.camel@localhost.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > -- 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 <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); > > > > > > -- -- 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, 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); > >
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? -- 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
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);
Resubmit it with a typo fix. Signed-off-by: Shirley Ma <xma@us.ibm.com> --- -- 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