Re: [PATCH RFC 1/4] virtio_net: pass vi around

2014-10-27 Thread Rusty Russell 
Michael S. Tsirkin m...@redhat.com writes:
 Too many places poke at [rs]q-vq-vdev-priv just to get
 the the vi structure.  Let's just pass the pointer around: seems
 cleaner, and might even be faster.

Agreed, it's neater.

Acked-by: Rusty Russell ru...@rustcorp.com.au

Thanks,
Rusty.


 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  drivers/net/virtio_net.c | 36 +++-
  1 file changed, 19 insertions(+), 17 deletions(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index 57cbc7d..36f3dfc 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -241,11 +241,11 @@ static unsigned long mergeable_buf_to_ctx(void *buf, 
 unsigned int truesize)
  }
  
  /* Called from bottom half context */
 -static struct sk_buff *page_to_skb(struct receive_queue *rq,
 +static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 +struct receive_queue *rq,
  struct page *page, unsigned int offset,
  unsigned int len, unsigned int truesize)
  {
 - struct virtnet_info *vi = rq-vq-vdev-priv;
   struct sk_buff *skb;
   struct skb_vnet_hdr *hdr;
   unsigned int copy, hdr_len, hdr_padded_len;
 @@ -328,12 +328,13 @@ static struct sk_buff *receive_small(void *buf, 
 unsigned int len)
  }
  
  static struct sk_buff *receive_big(struct net_device *dev,
 +struct virtnet_info *vi,
  struct receive_queue *rq,
  void *buf,
  unsigned int len)
  {
   struct page *page = buf;
 - struct sk_buff *skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
 + struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE);
  
   if (unlikely(!skb))
   goto err;
 @@ -359,7 +360,8 @@ static struct sk_buff *receive_mergeable(struct 
 net_device *dev,
   int offset = buf - page_address(page);
   unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
  
 - struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, truesize);
 + struct sk_buff *head_skb = page_to_skb(vi, rq, page, offset, len,
 +truesize);
   struct sk_buff *curr_skb = head_skb;
  
   if (unlikely(!curr_skb))
 @@ -433,9 +435,9 @@ err_buf:
   return NULL;
  }
  
 -static void receive_buf(struct receive_queue *rq, void *buf, unsigned int 
 len)
 +static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 + void *buf, unsigned int len)
  {
 - struct virtnet_info *vi = rq-vq-vdev-priv;
   struct net_device *dev = vi-dev;
   struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
   struct sk_buff *skb;
 @@ -459,9 +461,9 @@ static void receive_buf(struct receive_queue *rq, void 
 *buf, unsigned int len)
   if (vi-mergeable_rx_bufs)
   skb = receive_mergeable(dev, vi, rq, (unsigned long)buf, len);
   else if (vi-big_packets)
 - skb = receive_big(dev, rq, buf, len);
 + skb = receive_big(dev, vi, rq, buf, len);
   else
 - skb = receive_small(buf, len);
 + skb = receive_small(vi, buf, len);
  
   if (unlikely(!skb))
   return;
 @@ -530,9 +532,9 @@ frame_err:
   dev_kfree_skb(skb);
  }
  
 -static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp)
 +static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue 
 *rq,
 +  gfp_t gfp)
  {
 - struct virtnet_info *vi = rq-vq-vdev-priv;
   struct sk_buff *skb;
   struct skb_vnet_hdr *hdr;
   int err;
 @@ -655,9 +657,9 @@ static int add_recvbuf_mergeable(struct receive_queue 
 *rq, gfp_t gfp)
   * before we're receiving packets, or from refill_work which is
   * careful to disable receiving (using napi_disable).
   */
 -static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
 +static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
 +   gfp_t gfp)
  {
 - struct virtnet_info *vi = rq-vq-vdev-priv;
   int err;
   bool oom;
  
 @@ -668,7 +670,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t 
 gfp)
   else if (vi-big_packets)
   err = add_recvbuf_big(rq, gfp);
   else
 - err = add_recvbuf_small(rq, gfp);
 + err = add_recvbuf_small(vi, rq, gfp);
  
   oom = err == -ENOMEM;
   if (err)
 @@ -717,7 +719,7 @@ static void refill_work(struct work_struct *work)
   struct receive_queue *rq = vi-rq[i];
  
   napi_disable(rq-napi);
 - still_empty = !try_fill_recv(rq, GFP_KERNEL);
 + still_empty = !try_fill_recv(vi, rq, GFP_KERNEL);
   virtnet_napi_enable(rq);
  
   /* In theory, this can happen:

RE: [PATCH RFC 1/4] virtio_net: pass vi around

2014-10-24 Thread David Laight 
From: Michael S. Tsirkin
 
 Too many places poke at [rs]q-vq-vdev-priv just to get
 the the vi structure.  Let's just pass the pointer around: seems
 cleaner, and might even be faster.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  drivers/net/virtio_net.c | 36 +++-
  1 file changed, 19 insertions(+), 17 deletions(-)
 
 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index 57cbc7d..36f3dfc 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
...
  static struct sk_buff *receive_big(struct net_device *dev,
 +struct virtnet_info *vi,

Do you need to pass 'dev' here?
Looks like it is obtainable from vi-dev (as below).

David

  struct receive_queue *rq,
  void *buf,
  unsigned int len)
  {
   struct page *page = buf;
 - struct sk_buff *skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
 + struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE);
...
 -static void receive_buf(struct receive_queue *rq, void *buf, unsigned int 
 len)
 +static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 + void *buf, unsigned int len)
  {
 - struct virtnet_info *vi = rq-vq-vdev-priv;
   struct net_device *dev = vi-dev;
...


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

Re: [PATCH RFC 1/4] virtio_net: pass vi around

2014-10-24 Thread Michael S. Tsirkin 
On Fri, Oct 24, 2014 at 10:02:15AM +, David Laight wrote:
 From: Michael S. Tsirkin
  
  Too many places poke at [rs]q-vq-vdev-priv just to get
  the the vi structure.  Let's just pass the pointer around: seems
  cleaner, and might even be faster.
  
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
   drivers/net/virtio_net.c | 36 +++-
   1 file changed, 19 insertions(+), 17 deletions(-)
  
  diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
  index 57cbc7d..36f3dfc 100644
  --- a/drivers/net/virtio_net.c
  +++ b/drivers/net/virtio_net.c
 ...
   static struct sk_buff *receive_big(struct net_device *dev,
  +  struct virtnet_info *vi,
 
 Do you need to pass 'dev' here?
 Looks like it is obtainable from vi-dev (as below).
 
   David
 
 struct receive_queue *rq,
 void *buf,
 unsigned int len)
   {
  struct page *page = buf;
  -   struct sk_buff *skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
  +   struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE);
 ...
  -static void receive_buf(struct receive_queue *rq, void *buf, unsigned int 
  len)
  +static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
  +   void *buf, unsigned int len)
   {
  -   struct virtnet_info *vi = rq-vq-vdev-priv;
  struct net_device *dev = vi-dev;
 ...

It's a matter of style, isn't it?
We have dev to hand, it seems cleaner to just pass it around.

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

[PATCH RFC 1/4] virtio_net: pass vi around

2014-10-23 Thread Michael S. Tsirkin 
Too many places poke at [rs]q-vq-vdev-priv just to get
the the vi structure.  Let's just pass the pointer around: seems
cleaner, and might even be faster.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/net/virtio_net.c | 36 +++-
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 57cbc7d..36f3dfc 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -241,11 +241,11 @@ static unsigned long mergeable_buf_to_ctx(void *buf, 
unsigned int truesize)
 }
 
 /* Called from bottom half context */
-static struct sk_buff *page_to_skb(struct receive_queue *rq,
+static struct sk_buff *page_to_skb(struct virtnet_info *vi,
+  struct receive_queue *rq,
   struct page *page, unsigned int offset,
   unsigned int len, unsigned int truesize)
 {
-   struct virtnet_info *vi = rq-vq-vdev-priv;
struct sk_buff *skb;
struct skb_vnet_hdr *hdr;
unsigned int copy, hdr_len, hdr_padded_len;
@@ -328,12 +328,13 @@ static struct sk_buff *receive_small(void *buf, unsigned 
int len)
 }
 
 static struct sk_buff *receive_big(struct net_device *dev,
+  struct virtnet_info *vi,
   struct receive_queue *rq,
   void *buf,
   unsigned int len)
 {
struct page *page = buf;
-   struct sk_buff *skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
+   struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE);
 
if (unlikely(!skb))
goto err;
@@ -359,7 +360,8 @@ static struct sk_buff *receive_mergeable(struct net_device 
*dev,
int offset = buf - page_address(page);
unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
 
-   struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, truesize);
+   struct sk_buff *head_skb = page_to_skb(vi, rq, page, offset, len,
+  truesize);
struct sk_buff *curr_skb = head_skb;
 
if (unlikely(!curr_skb))
@@ -433,9 +435,9 @@ err_buf:
return NULL;
 }
 
-static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
+static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
+   void *buf, unsigned int len)
 {
-   struct virtnet_info *vi = rq-vq-vdev-priv;
struct net_device *dev = vi-dev;
struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
struct sk_buff *skb;
@@ -459,9 +461,9 @@ static void receive_buf(struct receive_queue *rq, void 
*buf, unsigned int len)
if (vi-mergeable_rx_bufs)
skb = receive_mergeable(dev, vi, rq, (unsigned long)buf, len);
else if (vi-big_packets)
-   skb = receive_big(dev, rq, buf, len);
+   skb = receive_big(dev, vi, rq, buf, len);
else
-   skb = receive_small(buf, len);
+   skb = receive_small(vi, buf, len);
 
if (unlikely(!skb))
return;
@@ -530,9 +532,9 @@ frame_err:
dev_kfree_skb(skb);
 }
 
-static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp)
+static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
+gfp_t gfp)
 {
-   struct virtnet_info *vi = rq-vq-vdev-priv;
struct sk_buff *skb;
struct skb_vnet_hdr *hdr;
int err;
@@ -655,9 +657,9 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, 
gfp_t gfp)
  * before we're receiving packets, or from refill_work which is
  * careful to disable receiving (using napi_disable).
  */
-static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
+static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
+ gfp_t gfp)
 {
-   struct virtnet_info *vi = rq-vq-vdev-priv;
int err;
bool oom;
 
@@ -668,7 +670,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t 
gfp)
else if (vi-big_packets)
err = add_recvbuf_big(rq, gfp);
else
-   err = add_recvbuf_small(rq, gfp);
+   err = add_recvbuf_small(vi, rq, gfp);
 
oom = err == -ENOMEM;
if (err)
@@ -717,7 +719,7 @@ static void refill_work(struct work_struct *work)
struct receive_queue *rq = vi-rq[i];
 
napi_disable(rq-napi);
-   still_empty = !try_fill_recv(rq, GFP_KERNEL);
+   still_empty = !try_fill_recv(vi, rq, GFP_KERNEL);
virtnet_napi_enable(rq);
 
/* In theory, this can happen: if we don't get any buffers in
@@ -736,12 +738,12 @@ static int virtnet_receive(struct receive_queue *rq, int 
budget)