Re: PATCH v2 3/4] Defer skb allocation -- new recvbuf alloc receive calls

2009-12-15 Thread Michael S. Tsirkin
On Mon, Dec 14, 2009 at 02:08:38PM -0800, Shirley Ma wrote: On Sun, 2009-12-13 at 13:43 +0200, Michael S. Tsirkin wrote: Interesting. I think skb_goodcopy will sometimes set *page to NULL. Will the above crash then? Nope, when *page is NULL, *len is 0. Hmm. Yes, I see, it is here: +

Re: PATCH v2 3/4] Defer skb allocation -- new recvbuf alloc receive calls

2009-12-15 Thread Shirley Ma
On Tue, 2009-12-15 at 13:33 +0200, Michael S. Tsirkin wrote: So what I would suggest is, have function that just copies part of skb, and have caller open-code allocating the skb and free up pages as necessary. Yes, the updated patch has changed the function. What I am asking is why do we add

Re: PATCH v2 3/4] Defer skb allocation -- new recvbuf alloc receive calls

2009-12-15 Thread Michael S. Tsirkin
On Tue, Dec 15, 2009 at 08:25:20AM -0800, Shirley Ma wrote: On Tue, 2009-12-15 at 13:33 +0200, Michael S. Tsirkin wrote: So what I would suggest is, have function that just copies part of skb, and have caller open-code allocating the skb and free up pages as necessary. Yes, the updated

Re: PATCH v2 3/4] Defer skb allocation -- new recvbuf alloc receive calls

2009-12-14 Thread Shirley Ma
On Sun, 2009-12-13 at 13:43 +0200, Michael S. Tsirkin wrote: Interesting. I think skb_goodcopy will sometimes set *page to NULL. Will the above crash then? Nope, when *page is NULL, *len is 0. don't put empty line here. if below is part of same logical block as skb_goodcopy. Ok. Local

Re: PATCH v2 3/4] Defer skb allocation -- new recvbuf alloc receive calls

2009-12-14 Thread Shirley Ma
Hello Michael, On Mon, 2009-12-14 at 14:08 -0800, Shirley Ma wrote: + + err = vi-rvq-vq_ops-add_buf(vi-rvq, sg, 0, 2, skb); + if (err 0) + kfree_skb(skb); + else + skb_queue_head(vi-recv, skb); So why are we queueing this still? This is

Re: PATCH v2 3/4] Defer skb allocation -- new recvbuf alloc receive calls

2009-12-13 Thread Michael S. Tsirkin
On Fri, Dec 11, 2009 at 04:46:53AM -0800, Shirley Ma wrote: Signed-off-by: Shirley Ma x...@us.ibm.com - diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 100b4b9..dde8060 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -203,6