Skip to contentSkip to conversation

Strange oopses in 2.6.30

DISCUSSION Maxim Levitsky 8 participants 22 messages Jun 19, 2009 – Jun 30, 2009

Subsystems (3)
Memory management MM core Page allocator
lore
Share & tools
lore

Maxim Levitsky

author
#0 lore raw

I see lots of following oopses in 2.6.30 and latest -git

Many different applications shows up, not just reiserfsck Something in MM I guess, it makes me worry. But system seems to work.

Is this known?

dmesg attached.

[   34.544040] BUG: Bad page state in process reiserfsck  pfn:37d86
[   34.544044] page:c2a34f38 flags:3650000c count:0 mapcount:0
mapping:(null) index:bffeb
[   34.544048] Pid: 2654, comm: reiserfsck Tainted: G    B
2.6.30-git #4
[   34.544051] Call Trace:
[   34.544055]  [<c04cd26a>] ? printk+0x18/0x1e
[   34.544059]  [<c018f065>] bad_page+0xd5/0x140
[   34.544064]  [<c0190097>] free_hot_cold_page+0x1e7/0x280
[   34.544069]  [<c0193682>] ? release_pages+0x92/0x1b0
[   34.544074]  [<c0190155>] __pagevec_free+0x25/0x30
[   34.544078]  [<c0193758>] release_pages+0x168/0x1b0
[   34.544084]  [<c0193cf3>] ? lru_add_drain+0x53/0xd0
[   34.544088]  [<c01ab7d4>] free_pages_and_swap_cache+0x84/0xa0
[   34.544093]  [<c019ff5d>] unmap_vmas+0x73d/0x760
[   34.544099]  [<c016480e>] ? lock_release_non_nested+0x15e/0x270
[   34.544104]  [<c01a43b5>] exit_mmap+0xb5/0x1b0
[   34.544109]  [<c0138666>] mmput+0x36/0xc0
[   34.544113]  [<c013c874>] exit_mm+0xe4/0x120
[   34.544117]  [<c0175539>] ? acct_collect+0x139/0x180
[   34.544122]  [<c013e889>] do_exit+0x6b9/0x720
[   34.544142]  [<c01bcac2>] ? vfs_write+0x122/0x180
[   34.544146]  [<c01bbda0>] ? do_sync_write+0x0/0x110
[   34.544151]  [<c013e920>] do_group_exit+0x30/0x90
[   34.544156]  [<c013e993>] sys_exit_group+0x13/0x20
[   34.544161]  [<c01039e8>] sysenter_do_call+0x12/0x3c
[   34.544180] BUG: Bad page state in process reiserfsck  pfn:37d91
[   34.544184] page:c2a35174 flags:3650000c count:0 mapcount:0
mapping:(null) index:bfff6
[   34.544188] Pid: 2654, comm: reiserfsck Tainted: G    B

2.6.30-git #4

author
#1 lore raw

BUG] Strange oopses in 2.6.30

Quoted 40 lines · Maxim Levitsky

I see lots of following oopses in 2.6.30 and latest -git

Many different applications shows up, not just reiserfsck Something in MM I guess, it makes me worry. But system seems to work.

Is this known?

dmesg attached.

[   34.544040] BUG: Bad page state in process reiserfsck  pfn:37d86
[   34.544044] page:c2a34f38 flags:3650000c count:0 mapcount:0
mapping:(null) index:bffeb
[   34.544048] Pid: 2654, comm: reiserfsck Tainted: G    B
2.6.30-git #4
[   34.544051] Call Trace:
[   34.544055]  [<c04cd26a>] ? printk+0x18/0x1e
[   34.544059]  [<c018f065>] bad_page+0xd5/0x140
[   34.544064]  [<c0190097>] free_hot_cold_page+0x1e7/0x280
[   34.544069]  [<c0193682>] ? release_pages+0x92/0x1b0
[   34.544074]  [<c0190155>] __pagevec_free+0x25/0x30
[   34.544078]  [<c0193758>] release_pages+0x168/0x1b0
[   34.544084]  [<c0193cf3>] ? lru_add_drain+0x53/0xd0
[   34.544088]  [<c01ab7d4>] free_pages_and_swap_cache+0x84/0xa0
[   34.544093]  [<c019ff5d>] unmap_vmas+0x73d/0x760
[   34.544099]  [<c016480e>] ? lock_release_non_nested+0x15e/0x270
[   34.544104]  [<c01a43b5>] exit_mmap+0xb5/0x1b0
[   34.544109]  [<c0138666>] mmput+0x36/0xc0
[   34.544113]  [<c013c874>] exit_mm+0xe4/0x120
[   34.544117]  [<c0175539>] ? acct_collect+0x139/0x180
[   34.544122]  [<c013e889>] do_exit+0x6b9/0x720
[   34.544142]  [<c01bcac2>] ? vfs_write+0x122/0x180
[   34.544146]  [<c01bbda0>] ? do_sync_write+0x0/0x110
[   34.544151]  [<c013e920>] do_group_exit+0x30/0x90
[   34.544156]  [<c013e993>] sys_exit_group+0x13/0x20
[   34.544161]  [<c01039e8>] sysenter_do_call+0x12/0x3c
[   34.544180] BUG: Bad page state in process reiserfsck  pfn:37d91
[   34.544184] page:c2a35174 flags:3650000c count:0 mapcount:0
mapping:(null) index:bfff6
[   34.544188] Pid: 2654, comm: reiserfsck Tainted: G    B
Quoted 2 lines

2.6.30-git #4

This really worries me

#2 lore raw

BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 34 lines · Maxim Levitsky
Quoted 33 lines · Maxim Levitsky

I see lots of following oopses in 2.6.30 and latest -git

Many different applications shows up, not just reiserfsck Something in MM I guess, it makes me worry. But system seems to work.

Is this known?

dmesg attached.

[   34.544040] BUG: Bad page state in process reiserfsck  pfn:37d86
[   34.544044] page:c2a34f38 flags:3650000c count:0 mapcount:0
mapping:(null) index:bffeb
[   34.544048] Pid: 2654, comm: reiserfsck Tainted: G    B
2.6.30-git #4
[   34.544051] Call Trace:
[   34.544055]  [<c04cd26a>] ? printk+0x18/0x1e
[   34.544059]  [<c018f065>] bad_page+0xd5/0x140
[   34.544064]  [<c0190097>] free_hot_cold_page+0x1e7/0x280
[   34.544069]  [<c0193682>] ? release_pages+0x92/0x1b0
[   34.544074]  [<c0190155>] __pagevec_free+0x25/0x30
[   34.544078]  [<c0193758>] release_pages+0x168/0x1b0
[   34.544084]  [<c0193cf3>] ? lru_add_drain+0x53/0xd0
[   34.544088]  [<c01ab7d4>] free_pages_and_swap_cache+0x84/0xa0
[   34.544093]  [<c019ff5d>] unmap_vmas+0x73d/0x760
[   34.544099]  [<c016480e>] ? lock_release_non_nested+0x15e/0x270
[   34.544104]  [<c01a43b5>] exit_mmap+0xb5/0x1b0
[   34.544109]  [<c0138666>] mmput+0x36/0xc0
[   34.544113]  [<c013c874>] exit_mm+0xe4/0x120
[   34.544117]  [<c0175539>] ? acct_collect+0x139/0x180
[   34.544122]  [<c013e889>] do_exit+0x6b9/0x720
[   34.544142]  [<c01bcac2>] ? vfs_write+0x122/0x180
[   34.544146]  [<c01bbda0>] ? do_sync_write+0x0/0x110
Quoted 8 lines
Quoted 8 lines
[   34.544151]  [<c013e920>] do_group_exit+0x30/0x90
[   34.544156]  [<c013e993>] sys_exit_group+0x13/0x20
[   34.544161]  [<c01039e8>] sysenter_do_call+0x12/0x3c
[   34.544180] BUG: Bad page state in process reiserfsck  pfn:37d91
[   34.544184] page:c2a35174 flags:3650000c count:0 mapcount:0
mapping:(null) index:bfff6
[   34.544188] Pid: 2654, comm: reiserfsck Tainted: G    B

2.6.30-git #4

I got similar on 64-bit mmotm 2009-06-12-12-20. You seem not to use 2.6.30, but some git post 2.6.30.

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

In my .config, there is
CONFIG_PAGEFLAGS_EXTENDED=y
CONFIG_HAVE_MLOCKED_PAGE_BIT=y

The traces:

BUG: Bad page state in process gpg-agent  pfn:1c83c9
page:ffffea00063cd3f8 flags:800000000050000c count:0 mapcount:0
mapping:(null) i
ndex:7feda9eae
Pid: 3859, comm: gpg-agent Not tainted 2.6.30-mm1_64 #641
Call Trace:
 [<ffffffff8108fab2>] bad_page+0xd2/0x130
 [<ffffffff81091a57>] free_hot_cold_page+0x47/0x200
 [<ffffffff81091c48>] __pagevec_free+0x38/0x50
 [<ffffffff81094eac>] release_pages+0x20c/0x240
 [<ffffffff810afc6f>] free_pages_and_swap_cache+0xaf/0xd0
 [<ffffffff8115d53a>] ? cpumask_any_but+0x2a/0x40
 [<ffffffff810a8520>] unmap_region+0x150/0x170
 [<ffffffff810a87b4>] do_munmap+0x274/0x370
 [<ffffffff810a88fc>] sys_munmap+0x4c/0x70
 [<ffffffff8100beab>] system_call_fastpath+0x16/0x1b
Disabling lock debugging due to kernel taint
BUG: Bad page state in process gpg-agent  pfn:1c83c8
page:ffffea00063cd3c0 flags:800000000050000c count:0 mapcount:0
mapping:(null) i
ndex:7feda9ead
Pid: 3859, comm: gpg-agent Tainted: G    B      2.6.30-mm1_64 #641
Call Trace:
 [<ffffffff8108fab2>] bad_page+0xd2/0x130
 [<ffffffff81091a57>] free_hot_cold_page+0x47/0x200
 [<ffffffff81091c48>] __pagevec_free+0x38/0x50
 [<ffffffff81094eac>] release_pages+0x20c/0x240
 [<ffffffff810afc6f>] free_pages_and_swap_cache+0xaf/0xd0
 [<ffffffff8115d53a>] ? cpumask_any_but+0x2a/0x40
 [<ffffffff810a8520>] unmap_region+0x150/0x170
 [<ffffffff810a87b4>] do_munmap+0x274/0x370
 [<ffffffff810a88fc>] sys_munmap+0x4c/0x70
 [<ffffffff8100beab>] system_call_fastpath+0x16/0x1b
BUG: Bad page state in process gpg-agent  pfn:1c800f
page:ffffea00063c0348 flags:800000000050000c count:0 mapcount:0
mapping:(null) i
ndex:7feda9eac
Pid: 3859, comm: gpg-agent Tainted: G    B      2.6.30-mm1_64 #641
Call Trace:
 [<ffffffff8108fab2>] bad_page+0xd2/0x130
 [<ffffffff81091a57>] free_hot_cold_page+0x47/0x200
 [<ffffffff81091c48>] __pagevec_free+0x38/0x50
 [<ffffffff81094eac>] release_pages+0x20c/0x240
 [<ffffffff810afc6f>] free_pages_and_swap_cache+0xaf/0xd0
 [<ffffffff8115d53a>] ? cpumask_any_but+0x2a/0x40
 [<ffffffff810a8520>] unmap_region+0x150/0x170
 [<ffffffff810a87b4>] do_munmap+0x274/0x370
 [<ffffffff810a88fc>] sys_munmap+0x4c/0x70
 [<ffffffff8100beab>] system_call_fastpath+0x16/0x1b

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#19 lore raw

Re: BUG] Strange oopses in 2.6.30

In reply to Maxim Levitsky
Quoted 7 lines · Maxim Levitsky
Quoted 6 lines · Maxim Levitsky

I see lots of following oopses in 2.6.30 and latest -git

Many different applications shows up, not just reiserfsck Something in MM I guess, it makes me worry. But system seems to work.

Is this known?

I think so...

Quoted 37 lines
Quoted 34 lines

dmesg attached.

[   34.544040] BUG: Bad page state in process reiserfsck  pfn:37d86
[   34.544044] page:c2a34f38 flags:3650000c count:0 mapcount:0
mapping:(null) index:bffeb
[   34.544048] Pid: 2654, comm: reiserfsck Tainted: G    B
2.6.30-git #4
[   34.544051] Call Trace:
[   34.544055]  [<c04cd26a>] ? printk+0x18/0x1e
[   34.544059]  [<c018f065>] bad_page+0xd5/0x140
[   34.544064]  [<c0190097>] free_hot_cold_page+0x1e7/0x280
[   34.544069]  [<c0193682>] ? release_pages+0x92/0x1b0
[   34.544074]  [<c0190155>] __pagevec_free+0x25/0x30
[   34.544078]  [<c0193758>] release_pages+0x168/0x1b0
[   34.544084]  [<c0193cf3>] ? lru_add_drain+0x53/0xd0
[   34.544088]  [<c01ab7d4>] free_pages_and_swap_cache+0x84/0xa0
[   34.544093]  [<c019ff5d>] unmap_vmas+0x73d/0x760
[   34.544099]  [<c016480e>] ? lock_release_non_nested+0x15e/0x270
[   34.544104]  [<c01a43b5>] exit_mmap+0xb5/0x1b0
[   34.544109]  [<c0138666>] mmput+0x36/0xc0
[   34.544113]  [<c013c874>] exit_mm+0xe4/0x120
[   34.544117]  [<c0175539>] ? acct_collect+0x139/0x180
[   34.544122]  [<c013e889>] do_exit+0x6b9/0x720
[   34.544142]  [<c01bcac2>] ? vfs_write+0x122/0x180
[   34.544146]  [<c01bbda0>] ? do_sync_write+0x0/0x110
[   34.544151]  [<c013e920>] do_group_exit+0x30/0x90
[   34.544156]  [<c013e993>] sys_exit_group+0x13/0x20
[   34.544161]  [<c01039e8>] sysenter_do_call+0x12/0x3c
[   34.544180] BUG: Bad page state in process reiserfsck  pfn:37d91
[   34.544184] page:c2a35174 flags:3650000c count:0 mapcount:0
mapping:(null) index:bfff6
[   34.544188] Pid: 2654, comm: reiserfsck Tainted: G    B
Quoted 2 lines

2.6.30-git #4

Quoted 1 line

This really worries me

I hope it's fixed by this patch Hannes posted yesterday...

Quoted 1 line

From [email protected] Fri Jun 19 19:04:49 2009

Date: Fri, 19 Jun 2009 19:45:02 +0200
From: Johannes Weiner <[email protected]>
To: Peter Chubb <[email protected]>
Cc: <[email protected]>, <[email protected]>, <[email protected]>
Subject: Re: [BUG] Bad page flags when process using mlock()ed memory exits

Quoted 22 lines · Peter Chubb

In recent kernels I've been seeing many mesages of the form:

BUG: Bad page state in process reiserfsck  pfn:79c58
page:c3d03b00 flags:8050000c count:0 mapcount:0 mapping:(null) index:8095
Pid: 3927, comm: reiserfsck Not tainted 2.6.30-test-05456-gda456f1 #60
Call Trace:
 [<c134a67c>] ? printk+0xf/0x13
 [<c10774dc>] bad_page+0xc9/0xe2
 [<c1078041>] free_hot_cold_page+0x5c/0x204
 [<c1078206>] __pagevec_free+0x1d/0x25
 [<c107ac3e>] release_pages+0x14e/0x18e)
 [<c108ef8a>] free_pages_and_swap_cache+0x69/0x82
 [<c1089458>] exit_mmap+0xf6/0x11f
 [<c102afcd>] mmput+0x39/0xaf
 [<c102e534>] exit_mm+0xe5/0xed
 [<c102fa66>] do_exit+0x13f/0x578
 [<c102fefd>] do_group_exit+0x5e/0x85
 [<c102ff37>] sys_exit_group+0x13/0x17
 [<c10031ef>] sysenter_do_call+0x12/0x3c

Disabling lock debugging due to kernel taint

Quoted 8 lines

This appears to have been introduced by patch
da456f14d2f2d7350f2b9440af79c85a34c7eed5
page allocator: do not disable interrupts in free_page_mlock()

That patch removed the free_page_mlock() from free_pages_check(), so if free_hot_cold_page() is called on an Mlocked page (e.g., if a process that used mlock() calls exit()) free_pages_check() will always barf, whereas before it would just unlock the page.

I prepared a fix, thanks for chasing it down.

Mel, to keep this simple I just used the atomic test-clear, but if I am not mistaken we should not need any atomicity here, so we could probably add a __TestClearPage version and use this instead...?

---

Quoted 1 line

From 493b74e8615db4e3323b5b169b0b8265dfd7c3f4 Mon Sep 17 00:00:00 2001

From: Johannes Weiner <[email protected]>
Date: Fri, 19 Jun 2009 19:30:56 +0200
Subject: [patch] mm: page_alloc: clear PG_locked before checking flags on free

da456f1 "page allocator: do not disable interrupts in free_page_mlock()" moved the PG_mlocked clearing after the flag sanity checking which makes mlocked pages always trigger 'bad page'. Fix this by clearing the bit up front.

Reported-by Peter Chubb <[email protected]>
Debugged-by Peter Chubb <[email protected]>
Signed-off-by Johannes Weiner <[email protected]>
Cc Mel Gorman <[email protected]>
---
 mm/page_alloc.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)
mm/page_alloc.c +4 -5
index 6f0753f..30d5093 100644
@@ -488,7 +488,6 @@ static inline void __free_one_page(struct page *page,
  */
 static inline void free_page_mlock(struct page *page)
 {
-	__ClearPageMlocked(page);
 	__dec_zone_page_state(page, NR_MLOCK);
 	__count_vm_event(UNEVICTABLE_MLOCKFREED);
 }

@@ -558,7 +557,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	unsigned long flags;
 	int i;
 	int bad = 0;
-	int clearMlocked = PageMlocked(page);
+	int wasMlocked = TestClearPageMlocked(page);
 
 	kmemcheck_free_shadow(page, order);
 

@@ -576,7 +575,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	kernel_map_pages(page, 1 << order, 0);
 
 	local_irq_save(flags);
-	if (unlikely(clearMlocked))
+	if (unlikely(wasMlocked))
 		free_page_mlock(page);
 	__count_vm_events(PGFREE, 1 << order);
 	free_one_page(page_zone(page), page, order,

@@ -1022,7 +1021,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 	struct zone *zone = page_zone(page);
 	struct per_cpu_pages *pcp;
 	unsigned long flags;
-	int clearMlocked = PageMlocked(page);
+	int wasMlocked = TestClearPageMlocked(page);
 
 	kmemcheck_free_shadow(page, 0);
 

@@ -1041,7 +1040,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 	pcp = &zone_pcp(zone, get_cpu())->pcp;
 	set_page_private(page, get_pageblock_migratetype(page));
 	local_irq_save(flags);
-	if (unlikely(clearMlocked))
+	if (unlikely(wasMlocked))
 		free_page_mlock(page);
 	__count_vm_event(PGFREE);
Signature · 1 line
1.6.3
author
#3 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to Jiri Slaby
Quoted 44 lines · Jiri Slaby
Quoted 34 lines · Maxim Levitsky
Quoted 33 lines · Maxim Levitsky

I see lots of following oopses in 2.6.30 and latest -git

Many different applications shows up, not just reiserfsck Something in MM I guess, it makes me worry. But system seems to work.

Is this known?

dmesg attached.

[   34.544040] BUG: Bad page state in process reiserfsck  pfn:37d86
[   34.544044] page:c2a34f38 flags:3650000c count:0 mapcount:0
mapping:(null) index:bffeb
[   34.544048] Pid: 2654, comm: reiserfsck Tainted: G    B
2.6.30-git #4
[   34.544051] Call Trace:
[   34.544055]  [<c04cd26a>] ? printk+0x18/0x1e
[   34.544059]  [<c018f065>] bad_page+0xd5/0x140
[   34.544064]  [<c0190097>] free_hot_cold_page+0x1e7/0x280
[   34.544069]  [<c0193682>] ? release_pages+0x92/0x1b0
[   34.544074]  [<c0190155>] __pagevec_free+0x25/0x30
[   34.544078]  [<c0193758>] release_pages+0x168/0x1b0
[   34.544084]  [<c0193cf3>] ? lru_add_drain+0x53/0xd0
[   34.544088]  [<c01ab7d4>] free_pages_and_swap_cache+0x84/0xa0
[   34.544093]  [<c019ff5d>] unmap_vmas+0x73d/0x760
[   34.544099]  [<c016480e>] ? lock_release_non_nested+0x15e/0x270
[   34.544104]  [<c01a43b5>] exit_mmap+0xb5/0x1b0
[   34.544109]  [<c0138666>] mmput+0x36/0xc0
[   34.544113]  [<c013c874>] exit_mm+0xe4/0x120
[   34.544117]  [<c0175539>] ? acct_collect+0x139/0x180
[   34.544122]  [<c013e889>] do_exit+0x6b9/0x720
[   34.544142]  [<c01bcac2>] ? vfs_write+0x122/0x180
[   34.544146]  [<c01bbda0>] ? do_sync_write+0x0/0x110
Quoted 8 lines
Quoted 8 lines
[   34.544151]  [<c013e920>] do_group_exit+0x30/0x90
[   34.544156]  [<c013e993>] sys_exit_group+0x13/0x20
[   34.544161]  [<c01039e8>] sysenter_do_call+0x12/0x3c
[   34.544180] BUG: Bad page state in process reiserfsck  pfn:37d91
[   34.544184] page:c2a35174 flags:3650000c count:0 mapcount:0
mapping:(null) index:bfff6
[   34.544188] Pid: 2654, comm: reiserfsck Tainted: G    B

2.6.30-git #4

Quoted 2 lines

I got similar on 64-bit mmotm 2009-06-12-12-20. You seem not to use 2.6.30, but some git post 2.6.30.

Yes, yesterday git
I tried 2.6.30 too, but it gives same warnings.

Quoted 53 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

In my .config, there is
CONFIG_PAGEFLAGS_EXTENDED=y
CONFIG_HAVE_MLOCKED_PAGE_BIT=y

The traces:

BUG: Bad page state in process gpg-agent  pfn:1c83c9
page:ffffea00063cd3f8 flags:800000000050000c count:0 mapcount:0
mapping:(null) i
ndex:7feda9eae
Pid: 3859, comm: gpg-agent Not tainted 2.6.30-mm1_64 #641
Call Trace:
 [<ffffffff8108fab2>] bad_page+0xd2/0x130
 [<ffffffff81091a57>] free_hot_cold_page+0x47/0x200
 [<ffffffff81091c48>] __pagevec_free+0x38/0x50
 [<ffffffff81094eac>] release_pages+0x20c/0x240
 [<ffffffff810afc6f>] free_pages_and_swap_cache+0xaf/0xd0
 [<ffffffff8115d53a>] ? cpumask_any_but+0x2a/0x40
 [<ffffffff810a8520>] unmap_region+0x150/0x170
 [<ffffffff810a87b4>] do_munmap+0x274/0x370
 [<ffffffff810a88fc>] sys_munmap+0x4c/0x70
 [<ffffffff8100beab>] system_call_fastpath+0x16/0x1b
Disabling lock debugging due to kernel taint
BUG: Bad page state in process gpg-agent  pfn:1c83c8
page:ffffea00063cd3c0 flags:800000000050000c count:0 mapcount:0
mapping:(null) i
ndex:7feda9ead
Pid: 3859, comm: gpg-agent Tainted: G    B      2.6.30-mm1_64 #641
Call Trace:
 [<ffffffff8108fab2>] bad_page+0xd2/0x130
 [<ffffffff81091a57>] free_hot_cold_page+0x47/0x200
 [<ffffffff81091c48>] __pagevec_free+0x38/0x50
 [<ffffffff81094eac>] release_pages+0x20c/0x240
 [<ffffffff810afc6f>] free_pages_and_swap_cache+0xaf/0xd0
 [<ffffffff8115d53a>] ? cpumask_any_but+0x2a/0x40
 [<ffffffff810a8520>] unmap_region+0x150/0x170
 [<ffffffff810a87b4>] do_munmap+0x274/0x370
 [<ffffffff810a88fc>] sys_munmap+0x4c/0x70
 [<ffffffff8100beab>] system_call_fastpath+0x16/0x1b
BUG: Bad page state in process gpg-agent  pfn:1c800f
page:ffffea00063c0348 flags:800000000050000c count:0 mapcount:0
mapping:(null) i
ndex:7feda9eac
Pid: 3859, comm: gpg-agent Tainted: G    B      2.6.30-mm1_64 #641
Call Trace:
 [<ffffffff8108fab2>] bad_page+0xd2/0x130
 [<ffffffff81091a57>] free_hot_cold_page+0x47/0x200
Quoted 8 lines
 [<ffffffff81091c48>] __pagevec_free+0x38/0x50
 [<ffffffff81094eac>] release_pages+0x20c/0x240
 [<ffffffff810afc6f>] free_pages_and_swap_cache+0xaf/0xd0
 [<ffffffff8115d53a>] ? cpumask_any_but+0x2a/0x40
 [<ffffffff810a8520>] unmap_region+0x150/0x170
 [<ffffffff810a87b4>] do_munmap+0x274/0x370
 [<ffffffff810a88fc>] sys_munmap+0x4c/0x70
 [<ffffffff8100beab>] system_call_fastpath+0x16/0x1b

Regards,
Maxim Levitsky

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#20 lore raw

Re: BUG] Strange oopses in 2.6.30

In reply to Hugh Dickins
Quoted 11 lines · Hugh Dickins
Quoted 7 lines · Maxim Levitsky
Quoted 6 lines · Maxim Levitsky

I see lots of following oopses in 2.6.30 and latest -git

Many different applications shows up, not just reiserfsck Something in MM I guess, it makes me worry. But system seems to work.

Is this known?

I think so...

Thanks Hugh.

Quoted 37 lines
Quoted 37 lines
Quoted 36 lines

dmesg attached.

[   34.544040] BUG: Bad page state in process reiserfsck  pfn:37d86
[   34.544044] page:c2a34f38 flags:3650000c count:0 mapcount:0
mapping:(null) index:bffeb
[   34.544048] Pid: 2654, comm: reiserfsck Tainted: G    B
2.6.30-git #4
[   34.544051] Call Trace:
[   34.544055]  [<c04cd26a>] ? printk+0x18/0x1e
[   34.544059]  [<c018f065>] bad_page+0xd5/0x140
[   34.544064]  [<c0190097>] free_hot_cold_page+0x1e7/0x280
[   34.544069]  [<c0193682>] ? release_pages+0x92/0x1b0
[   34.544074]  [<c0190155>] __pagevec_free+0x25/0x30
[   34.544078]  [<c0193758>] release_pages+0x168/0x1b0
[   34.544084]  [<c0193cf3>] ? lru_add_drain+0x53/0xd0
[   34.544088]  [<c01ab7d4>] free_pages_and_swap_cache+0x84/0xa0
[   34.544093]  [<c019ff5d>] unmap_vmas+0x73d/0x760
[   34.544099]  [<c016480e>] ? lock_release_non_nested+0x15e/0x270
[   34.544104]  [<c01a43b5>] exit_mmap+0xb5/0x1b0
[   34.544109]  [<c0138666>] mmput+0x36/0xc0
[   34.544113]  [<c013c874>] exit_mm+0xe4/0x120
[   34.544117]  [<c0175539>] ? acct_collect+0x139/0x180
[   34.544122]  [<c013e889>] do_exit+0x6b9/0x720
[   34.544142]  [<c01bcac2>] ? vfs_write+0x122/0x180
[   34.544146]  [<c01bbda0>] ? do_sync_write+0x0/0x110
[   34.544151]  [<c013e920>] do_group_exit+0x30/0x90
[   34.544156]  [<c013e993>] sys_exit_group+0x13/0x20
[   34.544161]  [<c01039e8>] sysenter_do_call+0x12/0x3c
[   34.544180] BUG: Bad page state in process reiserfsck  pfn:37d91
[   34.544184] page:c2a35174 flags:3650000c count:0 mapcount:0
mapping:(null) index:bfff6
[   34.544188] Pid: 2654, comm: reiserfsck Tainted: G    B

2.6.30-git #4

Quoted 4 lines
Quoted 1 line

This really worries me

I hope it's fixed by this patch Hannes posted yesterday...

Does the patch fix the problem up?

Quoted 114 lines

From [email protected] Fri Jun 19 19:04:49 2009
Date: Fri, 19 Jun 2009 19:45:02 +0200
From: Johannes Weiner <[email protected]>
To: Peter Chubb <[email protected]>
Cc: <[email protected]>, <[email protected]>, <[email protected]>
Subject: Re: [BUG] Bad page flags when process using mlock()ed memory exits

Quoted 22 lines · Peter Chubb

In recent kernels I've been seeing many mesages of the form:

BUG: Bad page state in process reiserfsck  pfn:79c58
page:c3d03b00 flags:8050000c count:0 mapcount:0 mapping:(null) index:8095
Pid: 3927, comm: reiserfsck Not tainted 2.6.30-test-05456-gda456f1 #60
Call Trace:
 [<c134a67c>] ? printk+0xf/0x13
 [<c10774dc>] bad_page+0xc9/0xe2
 [<c1078041>] free_hot_cold_page+0x5c/0x204
 [<c1078206>] __pagevec_free+0x1d/0x25
 [<c107ac3e>] release_pages+0x14e/0x18e)
 [<c108ef8a>] free_pages_and_swap_cache+0x69/0x82
 [<c1089458>] exit_mmap+0xf6/0x11f
 [<c102afcd>] mmput+0x39/0xaf
 [<c102e534>] exit_mm+0xe5/0xed
 [<c102fa66>] do_exit+0x13f/0x578
 [<c102fefd>] do_group_exit+0x5e/0x85
 [<c102ff37>] sys_exit_group+0x13/0x17
 [<c10031ef>] sysenter_do_call+0x12/0x3c

Disabling lock debugging due to kernel taint

Quoted 8 lines

This appears to have been introduced by patch
da456f14d2f2d7350f2b9440af79c85a34c7eed5
page allocator: do not disable interrupts in free_page_mlock()

That patch removed the free_page_mlock() from free_pages_check(), so if free_hot_cold_page() is called on an Mlocked page (e.g., if a process that used mlock() calls exit()) free_pages_check() will always barf, whereas before it would just unlock the page.

I prepared a fix, thanks for chasing it down.

Mel, to keep this simple I just used the atomic test-clear, but if I am not mistaken we should not need any atomicity here, so we could probably add a __TestClearPage version and use this instead...?

---

Quoted 1 line

From 493b74e8615db4e3323b5b169b0b8265dfd7c3f4 Mon Sep 17 00:00:00 2001

From: Johannes Weiner <[email protected]>
Date: Fri, 19 Jun 2009 19:30:56 +0200
Subject: [patch] mm: page_alloc: clear PG_locked before checking flags on free

da456f1 "page allocator: do not disable interrupts in free_page_mlock()" moved the PG_mlocked clearing after the flag sanity checking which makes mlocked pages always trigger 'bad page'. Fix this by clearing the bit up front.

Reported-by Peter Chubb <[email protected]>
Debugged-by Peter Chubb <[email protected]>
Signed-off-by Johannes Weiner <[email protected]>
Cc Mel Gorman <[email protected]>
---
 mm/page_alloc.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)
mm/page_alloc.c +4 -5
index 6f0753f..30d5093 100644
@@ -488,7 +488,6 @@ static inline void __free_one_page(struct page *page,
  */
 static inline void free_page_mlock(struct page *page)
 {
-	__ClearPageMlocked(page);
 	__dec_zone_page_state(page, NR_MLOCK);
 	__count_vm_event(UNEVICTABLE_MLOCKFREED);
 }

@@ -558,7 +557,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	unsigned long flags;
 	int i;
 	int bad = 0;
-	int clearMlocked = PageMlocked(page);
+	int wasMlocked = TestClearPageMlocked(page);
 
 	kmemcheck_free_shadow(page, order);
 

@@ -576,7 +575,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	kernel_map_pages(page, 1 << order, 0);
 
 	local_irq_save(flags);
-	if (unlikely(clearMlocked))
+	if (unlikely(wasMlocked))
 		free_page_mlock(page);
 	__count_vm_events(PGFREE, 1 << order);
 	free_one_page(page_zone(page), page, order,

@@ -1022,7 +1021,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 	struct zone *zone = page_zone(page);
 	struct per_cpu_pages *pcp;
 	unsigned long flags;
-	int clearMlocked = PageMlocked(page);
+	int wasMlocked = TestClearPageMlocked(page);
 
 	kmemcheck_free_shadow(page, 0);
 

@@ -1041,7 +1040,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 	pcp = &zone_pcp(zone, get_cpu())->pcp;
 	set_page_private(page, get_pageblock_migratetype(page));
 	local_irq_save(flags);
-	if (unlikely(clearMlocked))
+	if (unlikely(wasMlocked))
 		free_page_mlock(page);
 	__count_vm_event(PGFREE);
Signature · 1 line
1.6.3
Signature · 3 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
author
#21 lore raw

Re: BUG] Strange oopses in 2.6.30

In reply to Hugh Dickins
Quoted 48 lines · Hugh Dickins
Quoted 7 lines · Maxim Levitsky
Quoted 6 lines · Maxim Levitsky

I see lots of following oopses in 2.6.30 and latest -git

Many different applications shows up, not just reiserfsck Something in MM I guess, it makes me worry. But system seems to work.

Is this known?

I think so...

Quoted 37 lines
Quoted 36 lines

dmesg attached.

[   34.544040] BUG: Bad page state in process reiserfsck  pfn:37d86
[   34.544044] page:c2a34f38 flags:3650000c count:0 mapcount:0
mapping:(null) index:bffeb
[   34.544048] Pid: 2654, comm: reiserfsck Tainted: G    B
2.6.30-git #4
[   34.544051] Call Trace:
[   34.544055]  [<c04cd26a>] ? printk+0x18/0x1e
[   34.544059]  [<c018f065>] bad_page+0xd5/0x140
[   34.544064]  [<c0190097>] free_hot_cold_page+0x1e7/0x280
[   34.544069]  [<c0193682>] ? release_pages+0x92/0x1b0
[   34.544074]  [<c0190155>] __pagevec_free+0x25/0x30
[   34.544078]  [<c0193758>] release_pages+0x168/0x1b0
[   34.544084]  [<c0193cf3>] ? lru_add_drain+0x53/0xd0
[   34.544088]  [<c01ab7d4>] free_pages_and_swap_cache+0x84/0xa0
[   34.544093]  [<c019ff5d>] unmap_vmas+0x73d/0x760
[   34.544099]  [<c016480e>] ? lock_release_non_nested+0x15e/0x270
[   34.544104]  [<c01a43b5>] exit_mmap+0xb5/0x1b0
[   34.544109]  [<c0138666>] mmput+0x36/0xc0
[   34.544113]  [<c013c874>] exit_mm+0xe4/0x120
[   34.544117]  [<c0175539>] ? acct_collect+0x139/0x180
[   34.544122]  [<c013e889>] do_exit+0x6b9/0x720
[   34.544142]  [<c01bcac2>] ? vfs_write+0x122/0x180
[   34.544146]  [<c01bbda0>] ? do_sync_write+0x0/0x110
[   34.544151]  [<c013e920>] do_group_exit+0x30/0x90
[   34.544156]  [<c013e993>] sys_exit_group+0x13/0x20
[   34.544161]  [<c01039e8>] sysenter_do_call+0x12/0x3c
[   34.544180] BUG: Bad page state in process reiserfsck  pfn:37d91
[   34.544184] page:c2a35174 flags:3650000c count:0 mapcount:0
mapping:(null) index:bfff6
[   34.544188] Pid: 2654, comm: reiserfsck Tainted: G    B

2.6.30-git #4

Quoted 3 lines
Quoted 1 line

This really worries me

I hope it's fixed by this patch Hannes posted yesterday...

Indeed, this helps, thanks a lot

Best regards,

Maxim Levitsky

#4 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to Jiri Slaby

(cc to Mel and some reviewer)

Quoted 5 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 24 lines · KOSAKI Motohiro

(cc to Mel and some reviewer)

Quoted 5 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);
Quoted 6 lines

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

Well, I am not sure we need to revert the patch. I'd argue it's simply a bug in free_pages_check() that can be fixed with something like this. Mel, what do you think?

Pekka

include/linux/page-flags.h +1 -1
index d6792f8..b002b65 100644
@@ -385,7 +385,7 @@ static inline void __ClearPageTail(struct page *page)
  * these flags set.  It they are, there is a problem.
  */
 #define PAGE_FLAGS_CHECK_AT_FREE \
-	(1 << PG_lru	 | 1 << PG_locked    | \
+	(1 << PG_lru	 | \
 	 1 << PG_private | 1 << PG_private_2 | \
 	 1 << PG_buddy	 | 1 << PG_writeback | 1 << PG_reserved | \
 	 1 << PG_slab	 | 1 << PG_swapcache | 1 << PG_active | \
mm/page_alloc.c +5 -0
index a5f3c27..ff7c713 100644
@@ -497,6 +497,11 @@ static void free_page_mlock(struct page *page) { }
 
 static inline int free_pages_check(struct page *page)
 {
+	/*
+	 * Note: the page can have PG_mlock set here because we clear it
+	 * lazily to avoid unnecessary disabling and enabling of interrupts in
+	 * page free fastpath.
+	 */
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#7 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to KOSAKI Motohiro
Quoted 24 lines · KOSAKI Motohiro

(cc to Mel and some reviewer)

Quoted 5 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);
Quoted 7 lines

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

I think a revert is way overkill. The intention of the patch is sound - reducing the number of times interrupts are disabled. Having pages with the PG_locked bit is now somewhat of an expected situation. I'd prefer to go with either

1. Unconditionally clearing the bit with TestClearPageLocked as the patch already posted does
2. Removing PG_locked from the free_pages_check()
3. Unlocking the pages as we go when an mlocked VMA is being torn town

The patch that addresses 1 seemed ok to me. What do you think?

Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#8 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 2 lines · Mel Gorman
Quoted 1 line · KOSAKI Motohiro

(cc to Mel and some reviewer)

[added Rik so that he can get multiple copies, too. :)]

Quoted 36 lines
Quoted 23 lines
Quoted 6 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);
Quoted 7 lines

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

I think a revert is way overkill. The intention of the patch is sound - reducing the number of times interrupts are disabled. Having pages with the PG_locked bit is now somewhat of an expected situation. I'd prefer to go with either

Quoted 4 lines

1. Unconditionally clearing the bit with TestClearPageLocked as the patch already posted does
2. Removing PG_locked from the free_pages_check()
3. Unlocking the pages as we go when an mlocked VMA is being torn town

Mel,

#3 SHOULD be happening in all cases. The free_page_mlocked() function counts when this is not happening. We tried to fix all cases that we encountered before this feature was submitted, but left the vm_stat there to report if more PG_mlocked leaks were introduced. We also, inadvertently, left PG_mlocked in the flags to check at free. We didn't hit this before your patch because free_page_mlock() did a test&clear on the PG_mlocked before checking the flags. Since you moved the call, and used PageMlocked() instead of TestClearPageMlocked(), any PG_locked page will cause the bug.

So, we have another PG_mlocked flag leaking to free. I don't think this is terribly serious in itself, and probably not deserving of a BUG_ON. It probably doesn't deserve a vm_stat, either, I guess. However, it could indicate a more serious logic error and should be examined. So it would be nice to retain some indication that it's happening.

Quoted 2 lines

The patch that addresses 1 seemed ok to me. What do you think?

Your alternative #2 sounds less expensive that test&clear.

Lee

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#9 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 47 lines · Lee Schermerhorn
Quoted 2 lines · Mel Gorman
Quoted 1 line · KOSAKI Motohiro

(cc to Mel and some reviewer)

[added Rik so that he can get multiple copies, too. :)]

Quoted 36 lines
Quoted 23 lines
Quoted 6 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);
Quoted 7 lines

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

I think a revert is way overkill. The intention of the patch is sound - reducing the number of times interrupts are disabled. Having pages with the PG_locked bit is now somewhat of an expected situation. I'd prefer to go with either

Quoted 4 lines

1. Unconditionally clearing the bit with TestClearPageLocked as the patch already posted does
2. Removing PG_locked from the free_pages_check()
3. Unlocking the pages as we go when an mlocked VMA is being torn town

Quoted 6 lines

Mel,

#3 SHOULD be happening in all cases. The free_page_mlocked() function counts when this is not happening. We tried to fix all cases that we encountered before this feature was submitted, but left the vm_stat there to report if more PG_mlocked leaks were introduced.

That makes sense. I was surprised at the thought that the pages were apparently not getting freed properly and upon investigation I could not trivially reproduce the problem. Can someone with this problem post their .config please in case I'm missing something in there?

Quoted 13 lines

We also,
inadvertently, left PG_mlocked in the flags to check at free. We didn't hit this before your patch because free_page_mlock() did a test&clear on the PG_mlocked before checking the flags. Since you moved the call, and used PageMlocked() instead of TestClearPageMlocked(), any PG_locked page will cause the bug.

So, we have another PG_mlocked flag leaking to free. I don't think this is terribly serious in itself, and probably not deserving of a BUG_ON. It probably doesn't deserve a vm_stat, either, I guess. However, it could indicate a more serious logic error and should be examined. So it would be nice to retain some indication that it's happening.

Quoted 5 lines
Quoted 2 lines

The patch that addresses 1 seemed ok to me. What do you think?

Your alternative #2 sounds less expensive that test&clear.

How about the following? The intention is to warn once when PG_mlocked is set but continue to count the number of times the event occured.

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but due to commit da456f14d [page allocator: do not disable interrupts in free_page_mlock()], the page state is being treated as a bad page which is considered a severe bug.

This bug drops the severity of the report in the event a page is freed with PG_mlocked set. A warning is printed once and the subsequent events counted.

Signed-off-by Mel Gorman <[email protected]>

---

 include/linux/page-flags.h |   10 +++++++++-
 mm/page_alloc.c            |    9 +++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
include/linux/page-flags.h +9 -1
index d6792f8..81731cf 100644
@@ -389,7 +389,15 @@ static inline void __ClearPageTail(struct page *page)
 	 1 << PG_private | 1 << PG_private_2 | \
 	 1 << PG_buddy	 | 1 << PG_writeback | 1 << PG_reserved | \
 	 1 << PG_slab	 | 1 << PG_swapcache | 1 << PG_active | \
-	 1 << PG_unevictable | __PG_MLOCKED)
+	 1 << PG_unevictable)
+
+/*
+ * Flags checked when a page is freed. Pages being freed should not have
+ * these set but the situation is easily resolved and should just be
+ * reported as a once-off warning.
+ */
+#define PAGE_FLAGS_WARN_AT_FREE \
+	(__PG_MLOCKED)
 
 /*
  * Flags checked when a page is prepped for return by the page allocator.
mm/page_alloc.c +9 -0
index a5f3c27..c8c029e 100644
@@ -497,6 +497,15 @@ static void free_page_mlock(struct page *page) { }
 
 static inline int free_pages_check(struct page *page)
 {
+	if (unlikely(page->flags & PAGE_FLAGS_WARN_AT_FREE)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Sloppy page flags set process %s at pfn:%05lx\n"
+			"page:%p flags:%p\n",
+			current->comm, page_to_pfn(page),
+			page, (void *)page->flags);
+		page->flags &= ~PAGE_FLAGS_WARN_AT_FREE;
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#6 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to Pekka Enberg
Quoted 27 lines · Pekka Enberg
Quoted 26 lines · KOSAKI Motohiro

(cc to Mel and some reviewer)

Quoted 5 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);

-------------------------------------------------------------

Quoted 9 lines
Quoted 4 lines

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

Well, I am not sure we need to revert the patch. I'd argue it's simply a bug in free_pages_check() that can be fixed with something like this. Mel, what do you think?

I think you removed the check for the wrong flag - PG_locked vs PG_mlocked :).

That aside, I reckon your intention was not far off the mark. I posted a separate patch to see about warning once when PG_mlocked is set and counting the event. When the warning appears, it's not world ending but chances are it's something that needs to be fixed up.

Quoted 33 lines

Pekka

include/linux/page-flags.h +1 -1
index d6792f8..b002b65 100644
@@ -385,7 +385,7 @@ static inline void __ClearPageTail(struct page *page)
  * these flags set.  It they are, there is a problem.
  */
 #define PAGE_FLAGS_CHECK_AT_FREE \
-	(1 << PG_lru	 | 1 << PG_locked    | \
+	(1 << PG_lru	 | \
 	 1 << PG_private | 1 << PG_private_2 | \
 	 1 << PG_buddy	 | 1 << PG_writeback | 1 << PG_reserved | \
 	 1 << PG_slab	 | 1 << PG_swapcache | 1 << PG_active | \
mm/page_alloc.c +5 -0
index a5f3c27..ff7c713 100644
@@ -497,6 +497,11 @@ static void free_page_mlock(struct page *page) { }
 
 static inline int free_pages_check(struct page *page)
 {
+	/*
+	 * Note: the page can have PG_mlock set here because we clear it
+	 * lazily to avoid unnecessary disabling and enabling of interrupts in
+	 * page free fastpath.
+	 */
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |
Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#18 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to Mel Gorman
Quoted 38 lines · KOSAKI Motohiro
Quoted 24 lines

(cc to Mel and some reviewer)

Quoted 5 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);
Quoted 7 lines

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

I think a revert is way overkill. The intention of the patch is sound - reducing the number of times interrupts are disabled. Having pages with the PG_locked bit is now somewhat of an expected situation. I'd prefer to go with either

Quoted 6 lines

1. Unconditionally clearing the bit with TestClearPageLocked as the patch already posted does
2. Removing PG_locked from the free_pages_check()
3. Unlocking the pages as we go when an mlocked VMA is being torn town

The patch that addresses 1 seemed ok to me. What do you think?

Yes, I've overlooked Hanns's patch. I think that is good patch. Thansk folks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#10 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to Mel Gorman
Quoted 127 lines · Lee Schermerhorn
Quoted 47 lines · Mel Gorman
Quoted 2 lines · KOSAKI Motohiro
Quoted 1 line

(cc to Mel and some reviewer)

[added Rik so that he can get multiple copies, too. :)]

Quoted 36 lines
Quoted 23 lines
Quoted 6 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);
Quoted 7 lines

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

I think a revert is way overkill. The intention of the patch is sound - reducing the number of times interrupts are disabled. Having pages with the PG_locked bit is now somewhat of an expected situation. I'd prefer to go with either

Quoted 4 lines

1. Unconditionally clearing the bit with TestClearPageLocked as the patch already posted does
2. Removing PG_locked from the free_pages_check()
3. Unlocking the pages as we go when an mlocked VMA is being torn town

Quoted 6 lines

Mel,

#3 SHOULD be happening in all cases. The free_page_mlocked() function counts when this is not happening. We tried to fix all cases that we encountered before this feature was submitted, but left the vm_stat there to report if more PG_mlocked leaks were introduced.

That makes sense. I was surprised at the thought that the pages were apparently not getting freed properly and upon investigation I could not trivially reproduce the problem. Can someone with this problem post their .config please in case I'm missing something in there?

Quoted 13 lines

We also,
inadvertently, left PG_mlocked in the flags to check at free. We didn't hit this before your patch because free_page_mlock() did a test&clear on the PG_mlocked before checking the flags. Since you moved the call, and used PageMlocked() instead of TestClearPageMlocked(), any PG_locked page will cause the bug.

So, we have another PG_mlocked flag leaking to free. I don't think this is terribly serious in itself, and probably not deserving of a BUG_ON. It probably doesn't deserve a vm_stat, either, I guess. However, it could indicate a more serious logic error and should be examined. So it would be nice to retain some indication that it's happening.

Quoted 5 lines
Quoted 2 lines

The patch that addresses 1 seemed ok to me. What do you think?

Your alternative #2 sounds less expensive that test&clear.

How about the following? The intention is to warn once when PG_mlocked is set but continue to count the number of times the event occured.

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but due to commit da456f14d [page allocator: do not disable interrupts in free_page_mlock()], the page state is being treated as a bad page which is considered a severe bug.

This bug drops the severity of the report in the event a page is freed with PG_mlocked set. A warning is printed once and the subsequent events counted.

Signed-off-by Mel Gorman <[email protected]>

---

 include/linux/page-flags.h |   10 +++++++++-
 mm/page_alloc.c            |    9 +++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
include/linux/page-flags.h +9 -1
index d6792f8..81731cf 100644
@@ -389,7 +389,15 @@ static inline void __ClearPageTail(struct page *page)
 	 1 << PG_private | 1 << PG_private_2 | \
 	 1 << PG_buddy	 | 1 << PG_writeback | 1 << PG_reserved | \
 	 1 << PG_slab	 | 1 << PG_swapcache | 1 << PG_active | \
-	 1 << PG_unevictable | __PG_MLOCKED)
+	 1 << PG_unevictable)
+
+/*
+ * Flags checked when a page is freed. Pages being freed should not have
+ * these set but the situation is easily resolved and should just be
+ * reported as a once-off warning.
+ */
+#define PAGE_FLAGS_WARN_AT_FREE \
+	(__PG_MLOCKED)
 
 /*
  * Flags checked when a page is prepped for return by the page allocator.
mm/page_alloc.c +0 -0
index a5f3c27..c8c029e 100644

mm/page_alloc.c static void free_page_mlock(struct page *page) { } · line 500

Quoted 4 lines
mm/page_alloc.c +1 -0
@@ -497,6 +497,15 @@ static void free_page_mlock(struct page *page) { }
 
 static inline int free_pages_check(struct page *page)
 {
+	if (unlikely(page->flags & PAGE_FLAGS_WARN_AT_FREE)) {

this condition is always false. it's because caller clear PG_Mlocked flag before calling free_pages_check().

mm/page_alloc.c static void free_page_mlock(struct page *page) { } · line 505

Quoted 5 lines
mm/page_alloc.c +5 -0
+		WARN_ONCE(1, KERN_WARNING
+			"Sloppy page flags set process %s at pfn:%05lx\n"
+			"page:%p flags:%p\n",
+			current->comm, page_to_pfn(page),
+			page, (void *)page->flags);

hmm, mystery (void*) casting is here.

mm/page_alloc.c static void free_page_mlock(struct page *page) { } · line 511

Quoted 6 lines
mm/page_alloc.c +3 -0
+		page->flags &= ~PAGE_FLAGS_WARN_AT_FREE;
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

Howerver, I like this patch concept. this warning is useful and meaningful IMHO.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#11 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 128 lines · KOSAKI Motohiro
Quoted 128 lines · Lee Schermerhorn
Quoted 47 lines · Mel Gorman
Quoted 2 lines · KOSAKI Motohiro
Quoted 1 line

(cc to Mel and some reviewer)

[added Rik so that he can get multiple copies, too. :)]

Quoted 36 lines
Quoted 23 lines
Quoted 6 lines

Flags are:
0000000000400000 -- __PG_MLOCKED
800000000050000c -- my page flags

        3650000c -- Maxim's page flags

0000000000693ce1 -- my PAGE_FLAGS_CHECK_AT_FREE

I guess commit da456f14d (page allocator: do not disable interrupts in free_page_mlock()) is a bit wrong.

current code is:
-------------------------------------------------------------
static void free_hot_cold_page(struct page *page, int cold) {
(snip)

        int clearMlocked = PageMlocked(page);
(snip)
        if (free_pages_check(page))
                return;
(snip)
        local_irq_save(flags);
        if (unlikely(clearMlocked))
                free_page_mlock(page);
Quoted 7 lines

-------------------------------------------------------------

Oh well, we remove PG_Mlocked *after* free_pages_check(). Then, it makes false-positive warning.

Sorry, my review was also wrong. I think reverting this patch is better ;)

I think a revert is way overkill. The intention of the patch is sound - reducing the number of times interrupts are disabled. Having pages with the PG_locked bit is now somewhat of an expected situation. I'd prefer to go with either

Quoted 4 lines

1. Unconditionally clearing the bit with TestClearPageLocked as the patch already posted does
2. Removing PG_locked from the free_pages_check()
3. Unlocking the pages as we go when an mlocked VMA is being torn town

Quoted 6 lines

Mel,

#3 SHOULD be happening in all cases. The free_page_mlocked() function counts when this is not happening. We tried to fix all cases that we encountered before this feature was submitted, but left the vm_stat there to report if more PG_mlocked leaks were introduced.

That makes sense. I was surprised at the thought that the pages were apparently not getting freed properly and upon investigation I could not trivially reproduce the problem. Can someone with this problem post their .config please in case I'm missing something in there?

Quoted 13 lines

We also,
inadvertently, left PG_mlocked in the flags to check at free. We didn't hit this before your patch because free_page_mlock() did a test&clear on the PG_mlocked before checking the flags. Since you moved the call, and used PageMlocked() instead of TestClearPageMlocked(), any PG_locked page will cause the bug.

So, we have another PG_mlocked flag leaking to free. I don't think this is terribly serious in itself, and probably not deserving of a BUG_ON. It probably doesn't deserve a vm_stat, either, I guess. However, it could indicate a more serious logic error and should be examined. So it would be nice to retain some indication that it's happening.

Quoted 5 lines
Quoted 2 lines

The patch that addresses 1 seemed ok to me. What do you think?

Your alternative #2 sounds less expensive that test&clear.

How about the following? The intention is to warn once when PG_mlocked is set but continue to count the number of times the event occured.

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but due to commit da456f14d [page allocator: do not disable interrupts in free_page_mlock()], the page state is being treated as a bad page which is considered a severe bug.

This bug drops the severity of the report in the event a page is freed with PG_mlocked set. A warning is printed once and the subsequent events counted.

Signed-off-by Mel Gorman <[email protected]>

---

 include/linux/page-flags.h |   10 +++++++++-
 mm/page_alloc.c            |    9 +++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
include/linux/page-flags.h +9 -1
index d6792f8..81731cf 100644
@@ -389,7 +389,15 @@ static inline void __ClearPageTail(struct page *page)
 	 1 << PG_private | 1 << PG_private_2 | \
 	 1 << PG_buddy	 | 1 << PG_writeback | 1 << PG_reserved | \
 	 1 << PG_slab	 | 1 << PG_swapcache | 1 << PG_active | \
-	 1 << PG_unevictable | __PG_MLOCKED)
+	 1 << PG_unevictable)
+
+/*
+ * Flags checked when a page is freed. Pages being freed should not have
+ * these set but the situation is easily resolved and should just be
+ * reported as a once-off warning.
+ */
+#define PAGE_FLAGS_WARN_AT_FREE \
+	(__PG_MLOCKED)
 
 /*
  * Flags checked when a page is prepped for return by the page allocator.
mm/page_alloc.c +0 -0
index a5f3c27..c8c029e 100644
@@ -497,6 +497,15 @@ static void free_page_mlock(struct page *page) { }
Quoted 7 lines

mm/page_alloc.c static void free_page_mlock(struct page *page) { } · line 500

Quoted 3 lines
mm/page_alloc.c +1 -0
 static inline int free_pages_check(struct page *page)
 {
+	if (unlikely(page->flags & PAGE_FLAGS_WARN_AT_FREE)) {

this condition is always false. it's because caller clear PG_Mlocked flag before calling free_pages_check().

This patch is intended as an alternative to the patch that replaces PageMlocked() with TestClearPageMlocked() so I expect the flag to only be set in the situation where PG_mlocked is not being cleared properly.

I see the unconditionoal clearing of the flag was merged since but even that might be too heavy handed as we are making a locked bit operation on every page free. That's unfortunate overhead to incur on every page free to handle a situation that should not be occurring at all.

Quoted 9 lines

mm/page_alloc.c static void free_page_mlock(struct page *page) { } · line 505

Quoted 6 lines
mm/page_alloc.c +5 -0
+		WARN_ONCE(1, KERN_WARNING
+			"Sloppy page flags set process %s at pfn:%05lx\n"
+			"page:%p flags:%p\n",
+			current->comm, page_to_pfn(page),
+			page, (void *)page->flags);

hmm, mystery (void*) casting is here.

Code was taken from bad_page(). I should have used 0x%lX here.

Quoted 10 lines

mm/page_alloc.c static void free_page_mlock(struct page *page) { } · line 511

Quoted 7 lines
mm/page_alloc.c +3 -0
+		page->flags &= ~PAGE_FLAGS_WARN_AT_FREE;
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

Howerver, I like this patch concept. this warning is useful and meaningful IMHO.

This is a version that is based on top of current mainline that just displays the warning. However, I think we should consider changing TestClearPageMlocked() back to PageMlocked() and only clearing the flags when the unusual condition is encountered.

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but it is easy to miss that this event has occured at all. This patch warns once when PG_mlocked is set to prompt debuggers to check the counter to see how often it is happening.

Signed-off-by Mel Gorman <[email protected]>

---

 mm/page_alloc.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
mm/page_alloc.c +12 -4
index 5d714f8..519ea6e 100644
@@ -495,8 +495,16 @@ static inline void free_page_mlock(struct page *page)
 static void free_page_mlock(struct page *page) { }
 #endif
 
-static inline int free_pages_check(struct page *page)
-{
+static inline int free_pages_check(struct page *page, int wasMlocked)
+{
+	if (unlikely(wasMlocked)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Page flag mlocked set for process %s at pfn:%05lx\n"
+			"page:%p flags:0x%lX\n",
+			current->comm, page_to_pfn(page),
+			page, page->flags|__PG_MLOCKED);
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

@@ -562,7 +570,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	kmemcheck_free_shadow(page, order);
 
 	for (i = 0 ; i < (1 << order) ; ++i)
-		bad += free_pages_check(page + i);
+		bad += free_pages_check(page + i, wasMlocked);
 	if (bad)
 		return;
 

@@ -1027,7 +1035,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 
 	if (PageAnon(page))
 		page->mapping = NULL;
-	if (free_pages_check(page))
+	if (free_pages_check(page, wasMlocked))
 		return;
 
 	if (!PageHighMem(page)) {
Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
reviewer
#12 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 4 lines · Mel Gorman

I see the unconditionoal clearing of the flag was merged since but even that might be too heavy handed as we are making a locked bit operation on every page free. That's unfortunate overhead to incur on every page free to handle a situation that should not be occurring at all.

Linus was probably quick to merge it as istr several people hitting bad_page() triggering. We should get rid of the locked op, I was just not 100% sure and chose the safer version.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
(quoted diff) +5 -0
+		WARN_ONCE(1, KERN_WARNING
+			"Sloppy page flags set process %s at pfn:%05lx\n"
+			"page:%p flags:%p\n",
+			current->comm, page_to_pfn(page),
+			page, (void *)page->flags);

[...]

Quoted 10 lines
Quoted 9 lines

mm/page_alloc.c static inline void free_page_mlock(struct page *page) · line 510

Quoted 6 lines
mm/page_alloc.c +3 -0
+		page->flags &= ~PAGE_FLAGS_WARN_AT_FREE;
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

Howerver, I like this patch concept. this warning is useful and meaningful IMHO.

Quoted 4 lines

This is a version that is based on top of current mainline that just displays the warning. However, I think we should consider changing TestClearPageMlocked() back to PageMlocked() and only clearing the flags when the unusual condition is encountered.

I have a diff at home that makes this an unlocked __TestClearPageMlocked(), would you be okay with this?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#13 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 10 lines · Johannes Weiner
Quoted 4 lines · Mel Gorman

I see the unconditionoal clearing of the flag was merged since but even that might be too heavy handed as we are making a locked bit operation on every page free. That's unfortunate overhead to incur on every page free to handle a situation that should not be occurring at all.

Linus was probably quick to merge it as istr several people hitting bad_page() triggering. We should get rid of the locked op, I was just not 100% sure and chose the safer version.

And I have no problem with the decision. Leaving it as it was would have caused a storm of bug reports, all similar.

Quoted 16 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
(quoted diff) +5 -0
+		WARN_ONCE(1, KERN_WARNING
+			"Sloppy page flags set process %s at pfn:%05lx\n"
+			"page:%p flags:%p\n",
+			current->comm, page_to_pfn(page),
+			page, (void *)page->flags);

[...]

Quoted 10 lines
Quoted 9 lines

mm/page_alloc.c static inline void free_page_mlock(struct page *page) · line 510

Quoted 6 lines
mm/page_alloc.c +3 -0
+		page->flags &= ~PAGE_FLAGS_WARN_AT_FREE;
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

Howerver, I like this patch concept. this warning is useful and meaningful IMHO.

Quoted 8 lines
Quoted 4 lines

This is a version that is based on top of current mainline that just displays the warning. However, I think we should consider changing TestClearPageMlocked() back to PageMlocked() and only clearing the flags when the unusual condition is encountered.

I have a diff at home that makes this an unlocked __TestClearPageMlocked(), would you be okay with this?

It'd be an improvement for sure. Post it and I'll take a look.

My preference is still to clear the flag only when found to be erroneously set and print a warning once but that's because it was the patch I put together so I'm biased :)

Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#14 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to Mel Gorman

Hi

Thank you new version.

Quoted 20 lines

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but it is easy to miss that this event has occured at all. This patch warns once when PG_mlocked is set to prompt debuggers to check the counter to see how often it is happening.

Signed-off-by Mel Gorman <[email protected]>

---

 mm/page_alloc.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
mm/page_alloc.c +0 -0
index 5d714f8..519ea6e 100644
@@ -495,8 +495,16 @@ static inline void free_page_mlock(struct page *page)
 static void free_page_mlock(struct page *page) { }

mm/page_alloc.c static inline void free_page_mlock(struct page *page) · line 503

Quoted 10 lines
mm/page_alloc.c +6 -2
 #endif
 
-static inline int free_pages_check(struct page *page)
-{
+static inline int free_pages_check(struct page *page, int wasMlocked)
+{
+	if (unlikely(wasMlocked)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Page flag mlocked set for process %s at pfn:%05lx\n"
+			"page:%p flags:0x%lX\n",

0x%lX is a bit redundunt.
%lX insert "0x" string by itself, I think.

Quoted 17 lines
mm/page_alloc.c +5 -1
+			current->comm, page_to_pfn(page),
+			page, page->flags|__PG_MLOCKED);
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

@@ -562,7 +570,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	kmemcheck_free_shadow(page, order);
 
 	for (i = 0 ; i < (1 << order) ; ++i)
-		bad += free_pages_check(page + i);
+		bad += free_pages_check(page + i, wasMlocked);
 	if (bad)
 		return;

mm/page_alloc.c static void free_hot_cold_page(struct page *page, int cold) · line 1041

Quoted 8 lines
mm/page_alloc.c +1 -1
@@ -1027,7 +1035,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 
 	if (PageAnon(page))
 		page->mapping = NULL;
-	if (free_pages_check(page))
+	if (free_pages_check(page, wasMlocked))
 		return;
 
 	if (!PageHighMem(page)) {

Other part looks fine. thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#15 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 26 lines · KOSAKI Motohiro

Hi

Thank you new version.

Quoted 22 lines

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but it is easy to miss that this event has occured at all. This patch warns once when PG_mlocked is set to prompt debuggers to check the counter to see how often it is happening.

Signed-off-by Mel Gorman <[email protected]>

---

 mm/page_alloc.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
mm/page_alloc.c +0 -0
index 5d714f8..519ea6e 100644
@@ -495,8 +495,16 @@ static inline void free_page_mlock(struct page *page)
 static void free_page_mlock(struct page *page) { }
 #endif
Quoted 12 lines

mm/page_alloc.c static inline void free_page_mlock(struct page *page) · line 503

Quoted 8 lines
mm/page_alloc.c +6 -2
-static inline int free_pages_check(struct page *page)
-{
+static inline int free_pages_check(struct page *page, int wasMlocked)
+{
+	if (unlikely(wasMlocked)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Page flag mlocked set for process %s at pfn:%05lx\n"
+			"page:%p flags:0x%lX\n",

0x%lX is a bit redundunt.
%lX insert "0x" string by itself, I think.

It does not automatically insert the 0x for me and I just did a quick test there. Can you double check please?

Quoted 31 lines
Quoted 18 lines
mm/page_alloc.c +5 -1
+			current->comm, page_to_pfn(page),
+			page, page->flags|__PG_MLOCKED);
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

@@ -562,7 +570,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	kmemcheck_free_shadow(page, order);
 
 	for (i = 0 ; i < (1 << order) ; ++i)
-		bad += free_pages_check(page + i);
+		bad += free_pages_check(page + i, wasMlocked);
 	if (bad)
 		return;

mm/page_alloc.c static void free_hot_cold_page(struct page *page, int cold) · line 1041

Quoted 8 lines
mm/page_alloc.c +1 -1
@@ -1027,7 +1035,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 
 	if (PageAnon(page))
 		page->mapping = NULL;
-	if (free_pages_check(page))
+	if (free_pages_check(page, wasMlocked))
 		return;
 
 	if (!PageHighMem(page)) {

Other part looks fine. thanks.

Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#16 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

In reply to KOSAKI Motohiro
Quoted 12 lines · KOSAKI Motohiro

Review

Quoted 8 lines
(quoted diff) +6 -2
-static inline int free_pages_check(struct page *page)
-{
+static inline int free_pages_check(struct page *page, int wasMlocked)
+{
+	if (unlikely(wasMlocked)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Page flag mlocked set for process %s at pfn:%05lx\n"
+			"page:%p flags:0x%lX\n",

0x%lX is a bit redundunt.
%lX insert "0x" string by itself, I think.

/me slaps self

As hnaz pointed out to me on IRC, %#lX would have done the job of putting in the 0x automatically.

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but it is easy to miss that this event has occured at all. This patch warns once when PG_mlocked is set to prompt debuggers to check the counter to see how often it is happening.

Signed-off-by Mel Gorman <[email protected]>
---
 mm/page_alloc.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
mm/page_alloc.c +12 -4
index 5d714f8..519ea6e 100644
@@ -495,8 +495,16 @@ static inline void free_page_mlock(struct page *page)
 static void free_page_mlock(struct page *page) { }
 #endif
 
-static inline int free_pages_check(struct page *page)
-{
+static inline int free_pages_check(struct page *page, int wasMlocked)
+{
+	if (unlikely(wasMlocked)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Page flag mlocked set for process %s at pfn:%05lx\n"
+			"page:%p flags:%#lX\n",
+			current->comm, page_to_pfn(page),
+			page, page->flags|__PG_MLOCKED);
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

@@ -562,7 +570,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	kmemcheck_free_shadow(page, order);
 
 	for (i = 0 ; i < (1 << order) ; ++i)
-		bad += free_pages_check(page + i);
+		bad += free_pages_check(page + i, wasMlocked);
 	if (bad)
 		return;
 

@@ -1027,7 +1035,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 
 	if (PageAnon(page))
 		page->mapping = NULL;
-	if (free_pages_check(page))
+	if (free_pages_check(page, wasMlocked))
 		return;
 
 	if (!PageHighMem(page)) {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Reviewed-by
#17 lore raw

Re: BUG: Bad page state [was: Strange oopses in 2.6.30]

Quoted 14 lines · KOSAKI Motohiro
Quoted 12 lines

mm/page_alloc.c static inline void free_page_mlock(struct page *page)

Quoted 8 lines
mm/page_alloc.c +6 -2
-static inline int free_pages_check(struct page *page)
-{
+static inline int free_pages_check(struct page *page, int wasMlocked)
+{
+	if (unlikely(wasMlocked)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Page flag mlocked set for process %s at pfn:%05lx\n"
+			"page:%p flags:0x%lX\n",

0x%lX is a bit redundunt.
%lX insert "0x" string by itself, I think.

Quoted 4 lines

/me slaps self

As hnaz pointed out to me on IRC, %#lX would have done the job of putting in the 0x automatically.

No. it's my fault. my last mail has typo (i forgot to write '#'), sorry.

Quoted 48 lines

==== CUT HERE ====
mm: Warn once when a page is freed with PG_mlocked set

When a page is freed with the PG_mlocked set, it is considered an unexpected but recoverable situation. A counter records how often this event happens but it is easy to miss that this event has occured at all. This patch warns once when PG_mlocked is set to prompt debuggers to check the counter to see how often it is happening.

Signed-off-by Mel Gorman <[email protected]>
---
 mm/page_alloc.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
mm/page_alloc.c +11 -3
index 5d714f8..519ea6e 100644
@@ -495,8 +495,16 @@ static inline void free_page_mlock(struct page *page)
 static void free_page_mlock(struct page *page) { }
 #endif
 
-static inline int free_pages_check(struct page *page)
-{
+static inline int free_pages_check(struct page *page, int wasMlocked)
+{
+	if (unlikely(wasMlocked)) {
+		WARN_ONCE(1, KERN_WARNING
+			"Page flag mlocked set for process %s at pfn:%05lx\n"
+			"page:%p flags:%#lX\n",
+			current->comm, page_to_pfn(page),
+			page, page->flags|__PG_MLOCKED);
+	}
+
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |

@@ -562,7 +570,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	kmemcheck_free_shadow(page, order);
 
 	for (i = 0 ; i < (1 << order) ; ++i)
-		bad += free_pages_check(page + i);
+		bad += free_pages_check(page + i, wasMlocked);
 	if (bad)
 		return;

mm/page_alloc.c static void free_hot_cold_page(struct page *page, int cold) · line 1041

Quoted 8 lines
mm/page_alloc.c +1 -1
@@ -1027,7 +1035,7 @@ static void free_hot_cold_page(struct page *page, int cold)
 
 	if (PageAnon(page))
 		page->mapping = NULL;
-	if (free_pages_check(page))
+	if (free_pages_check(page, wasMlocked))
 		return;
 
 	if (!PageHighMem(page)) {

OK, looks fine.

Reviewed-by KOSAKI Motohiro <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>