del_page_from_lru_list alreay called mem_cgroup_del_lru.
So we need to call it again. It makes wrong stat of memcg and
even happen VM_BUG_ON hit.
Cc: Balbir Singh <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Mel Gorman <[email protected]>
Signed-off-by: Minchan Kim <[email protected]>
---
mm/compaction.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 50b0a90..b0fbfdf 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -302,7 +302,6 @@ static unsigned long isolate_migratepages(struct zone *zone,
/* Successfully isolated */
del_page_from_lru_list(zone, page, page_lru(page));
list_add(&page->lru, migratelist);
- mem_cgroup_del_lru(page);
cc->nr_migratepages++;
nr_isolated++;
--
1.7.0.4
On Wed, 8 Dec 2010 00:01:26 +0900
Minchan Kim <[email protected]> wrote:
> del_page_from_lru_list alreay called mem_cgroup_del_lru.
> So we need to call it again. It makes wrong stat of memcg and
> even happen VM_BUG_ON hit.
>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
> ---
> mm/compaction.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 50b0a90..b0fbfdf 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -302,7 +302,6 @@ static unsigned long isolate_migratepages(struct zone *zone,
> /* Successfully isolated */
> del_page_from_lru_list(zone, page, page_lru(page));
> list_add(&page->lru, migratelist);
> - mem_cgroup_del_lru(page);
> cc->nr_migratepages++;
> nr_isolated++;
>
err, yes, that looks bad.
This bug is present in 2.6.35 and 2.6.36 afaict, so I tagged the fix
for -stable backporting.
On Wed, Dec 8, 2010 at 6:53 AM, Andrew Morton <[email protected]> wrote:
> On Wed, ?8 Dec 2010 00:01:26 +0900
> Minchan Kim <[email protected]> wrote:
>
>> del_page_from_lru_list alreay called mem_cgroup_del_lru.
>> So we need to call it again. It makes wrong stat of memcg and
>> even happen VM_BUG_ON hit.
>>
>> Cc: Balbir Singh <[email protected]>
>> Cc: KAMEZAWA Hiroyuki <[email protected]>
>> Cc: Mel Gorman <[email protected]>
>> Signed-off-by: Minchan Kim <[email protected]>
>> ---
>> ?mm/compaction.c | ? ?1 -
>> ?1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/compaction.c b/mm/compaction.c
>> index 50b0a90..b0fbfdf 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -302,7 +302,6 @@ static unsigned long isolate_migratepages(struct zone *zone,
>> ? ? ? ? ? ? ? /* Successfully isolated */
>> ? ? ? ? ? ? ? del_page_from_lru_list(zone, page, page_lru(page));
>> ? ? ? ? ? ? ? list_add(&page->lru, migratelist);
>> - ? ? ? ? ? ? mem_cgroup_del_lru(page);
>> ? ? ? ? ? ? ? cc->nr_migratepages++;
>> ? ? ? ? ? ? ? nr_isolated++;
>>
>
> err, yes, that looks bad.
>
> This bug is present in 2.6.35 and 2.6.36 afaict, so I tagged the fix
> for -stable backporting.
Thanks, Andrew.
>
>
--
Kind regards,
Minchan Kim
On Wed, 8 Dec 2010 00:01:26 +0900
Minchan Kim <[email protected]> wrote:
> del_page_from_lru_list alreay called mem_cgroup_del_lru.
> So we need to call it again. It makes wrong stat of memcg and
> even happen VM_BUG_ON hit.
>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
Acked-by: KAMEZAWA Hiroyuki <[email protected]>
Thank you.
* MinChan Kim <[email protected]> [2010-12-08 00:01:26]:
> del_page_from_lru_list alreay called mem_cgroup_del_lru.
> So we need to call it again. It makes wrong stat of memcg and
> even happen VM_BUG_ON hit.
>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
> ---
Acked-by: Balbir Singh <[email protected]>
--
Three Cheers,
Balbir
On Wed, Dec 08, 2010 at 12:01:26AM +0900, Minchan Kim wrote:
> del_page_from_lru_list alreay called mem_cgroup_del_lru.
> So we need to call it again. It makes wrong stat of memcg and
> even happen VM_BUG_ON hit.
>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
Thanks
Acked-by: Mel Gorman <[email protected]>
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
On Wed, Dec 08, 2010 at 12:01:26AM +0900, Minchan Kim wrote:
> del_page_from_lru_list alreay called mem_cgroup_del_lru.
> So we need to call it again. It makes wrong stat of memcg and
> even happen VM_BUG_ON hit.
>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
Reviewed-by: Johannes Weiner <[email protected]>
But regarding the severity of this: shouldn't the second deletion
attempt be caught by the TestClearPageCgroupAcctLRU() early in
mem_cgroup_del_lru_list()?
On Thu, Dec 9, 2010 at 1:43 AM, Johannes Weiner <[email protected]> wrote:
> On Wed, Dec 08, 2010 at 12:01:26AM +0900, Minchan Kim wrote:
>> del_page_from_lru_list alreay called mem_cgroup_del_lru.
>> So we need to call it again. It makes wrong stat of memcg and
>> even happen VM_BUG_ON hit.
>>
>> Cc: Balbir Singh <[email protected]>
>> Cc: KAMEZAWA Hiroyuki <[email protected]>
>> Cc: Mel Gorman <[email protected]>
>> Signed-off-by: Minchan Kim <[email protected]>
>
> Reviewed-by: Johannes Weiner <[email protected]>
>
> But regarding the severity of this: shouldn't the second deletion
> attempt be caught by the TestClearPageCgroupAcctLRU() early in
> mem_cgroup_del_lru_list()?
>
Right, I missed that.
Andrew, I will resend modified description.
Thanks for careful review again, Hannes.
--
Kind regards,
Minchan Kim
On Wed, Dec 08, 2010 at 12:01:26AM +0900, Minchan Kim wrote:
> del_page_from_lru_list alreay called mem_cgroup_del_lru.
> So we need to call it again. It makes wrong stat of memcg and
> even happen VM_BUG_ON hit.
>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
> ---
> mm/compaction.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 50b0a90..b0fbfdf 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -302,7 +302,6 @@ static unsigned long isolate_migratepages(struct zone *zone,
> /* Successfully isolated */
> del_page_from_lru_list(zone, page, page_lru(page));
> list_add(&page->lru, migratelist);
> - mem_cgroup_del_lru(page);
> cc->nr_migratepages++;
> nr_isolated++;
>
> --
> 1.7.0.4
>
Hi Andrew,
Please drop above(mm-compactionc-avoid-double-mem_cgroup_del_lru.patch)
This is a new version with modified description and added Acked-by.
>From 64b3a2c1205d3ffc55878ab8f00f4d450318bb91 Mon Sep 17 00:00:00 2001
From: Minchan Kim <[email protected]>
Date: Thu, 9 Dec 2010 23:49:27 +0900
Subject: [PATCH] mm/compaction.c: avoid double mem_cgroup_del_lru()
del_page_from_lru_list() already called mem_cgroup_del_lru(). So we must
not call it again. It adds unnecessary overhead.
Acked-by: KAMEZAWA Hiroyuki <[email protected]>
Acked-by: Balbir Singh <[email protected]>
Acked-by: Mel Gorman <[email protected]>
Reviewed-by: Johannes Weiner <[email protected]>
Signed-off-by: Minchan Kim <[email protected]>
---
mm/compaction.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 50b0a90..b0fbfdf 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -302,7 +302,6 @@ static unsigned long isolate_migratepages(struct zone *zone,
/* Successfully isolated */
del_page_from_lru_list(zone, page, page_lru(page));
list_add(&page->lru, migratelist);
- mem_cgroup_del_lru(page);
cc->nr_migratepages++;
nr_isolated++;
--
1.7.0.4
--
Kind regards,
Minchan Kim