mm:add VM_BUG_ON() for page_mapcount()

PATCH Superseded Hillf Danton 4 participants 7 messages Dec 8, 2014 – Jun 9, 2015

Subsystems (2)

Memory management MM core

lore

Share & tools

lore

Hillf Danton

author

2014-12-08 09:33 UTC #0 lore raw

Quoted 16 lines

This patch add VM_BUG_ON() for slab page,
because _mapcount is an union with slab struct in struct page, avoid access _mapcount if this page is a slab page. Also remove the unneeded bracket.
Signed-off-by Yalin Wang <[email protected]>
---
 include/linux/mm.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
include/linux/mm.h +0 -0
index 11b65cf..34124c4 100644

include/linux/mm.h static inline void reset_page_mapcount(struct page *page) · line 376

Quoted 5 lines

include/linux/mm.h +1 -1

@@ -373,7 +373,8 @@ static inline void reset_page_mapcount(struct page *page)
 
 static inline int page_mapcount(struct page *page)
 {
-	return atomic_read(&(page)->_mapcount) + 1;
+	VM_BUG_ON(PageSlab(page));

s/ VM_BUG_ON/ VM_BUG_ON_PAGE/ ?

include/linux/mm.h static inline void reset_page_mapcount(struct page *page) · line 379

Quoted 3 lines

include/linux/mm.h +1 -0

+	return atomic_read(&page->_mapcount) + 1;
 }

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

Wang, Yalin

2014-12-08 09:51 UTC #1 lore raw

Quoted 26 lines

-----Original Message-----
From: Hillf Danton [mailto:hillf.zj at alibaba-inc.com]
Sent: Monday, December 08, 2014 5:33 PM
To: Wang, Yalin
Cc: linux-kernel; linux-mm at kvack.org; linux-arm-kernel at lists.infradead.org; Andrew Morton; Hillf Danton
Subject: Re: [PATCH] mm:add VM_BUG_ON() for page_mapcount()
Quoted 18 lines
This patch add VM_BUG_ON() for slab page, because _mapcount is an union with slab struct in struct page, avoid access _mapcount if this page is a slab page.
Also remove the unneeded bracket.
Signed-off-by Yalin Wang <[email protected]>
---
 include/linux/mm.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
include/linux/mm.h index renamed from include/linux/mm.h +0 -0
11b65cf..34124c4 100644
include/linux/mm.h +0 -0
@@ -373,7 +373,8 @@ static inline void reset_page_mapcount(struct page
 *page)
 

Quoted 5 lines

include/linux/mm.h static inline void reset_page_mapcount(struct page · line 376
Quoted 3 lines
include/linux/mm.h +1 -1
 static inline int page_mapcount(struct page *page)  {
-	return atomic_read(&(page)->_mapcount) + 1;
+	VM_BUG_ON(PageSlab(page));
s/ VM_BUG_ON/ VM_BUG_ON_PAGE/ ?

Yes, I will send again .

Wang, Yalin

2014-12-08 09:58 UTC #2 lore raw

PATCH v2 1/1 mm:add VM_BUG_ON() for page_mapcount()

In reply to Hillf Danton

This patch add VM_BUG_ON_PAGE() for slab page, because _mapcount is an union with slab struct in struct page, avoid access _mapcount if this page is a slab page. Also remove the unneeded bracket.

Signed-off-by Yalin Wang <[email protected]>

---
 include/linux/mm.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

include/linux/mm.h +2 -1

index b464611..a117527 100644
@@ -449,7 +449,8 @@ static inline void page_mapcount_reset(struct page *page)
 
 static inline int page_mapcount(struct page *page)
 {
-	return atomic_read(&(page)->_mapcount) + 1;
+	VM_BUG_ON_PAGE(PageSlab(page), page);
+	return atomic_read(&page->_mapcount) + 1;
 }
 
 static inline int page_count(struct page *page)

Signature · 1 line

2.1.3

Wang, Yalin

2014-12-08 09:59 UTC #3 lore raw

PATCH v3 1/1 mm:add VM_BUG_ON_PAGE() for page_mapcount()

Signed-off-by Yalin Wang <[email protected]>

---
 include/linux/mm.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

include/linux/mm.h +2 -1

index b464611..a117527 100644
@@ -449,7 +449,8 @@ static inline void page_mapcount_reset(struct page *page)
 
 static inline int page_mapcount(struct page *page)
 {
-	return atomic_read(&(page)->_mapcount) + 1;
+	VM_BUG_ON_PAGE(PageSlab(page), page);
+	return atomic_read(&page->_mapcount) + 1;
 }
 
 static inline int page_count(struct page *page)

Signature · 1 line

2.1.3

Kirill A. Shutemov

Acked-by on v3 1/1

2014-12-08 11:54 UTC #4 lore raw

Re: [PATCH V3] mm:add VM_BUG_ON_PAGE() for page_mapcount()

Quoted 6 lines · Yalin

This patch add VM_BUG_ON_PAGE() for slab page, because _mapcount is an union with slab struct in struct page, avoid access _mapcount if this page is a slab page. Also remove the unneeded bracket.
Signed-off-by Yalin Wang <[email protected]>

Acked-by Kirill A. Shutemov <[email protected]>

Signature · 1 line

 Kirill A. Shutemov

Hillf Danton

author Acked-by on v3 1/1

2014-12-09 03:18 UTC #5 lore raw

Re: [PATCH V3] mm:add VM_BUG_ON_PAGE() for page_mapcount()

In reply to Wang, Yalin

Quoted 8 lines

This patch add VM_BUG_ON_PAGE() for slab page, because _mapcount is an union with slab struct in struct page, avoid access _mapcount if this page is a slab page. Also remove the unneeded bracket.
Signed-off-by Yalin Wang <[email protected]>
---

Acked-by Hillf Danton <[email protected]>

Quoted 19 lines

 include/linux/mm.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

include/linux/mm.h +2 -2

index b464611..a117527 100644
@@ -449,7 +449,8 @@ static inline void page_mapcount_reset(struct page *page)
 
 static inline int page_mapcount(struct page *page)
 {
-	return atomic_read(&(page)->_mapcount) + 1;
+	VM_BUG_ON_PAGE(PageSlab(page), page);
+	return atomic_read(&page->_mapcount) + 1;
 }
 
 static inline int page_count(struct page *page)
--

2.1.3

Vlastimil Babka

2015-06-09 16:14 UTC #6 lore raw

Re: [PATCH V3] mm:add VM_BUG_ON_PAGE() for page_mapcount()

In reply to Wang, Yalin

Quoted 15 lines · Yalin

This patch add VM_BUG_ON_PAGE() for slab page, because _mapcount is an union with slab struct in struct page, avoid access _mapcount if this page is a slab page. Also remove the unneeded bracket.
Signed-off-by Yalin Wang <[email protected]>
---
  include/linux/mm.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
include/linux/mm.h +0 -0
index b464611..a117527 100644

include/linux/mm.h static inline void page_mapcount_reset(struct page *page) · line 455

Quoted 8 lines

include/linux/mm.h +2 -1

@@ -449,7 +449,8 @@ static inline void page_mapcount_reset(struct page *page)
 
  static inline int page_mapcount(struct page *page)
  {
-	return atomic_read(&(page)->_mapcount) + 1;
+	VM_BUG_ON_PAGE(PageSlab(page), page);
+	return atomic_read(&page->_mapcount) + 1;
  }

I think this might theoretically trigger on the following code in compaction's isolate_migratepages_block():

/*
* Migration will fail if an anonymous page is pinned in memory,
* so avoid taking lru_lock and isolating it unnecessarily in an
* admittedly racy check.
*/
if (!page_mapping(page) &&
page_count(page) > page_mapcount(page))
continue;

This is done after PageLRU() was positive, but the lru_lock might be not taken yet. So, there's some time window during which the page might have been reclaimed from LRU and become a PageSlab(page). !page_mapping(page) will be true in that case so it will proceed with page_mapcount(page) test and trigger the VM_BUG_ON.

(That test was added by DavidR year ago in commit
119d6d59dcc0980dcd581fdadb6b2033b512a473)

Vlastimil

Quoted 2 lines

include/linux/mm.h +0 -0

  static inline int page_count(struct page *page)