linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: zsmalloc: fix memory leak
@ 2012-04-02 14:13 Seth Jennings
  0 siblings, 0 replies; 6+ messages in thread
From: Seth Jennings @ 2012-04-02 14:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Nitin Gupta, Dan Magenheimer, Konrad Rzeszutek Wilk,
	Robert Jennings, Seth Jennings, devel, linux-kernel, linux-mm

From: Nitin Gupta <ngupta@vflare.org>

This patch fixes a memory leak in zsmalloc where the first
subpage of each zspage is leaked when the zspage is freed.

Based on 3.4-rc1.

Signed-off-by: Nitin Gupta <ngupta@vflare.org>
Acked-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/staging/zsmalloc/zsmalloc-main.c |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
index 09caa4f..917461c 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -267,33 +267,39 @@ static unsigned long obj_idx_to_offset(struct page *page,
 	return off + obj_idx * class_size;
 }
 
+static void reset_page(struct page *page)
+{
+	clear_bit(PG_private, &page->flags);
+	clear_bit(PG_private_2, &page->flags);
+	set_page_private(page, 0);
+	page->mapping = NULL;
+	page->freelist = NULL;
+	reset_page_mapcount(page);
+}
+
 static void free_zspage(struct page *first_page)
 {
-	struct page *nextp, *tmp;
+	struct page *nextp, *tmp, *head_extra;
 
 	BUG_ON(!is_first_page(first_page));
 	BUG_ON(first_page->inuse);
 
-	nextp = (struct page *)page_private(first_page);
+	head_extra = (struct page *)page_private(first_page);
 
-	clear_bit(PG_private, &first_page->flags);
-	clear_bit(PG_private_2, &first_page->flags);
-	set_page_private(first_page, 0);
-	first_page->mapping = NULL;
-	first_page->freelist = NULL;
-	reset_page_mapcount(first_page);
+	reset_page(first_page);
 	__free_page(first_page);
 
 	/* zspage with only 1 system page */
-	if (!nextp)
+	if (!head_extra)
 		return;
 
-	list_for_each_entry_safe(nextp, tmp, &nextp->lru, lru) {
+	list_for_each_entry_safe(nextp, tmp, &head_extra->lru, lru) {
 		list_del(&nextp->lru);
-		clear_bit(PG_private_2, &nextp->flags);
-		nextp->index = 0;
+		reset_page(nextp);
 		__free_page(nextp);
 	}
+	reset_page(head_extra);
+	__free_page(head_extra);
 }
 
 /* Initialize a newly allocated zspage */
-- 
1.7.5.4


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

* RE: [PATCH] staging: zsmalloc: fix memory leak
       [not found] <<1333376036-9841-1-git-send-email-sjenning@linux.vnet.ibm.com>
@ 2012-04-04 16:03 ` Dan Magenheimer
  2012-04-04 16:26   ` Seth Jennings
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Magenheimer @ 2012-04-04 16:03 UTC (permalink / raw)
  To: Seth Jennings, Greg Kroah-Hartman
  Cc: Nitin Gupta, Dan Magenheimer, Konrad Rzeszutek Wilk,
	Robert Jennings, devel, linux-kernel, linux-mm

> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> Sent: Monday, April 02, 2012 8:14 AM
> To: Greg Kroah-Hartman
> Cc: Nitin Gupta; Dan Magenheimer; Konrad Rzeszutek Wilk; Robert Jennings; Seth Jennings;
> devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org
> Subject: [PATCH] staging: zsmalloc: fix memory leak
> 
> From: Nitin Gupta <ngupta@vflare.org>
> 
> This patch fixes a memory leak in zsmalloc where the first
> subpage of each zspage is leaked when the zspage is freed.
> 
> Based on 3.4-rc1.
> 
> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
> Acked-by: Seth Jennings <sjenning@linux.vnet.ibm.com>

This is a rather severe memory leak and will affect most
benchmarking anyone does to evaluate zcache in 3.4 (e.g. as
to whether zcache is suitable for promotion), so t'would be nice
to get this patch in for -rc2.  (Note it fixes a "regression"
since it affects zcache only in 3.4+ because the fix is to
the new zsmalloc allocator... so no change to stable trees.)

Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>

> ---
>  drivers/staging/zsmalloc/zsmalloc-main.c |   30 ++++++++++++++++++------------
>  1 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index 09caa4f..917461c 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -267,33 +267,39 @@ static unsigned long obj_idx_to_offset(struct page *page,
>  	return off + obj_idx * class_size;
>  }
> 
> +static void reset_page(struct page *page)
> +{
> +	clear_bit(PG_private, &page->flags);
> +	clear_bit(PG_private_2, &page->flags);
> +	set_page_private(page, 0);
> +	page->mapping = NULL;
> +	page->freelist = NULL;
> +	reset_page_mapcount(page);
> +}
> +
>  static void free_zspage(struct page *first_page)
>  {
> -	struct page *nextp, *tmp;
> +	struct page *nextp, *tmp, *head_extra;
> 
>  	BUG_ON(!is_first_page(first_page));
>  	BUG_ON(first_page->inuse);
> 
> -	nextp = (struct page *)page_private(first_page);
> +	head_extra = (struct page *)page_private(first_page);
> 
> -	clear_bit(PG_private, &first_page->flags);
> -	clear_bit(PG_private_2, &first_page->flags);
> -	set_page_private(first_page, 0);
> -	first_page->mapping = NULL;
> -	first_page->freelist = NULL;
> -	reset_page_mapcount(first_page);
> +	reset_page(first_page);
>  	__free_page(first_page);
> 
>  	/* zspage with only 1 system page */
> -	if (!nextp)
> +	if (!head_extra)
>  		return;
> 
> -	list_for_each_entry_safe(nextp, tmp, &nextp->lru, lru) {
> +	list_for_each_entry_safe(nextp, tmp, &head_extra->lru, lru) {
>  		list_del(&nextp->lru);
> -		clear_bit(PG_private_2, &nextp->flags);
> -		nextp->index = 0;
> +		reset_page(nextp);
>  		__free_page(nextp);
>  	}
> +	reset_page(head_extra);
> +	__free_page(head_extra);
>  }
> 
>  /* Initialize a newly allocated zspage */
> --
> 1.7.5.4

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

* Re: [PATCH] staging: zsmalloc: fix memory leak
  2012-04-04 16:03 ` [PATCH] staging: zsmalloc: fix memory leak Dan Magenheimer
@ 2012-04-04 16:26   ` Seth Jennings
  2012-04-09 20:23     ` Seth Jennings
  0 siblings, 1 reply; 6+ messages in thread
From: Seth Jennings @ 2012-04-04 16:26 UTC (permalink / raw)
  To: Dan Magenheimer
  Cc: Greg Kroah-Hartman, Nitin Gupta, Konrad Rzeszutek Wilk,
	Robert Jennings, devel, linux-kernel, linux-mm

On 04/04/2012 11:03 AM, Dan Magenheimer wrote:
>> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
>> Sent: Monday, April 02, 2012 8:14 AM
>> To: Greg Kroah-Hartman
>> Cc: Nitin Gupta; Dan Magenheimer; Konrad Rzeszutek Wilk; Robert Jennings; Seth Jennings;
>> devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org
>> Subject: [PATCH] staging: zsmalloc: fix memory leak
>>
>> From: Nitin Gupta <ngupta@vflare.org>
>>
>> This patch fixes a memory leak in zsmalloc where the first
>> subpage of each zspage is leaked when the zspage is freed.
>>
>> Based on 3.4-rc1.
>>
>> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
>> Acked-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> 
> This is a rather severe memory leak and will affect most
> benchmarking anyone does to evaluate zcache in 3.4 (e.g. as
> to whether zcache is suitable for promotion), so t'would be nice
> to get this patch in for -rc2.  (Note it fixes a "regression"
> since it affects zcache only in 3.4+ because the fix is to
> the new zsmalloc allocator... so no change to stable trees.)
> 
> Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>

Thanks Dan for this clarification and the Ack.

I should have tagged this as urgent for the 3.4 release
and no impact on stable trees, since 3.4 is the first release
with this code.

--
Seth


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

* Re: [PATCH] staging: zsmalloc: fix memory leak
  2012-04-04 16:26   ` Seth Jennings
@ 2012-04-09 20:23     ` Seth Jennings
  2012-04-09 21:43       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Seth Jennings @ 2012-04-09 20:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dan Magenheimer, Nitin Gupta, Konrad Rzeszutek Wilk,
	Robert Jennings, devel, linux-kernel, linux-mm

Hey Greg,

Haven't heard back from you on this patch and it needs to
get into the 3.4 -rc releases ASAP.  It fixes a substantial
memory leak when frontswap/zcache are enabled.

Let me know if you need me to repost.

The patch was sent on 4/2.

Thanks,
Seth

On 04/04/2012 11:26 AM, Seth Jennings wrote:
> On 04/04/2012 11:03 AM, Dan Magenheimer wrote:
>>> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
>>> Sent: Monday, April 02, 2012 8:14 AM
>>> To: Greg Kroah-Hartman
>>> Cc: Nitin Gupta; Dan Magenheimer; Konrad Rzeszutek Wilk; Robert Jennings; Seth Jennings;
>>> devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org
>>> Subject: [PATCH] staging: zsmalloc: fix memory leak
>>>
>>> From: Nitin Gupta <ngupta@vflare.org>
>>>
>>> This patch fixes a memory leak in zsmalloc where the first
>>> subpage of each zspage is leaked when the zspage is freed.
>>>
>>> Based on 3.4-rc1.
>>>
>>> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
>>> Acked-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>
>> This is a rather severe memory leak and will affect most
>> benchmarking anyone does to evaluate zcache in 3.4 (e.g. as
>> to whether zcache is suitable for promotion), so t'would be nice
>> to get this patch in for -rc2.  (Note it fixes a "regression"
>> since it affects zcache only in 3.4+ because the fix is to
>> the new zsmalloc allocator... so no change to stable trees.)
>>
>> Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> 
> Thanks Dan for this clarification and the Ack.
> 
> I should have tagged this as urgent for the 3.4 release
> and no impact on stable trees, since 3.4 is the first release
> with this code.
> 
> --
> Seth


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

* Re: [PATCH] staging: zsmalloc: fix memory leak
  2012-04-09 20:23     ` Seth Jennings
@ 2012-04-09 21:43       ` Greg Kroah-Hartman
  2012-04-09 22:06         ` Seth Jennings
  0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2012-04-09 21:43 UTC (permalink / raw)
  To: Seth Jennings
  Cc: Dan Magenheimer, Nitin Gupta, Konrad Rzeszutek Wilk,
	Robert Jennings, devel, linux-kernel, linux-mm

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Mon, Apr 09, 2012 at 03:23:38PM -0500, Seth Jennings wrote:
> Hey Greg,
> 
> Haven't heard back from you on this patch and it needs to
> get into the 3.4 -rc releases ASAP.  It fixes a substantial
> memory leak when frontswap/zcache are enabled.
> 
> Let me know if you need me to repost.
> 
> The patch was sent on 4/2.

5 meager days ago, with a major holliday in the middle, not to mention a
conference as well.  That's bold.

It is not lost, is in my queue, and will get to Linus before 3.4-final
comes out, don't worry.

greg k-h

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

* Re: [PATCH] staging: zsmalloc: fix memory leak
  2012-04-09 21:43       ` Greg Kroah-Hartman
@ 2012-04-09 22:06         ` Seth Jennings
  0 siblings, 0 replies; 6+ messages in thread
From: Seth Jennings @ 2012-04-09 22:06 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dan Magenheimer, Nitin Gupta, Konrad Rzeszutek Wilk,
	Robert Jennings, devel, linux-kernel, linux-mm

On 04/09/2012 04:43 PM, Greg Kroah-Hartman wrote:
> A: No.
> Q: Should I include quotations after my reply?
> 
> http://daringfireball.net/2007/07/on_top

Noted. I only included what I did to keep the Ack chain in the
message, which I guess was unnecessary.

> On Mon, Apr 09, 2012 at 03:23:38PM -0500, Seth Jennings wrote:
>> Hey Greg,
>>
>> Haven't heard back from you on this patch and it needs to
>> get into the 3.4 -rc releases ASAP.  It fixes a substantial
>> memory leak when frontswap/zcache are enabled.
>>
>> Let me know if you need me to repost.
>>
>> The patch was sent on 4/2.
> 
> 5 meager days ago, with a major holliday in the middle, not to mention a
> conference as well.  That's bold.

Sorry about that.  Forgot about the summit and the holiday :-/

The date was not to convey "It's been a whole week! Why the delay?",
but to help you find the original patch in your stack of emails.

> It is not lost, is in my queue, and will get to Linus before 3.4-final
> comes out, don't worry.

I'll be more patient (and take into account summits/holiday) for replies.

Thanks Greg!

--
Seth


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

end of thread, other threads:[~2012-04-09 22:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <<1333376036-9841-1-git-send-email-sjenning@linux.vnet.ibm.com>
2012-04-04 16:03 ` [PATCH] staging: zsmalloc: fix memory leak Dan Magenheimer
2012-04-04 16:26   ` Seth Jennings
2012-04-09 20:23     ` Seth Jennings
2012-04-09 21:43       ` Greg Kroah-Hartman
2012-04-09 22:06         ` Seth Jennings
2012-04-02 14:13 Seth Jennings

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