[PATCH] Fix guest memory leak and panic

[PATCH] Fix guest memory leak and panic

[Krishna Kumar]

Commit 86ee8130 ("virtionet: convert to SKB paged frag API")
introduced a bug in guest. During RX testing, guest runs out
of memory within seconds, causing oom-killer; which then
panics the system: "Kernel panic - not syncing: Out of memory
and no killable processes...". /proc/meminfo just before the
panic shows MemFree is a few MB's:

	MemFree:         1928544 kB	(starts here)
	...
	...
	MemFree:           27488 kB
	MemFree:           26248 kB
	MemFree:           24636 kB
	MemFree:           22632 kB
	MemFree:           19580 kB
	MemFree:           17928 kB
	MemFree:           15548 kB
		(Panic)

The extra reference to the fragment pages causes those pages to
not get freed in skb_release_data(). The following patch fixes
the bug. I have not checked if any other converted driver has
the same issue.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 drivers/net/virtio_net.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c
--- org/drivers/net/virtio_net.c	2011-10-18 08:49:46.000000000 +0530
+++ new/drivers/net/virtio_net.c	2011-10-18 12:55:32.000000000 +0530
@@ -143,18 +143,15 @@ static void skb_xmit_done(struct virtque
 static void set_skb_frag(struct sk_buff *skb, struct page *page,
 			 unsigned int offset, unsigned int *len)
 {
+	int size = min((unsigned)PAGE_SIZE - offset, *len);
 	int i = skb_shinfo(skb)->nr_frags;
-	skb_frag_t *f;
 
-	f = &skb_shinfo(skb)->frags[i];
-	f->size = min((unsigned)PAGE_SIZE - offset, *len);
-	f->page_offset = offset;
-	__skb_frag_set_page(f, page);
+	__skb_fill_page_desc(skb, i, page, offset, size);
 
-	skb->data_len += f->size;
-	skb->len += f->size;
+	skb->data_len += size;
+	skb->len += size;
 	skb_shinfo(skb)->nr_frags++;
-	*len -= f->size;
+	*len -= size;
 }
 
 static struct sk_buff *page_to_skb(struct virtnet_info *vi,

Re: [PATCH] Fix guest memory leak and panic

[Ian Campbell]

On Tue, 2011-10-18 at 09:05 +0100, Krishna Kumar wrote:
> The extra reference to the fragment pages causes those pages to
> not get freed in skb_release_data(). The following patch fixes
> the bug. I have not checked if any other converted driver has
> the same issue.

Damn. You are completely correct and I appear to have made this same
mistake several times. A quick look suggests that at least cxbg,
myriage, vmxnet, cassini and bnx2 may potentially have a similar
issue :-( (I stopped looking at that point, I'll obviously do a full
audit).

I considered quite carefully whether (__)skb_frag_set_page should take a
reference or not and decided yes but I'm starting to reconsider whether
I made the right choice. It seems that is just confusing and violates
the principal of least surprise to have a function called "set" take a
new reference. In reality all existing drivers expect that adding a page
to an SKB frag will just take over the existing reference.

I think the best thing might be to remove the additional ref taking from
the setter function and audit the previous changes to ensure they
conform. I'll do that right away and post a fixup patch ASAP.

> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>

This change is correct as things stand today, so:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

but perhaps it would be better to hold off and let me fix all of these
all at once.

Thanks for bringing this to my attention Krishna.

Ian.

Re: [PATCH] Fix guest memory leak and panic

[Ian Campbell]

On Tue, 2011-10-18 at 09:36 +0100, Ian Campbell wrote:
> I think the best thing might be to remove the additional ref taking from
> the setter function and audit the previous changes to ensure they
> conform. I'll do that right away and post a fixup patch ASAP.

Sigh, only one out of the ten callers of (__)skb_frag_set_page expects
skb_frag_set_page to take a new reference. I think that's pretty
comprehensive evidence that the current behaviour is unexpected and
wrong. 

Sorry about this.

Ian.

8<---------------------------------------------------------

>From 42c26b7ca640bd5cb6f9c3bc76db96c92ed8ff82 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 18 Oct 2011 09:59:37 +0100
Subject: [PATCH] net: do not take an additional reference in skb_frag_set_page

I audited all of the callers in the tree and only one of them (pktgen) expects
it to do so. Taking this reference is pretty obviously confusing and error
prone.

In particular I looked at the following commits which switched callers of
(__)skb_frag_set_page to the skb paged fragment api:

6a930b9f163d7e6d9ef692e05616c4ede65038ec cxgb3: convert to SKB paged frag API.
5dc3e196ea21e833128d51eb5b788a070fea1f28 myri10ge: convert to SKB paged frag API.
0e0634d20dd670a89af19af2a686a6cce943ac14 vmxnet3: convert to SKB paged frag API.
86ee8130a46769f73f8f423f99dbf782a09f9233 virtionet: convert to SKB paged frag API.
4a22c4c919c201c2a7f4ee09e672435a3072d875 sfc: convert to SKB paged frag API.
18324d690d6a5028e3c174fc1921447aedead2b8 cassini: convert to SKB paged frag API.
b061b39e3ae18ad75466258cf2116e18fa5bbd80 benet: convert to SKB paged frag API.
b7b6a688d217936459ff5cf1087b2361db952509 bnx2: convert to SKB paged frag API.
804cf14ea5ceca46554d5801e2817bba8116b7e5 net: xfrm: convert to SKB frag APIs
ea2ab69379a941c6f8884e290fdd28c93936a778 net: convert core to skb paged frag APIs

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 include/linux/skbuff.h |    1 -
 net/core/pktgen.c      |    1 +
 2 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 64f8695..78741da 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1765,7 +1765,6 @@ static inline void *skb_frag_address_safe(const skb_frag_t *frag)
 static inline void __skb_frag_set_page(skb_frag_t *frag, struct page *page)
 {
 	frag->page = page;
-	__skb_frag_ref(frag);
 }
 
 /**
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 796044a..c4effd4 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2603,6 +2603,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
 					break;
 			}
 			skb_frag_set_page(skb, i, pkt_dev->page);
+			skb_frag_ref(skb, i);
 			skb_shinfo(skb)->frags[i].page_offset = 0;
 			/*last fragment, fill rest of data*/
 			if (i == (frags - 1))
-- 
1.7.2.5

Re: [PATCH] Fix guest memory leak and panic

[Krishna Kumar2]

Ian Campbell <Ian.Campbell@citrix.com> wrote on 10/18/2011 03:17:11 PM:

> > I think the best thing might be to remove the additional ref taking
from
> > the setter function and audit the previous changes to ensure they
> > conform. I'll do that right away and post a fixup patch ASAP.
>
> Sigh, only one out of the ten callers of (__)skb_frag_set_page expects
> skb_frag_set_page to take a new reference. I think that's pretty
> comprehensive evidence that the current behaviour is unexpected and
> wrong.

Looks good!

Does it make sense to commit both of these patches? The
reason being - my patch becomes a cleanup of set_skb_frag()
in virtio_net driver.

thanks,

- KK

Re: [PATCH] Fix guest memory leak and panic

[Ian Campbell]

On Tue, 2011-10-18 at 11:46 +0100, Krishna Kumar2 wrote:
> Ian Campbell <Ian.Campbell@citrix.com> wrote on 10/18/2011 03:17:11 PM:
> 
> > > I think the best thing might be to remove the additional ref taking
> from
> > > the setter function and audit the previous changes to ensure they
> > > conform. I'll do that right away and post a fixup patch ASAP.
> >
> > Sigh, only one out of the ten callers of (__)skb_frag_set_page expects
> > skb_frag_set_page to take a new reference. I think that's pretty
> > comprehensive evidence that the current behaviour is unexpected and
> > wrong.
> 
> Looks good!

Thanks.

> Does it make sense to commit both of these patches? The
> reason being - my patch becomes a cleanup of set_skb_frag()
> in virtio_net driver.

I think your patch remains a valid cleanup but I don't maintain that
code.

Ian.

Re: [PATCH] Fix guest memory leak and panic

[Krishna Kumar2]

Ian Campbell <Ian.Campbell@citrix.com> wrote on 10/18/2011 04:20:55 PM:

> > > Sigh, only one out of the ten callers of (__)skb_frag_set_page
expects
> > > skb_frag_set_page to take a new reference. I think that's pretty
> > > comprehensive evidence that the current behaviour is unexpected and
> > > wrong.
> >
> > Looks good!
>
> Thanks.

Tested this patch for virtio_net - hang/panic is fixed.
MemFree also doesn't reduce at end of test compared to
the start.

Tested-by: krkumar2@in.ibm.com

- KK

> > Does it make sense to commit both of these patches? The
> > reason being - my patch becomes a cleanup of set_skb_frag()
> > in virtio_net driver.
>
> I think your patch remains a valid cleanup but I don't maintain that
> code.
>
> Ian.
>
>

Re: [PATCH] Fix guest memory leak and panic

[David Miller]

Krishna could you please respin your original patch, fixing the subject
and adjusting the commit message to indicate this is a cleanup.

Assume I have applied Ian's patch, because that's what I'm about to
do.

Thanks!

Re: [PATCH] Fix guest memory leak and panic

[Eric Dumazet]

Le mardi 18 octobre 2011 à 10:47 +0100, Ian Campbell a écrit :
> On Tue, 2011-10-18 at 09:36 +0100, Ian Campbell wrote:
> > I think the best thing might be to remove the additional ref taking from
> > the setter function and audit the previous changes to ensure they
> > conform. I'll do that right away and post a fixup patch ASAP.
> 
> Sigh, only one out of the ten callers of (__)skb_frag_set_page expects
> skb_frag_set_page to take a new reference. I think that's pretty
> comprehensive evidence that the current behaviour is unexpected and
> wrong. 
> 
> Sorry about this.
> 
> Ian.
> 
> 8<---------------------------------------------------------
> 
> From 42c26b7ca640bd5cb6f9c3bc76db96c92ed8ff82 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Tue, 18 Oct 2011 09:59:37 +0100
> Subject: [PATCH] net: do not take an additional reference in skb_frag_set_page
> 
> I audited all of the callers in the tree and only one of them (pktgen) expects
> it to do so. Taking this reference is pretty obviously confusing and error
> prone.
> 
> In particular I looked at the following commits which switched callers of
> (__)skb_frag_set_page to the skb paged fragment api:
> 
> 6a930b9f163d7e6d9ef692e05616c4ede65038ec cxgb3: convert to SKB paged frag API.
> 5dc3e196ea21e833128d51eb5b788a070fea1f28 myri10ge: convert to SKB paged frag API.
> 0e0634d20dd670a89af19af2a686a6cce943ac14 vmxnet3: convert to SKB paged frag API.
> 86ee8130a46769f73f8f423f99dbf782a09f9233 virtionet: convert to SKB paged frag API.
> 4a22c4c919c201c2a7f4ee09e672435a3072d875 sfc: convert to SKB paged frag API.
> 18324d690d6a5028e3c174fc1921447aedead2b8 cassini: convert to SKB paged frag API.
> b061b39e3ae18ad75466258cf2116e18fa5bbd80 benet: convert to SKB paged frag API.
> b7b6a688d217936459ff5cf1087b2361db952509 bnx2: convert to SKB paged frag API.
> 804cf14ea5ceca46554d5801e2817bba8116b7e5 net: xfrm: convert to SKB frag APIs
> ea2ab69379a941c6f8884e290fdd28c93936a778 net: convert core to skb paged frag APIs
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  include/linux/skbuff.h |    1 -
>  net/core/pktgen.c      |    1 +
>  2 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 64f8695..78741da 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1765,7 +1765,6 @@ static inline void *skb_frag_address_safe(const skb_frag_t *frag)
>  static inline void __skb_frag_set_page(skb_frag_t *frag, struct page *page)
>  {
>  	frag->page = page;
> -	__skb_frag_ref(frag);
>  }
>  
>  /**
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 796044a..c4effd4 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2603,6 +2603,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
>  					break;
>  			}
>  			skb_frag_set_page(skb, i, pkt_dev->page);
> +			skb_frag_ref(skb, i);
>  			skb_shinfo(skb)->frags[i].page_offset = 0;
>  			/*last fragment, fill rest of data*/
>  			if (i == (frags - 1))

I am ok with this patch, since it makes sense to let driver do the
page->_count change itself (it can use a batched add() in case a page is
splitted in many frags)

But I suggest using get_page(pkt_dev->page), this seems more obvious to
me [ This was how I wrote the thing ;) ]

Re: [PATCH] Fix guest memory leak and panic

[Ian Campbell]

On Tue, 2011-10-18 at 14:20 +0100, Eric Dumazet wrote:
> But I suggest using get_page(pkt_dev->page), this seems more obvious to
> me [ This was how I wrote the thing ;) ]

I guess it depends on whether you consider the reference to be on the
page or to be on the frag (which contains the page). The distinction
would only matter if pktgen were to transition to using the forthcoming
destructors though.

Here's a version like you describe:

8<---------------------------------------------------------------

>From 369022220db31efb9c261cbabcb890a4d216a176 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 18 Oct 2011 09:59:37 +0100
Subject: [PATCH] net: do not take an additional reference in skb_frag_set_page

I audited all of the callers in the tree and only one of them (pktgen) expects
it to do so. Taking this reference is pretty obviously confusing and error
prone.

In particular I looked at the following commits which switched callers of
(__)skb_frag_set_page to the skb paged fragment api:

6a930b9f163d7e6d9ef692e05616c4ede65038ec cxgb3: convert to SKB paged frag API.
5dc3e196ea21e833128d51eb5b788a070fea1f28 myri10ge: convert to SKB paged frag API.
0e0634d20dd670a89af19af2a686a6cce943ac14 vmxnet3: convert to SKB paged frag API.
86ee8130a46769f73f8f423f99dbf782a09f9233 virtionet: convert to SKB paged frag API.
4a22c4c919c201c2a7f4ee09e672435a3072d875 sfc: convert to SKB paged frag API.
18324d690d6a5028e3c174fc1921447aedead2b8 cassini: convert to SKB paged frag API.
b061b39e3ae18ad75466258cf2116e18fa5bbd80 benet: convert to SKB paged frag API.
b7b6a688d217936459ff5cf1087b2361db952509 bnx2: convert to SKB paged frag API.
804cf14ea5ceca46554d5801e2817bba8116b7e5 net: xfrm: convert to SKB frag APIs
ea2ab69379a941c6f8884e290fdd28c93936a778 net: convert core to skb paged frag APIs

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 include/linux/skbuff.h |    1 -
 net/core/pktgen.c      |    1 +
 2 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 64f8695..78741da 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1765,7 +1765,6 @@ static inline void *skb_frag_address_safe(const skb_frag_t *frag)
 static inline void __skb_frag_set_page(skb_frag_t *frag, struct page *page)
 {
 	frag->page = page;
-	__skb_frag_ref(frag);
 }
 
 /**
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 796044a..e81f5d0 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2602,6 +2602,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
 				if (!pkt_dev->page)
 					break;
 			}
+			get_page(pkt_dev->page);
 			skb_frag_set_page(skb, i, pkt_dev->page);
 			skb_shinfo(skb)->frags[i].page_offset = 0;
 			/*last fragment, fill rest of data*/
-- 
1.7.2.5

Re: [PATCH] Fix guest memory leak and panic

[Eric Dumazet]

Le mercredi 19 octobre 2011 à 09:55 +0100, Ian Campbell a écrit :
> On Tue, 2011-10-18 at 14:20 +0100, Eric Dumazet wrote:
> > But I suggest using get_page(pkt_dev->page), this seems more obvious to
> > me [ This was how I wrote the thing ;) ]
> 
> I guess it depends on whether you consider the reference to be on the
> page or to be on the frag (which contains the page). The distinction
> would only matter if pktgen were to transition to using the forthcoming
> destructors though.
> 

Yes, but get_page() is now a clear sign we dirty a cache line, and I am
trying to explain to driver authors this is a contention point they
should address to get best performance from sub-page frags devices.

The hidden __skb_frag_ref() in skb_frag_set_page() was misleading
them ;)

> Here's a version like you describe:
> 
> 8<---------------------------------------------------------------
> 
> From 369022220db31efb9c261cbabcb890a4d216a176 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Tue, 18 Oct 2011 09:59:37 +0100
> Subject: [PATCH] net: do not take an additional reference in skb_frag_set_page
> 
> I audited all of the callers in the tree and only one of them (pktgen) expects
> it to do so. Taking this reference is pretty obviously confusing and error
> prone.
> 
> In particular I looked at the following commits which switched callers of
> (__)skb_frag_set_page to the skb paged fragment api:
> 
> 6a930b9f163d7e6d9ef692e05616c4ede65038ec cxgb3: convert to SKB paged frag API.
> 5dc3e196ea21e833128d51eb5b788a070fea1f28 myri10ge: convert to SKB paged frag API.
> 0e0634d20dd670a89af19af2a686a6cce943ac14 vmxnet3: convert to SKB paged frag API.
> 86ee8130a46769f73f8f423f99dbf782a09f9233 virtionet: convert to SKB paged frag API.
> 4a22c4c919c201c2a7f4ee09e672435a3072d875 sfc: convert to SKB paged frag API.
> 18324d690d6a5028e3c174fc1921447aedead2b8 cassini: convert to SKB paged frag API.
> b061b39e3ae18ad75466258cf2116e18fa5bbd80 benet: convert to SKB paged frag API.
> b7b6a688d217936459ff5cf1087b2361db952509 bnx2: convert to SKB paged frag API.
> 804cf14ea5ceca46554d5801e2817bba8116b7e5 net: xfrm: convert to SKB frag APIs
> ea2ab69379a941c6f8884e290fdd28c93936a778 net: convert core to skb paged frag APIs
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  include/linux/skbuff.h |    1 -
>  net/core/pktgen.c      |    1 +
>  2 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 64f8695..78741da 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1765,7 +1765,6 @@ static inline void *skb_frag_address_safe(const skb_frag_t *frag)
>  static inline void __skb_frag_set_page(skb_frag_t *frag, struct page *page)
>  {
>  	frag->page = page;
> -	__skb_frag_ref(frag);
>  }
>  
>  /**
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 796044a..e81f5d0 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2602,6 +2602,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
>  				if (!pkt_dev->page)
>  					break;
>  			}
> +			get_page(pkt_dev->page);
>  			skb_frag_set_page(skb, i, pkt_dev->page);
>  			skb_shinfo(skb)->frags[i].page_offset = 0;
>  			/*last fragment, fill rest of data*/

Thanks !

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Re: [PATCH] Fix guest memory leak and panic

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 19 Oct 2011 11:07:39 +0200

>> From 369022220db31efb9c261cbabcb890a4d216a176 Mon Sep 17 00:00:00 2001
>> From: Ian Campbell <ian.campbell@citrix.com>
>> Date: Tue, 18 Oct 2011 09:59:37 +0100
>> Subject: [PATCH] net: do not take an additional reference in skb_frag_set_page
>> 
>> I audited all of the callers in the tree and only one of them (pktgen) expects
>> it to do so. Taking this reference is pretty obviously confusing and error
>> prone.
>> 
>> In particular I looked at the following commits which switched callers of
>> (__)skb_frag_set_page to the skb paged fragment api:
 ...
>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
 ...
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks everyone.

Re: [PATCH] Fix guest memory leak and panic

[Krishna Kumar2]

Ian Campbell <Ian.Campbell@citrix.com> wrote on 10/18/2011 02:06:56 PM:

Hi Ian,

> On Tue, 2011-10-18 at 09:05 +0100, Krishna Kumar wrote:
> > The extra reference to the fragment pages causes those pages to
> > not get freed in skb_release_data(). The following patch fixes
> > the bug. I have not checked if any other converted driver has
> > the same issue.
>
> Damn. You are completely correct and I appear to have made this same
> mistake several times. A quick look suggests that at least cxbg,
> myriage, vmxnet, cassini and bnx2 may potentially have a similar
> issue :-( (I stopped looking at that point, I'll obviously do a full
> audit).
>
> I considered quite carefully whether (__)skb_frag_set_page should take a
> reference or not and decided yes but I'm starting to reconsider whether
> I made the right choice. It seems that is just confusing and violates
> the principal of least surprise to have a function called "set" take a
> new reference. In reality all existing drivers expect that adding a page
> to an SKB frag will just take over the existing reference.
>
> I think the best thing might be to remove the additional ref taking from
> the setter function and audit the previous changes to ensure they
> conform. I'll do that right away and post a fixup patch ASAP.
>
> > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
>
> This change is correct as things stand today, so:
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> but perhaps it would be better to hold off and let me fix all of these
> all at once.

Either way is fine with me. However, besides having a fix,
I would like to remove the manual initializations in
set_skb_frag(), and substitute with __skb_fill_page_desc,
like other drivers do.

thanks,

- KK

Re: [PATCH] Fix guest memory leak and panic

[David Miller]

From: Krishna Kumar <krkumar2@in.ibm.com>
Date: Tue, 18 Oct 2011 13:35:23 +0530

> Commit 86ee8130 ("virtionet: convert to SKB paged frag API")
> introduced a bug in guest. During RX testing, guest runs out
> of memory within seconds, causing oom-killer; which then
> panics the system: "Kernel panic - not syncing: Out of memory
> and no killable processes...". /proc/meminfo just before the
> panic shows MemFree is a few MB's:
> 
> 	MemFree:         1928544 kB	(starts here)
> 	...
> 	...
> 	MemFree:           27488 kB
> 	MemFree:           26248 kB
> 	MemFree:           24636 kB
> 	MemFree:           22632 kB
> 	MemFree:           19580 kB
> 	MemFree:           17928 kB
> 	MemFree:           15548 kB
> 		(Panic)
> 
> The extra reference to the fragment pages causes those pages to
> not get freed in skb_release_data(). The following patch fixes
> the bug. I have not checked if any other converted driver has
> the same issue.
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>

I'll wait for Ian's full audit, but Krishna please use more appropriate
subject lines in future patch submissions.

This patch is fixing a problem in the virtio_net driver, so please
mention that: "Subject: [PATCH] virtio_net: Fix guest memory leak and panic"

Re: [PATCH] Fix guest memory leak and panic

[Ian Campbell]

On Tue, 2011-10-18 at 15:12 -0400, David Miller wrote:
> I'll wait for Ian's full audit,

The result of that is in
<1318931232.16132.65.camel@zakaz.uk.xensource.com> in this thread.

Thanks,
Ian.